[Intel-gfx] ✗ Fi.CI.BAT: failure for Improve crc-core driver interface (rev12)
== Series Details ==
Series: Improve crc-core driver interface (rev12)
URL : https://patchwork.freedesktop.org/series/45246/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4664 -> Patchwork_9932 =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_9932 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9932, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/45246/revisions/12/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9932:
=== IGT changes ===
Possible regressions
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
fi-ivb-3520m: PASS -> FAIL +5
fi-skl-6700hq: PASS -> FAIL +5
fi-skl-guc: PASS -> FAIL +5
fi-blb-e6850: PASS -> FAIL +3
fi-byt-j1900: PASS -> FAIL +3
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-ilk-650: PASS -> FAIL +3
fi-elk-e7500: PASS -> FAIL +3
fi-byt-n2820: PASS -> FAIL +3
fi-snb-2520m: PASS -> FAIL +3
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
fi-bdw-5557u: PASS -> FAIL +5
fi-pnv-d510:PASS -> FAIL +3
fi-skl-6600u: PASS -> FAIL +5
{fi-byt-clapper}: PASS -> FAIL +3
fi-bxt-dsi: PASS -> FAIL +5
fi-hsw-4770:PASS -> FAIL +5
fi-ivb-3770:PASS -> FAIL +5
fi-bwr-2160:PASS -> FAIL +3
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
{fi-bdw-samus}: PASS -> FAIL +5
fi-hsw-peppy: PASS -> FAIL +5
fi-bdw-gvtdvm: PASS -> FAIL +5
fi-gdg-551: PASS -> FAIL +3
fi-kbl-7500u: PASS -> FAIL +5
fi-snb-2600:PASS -> FAIL +3
fi-hsw-4770r: PASS -> FAIL +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
fi-kbl-7567u: PASS -> FAIL +5
fi-skl-6260u: PASS -> FAIL +5
fi-skl-6700k2: PASS -> FAIL +5
fi-skl-gvtdvm: PASS -> FAIL +5
{fi-skl-iommu}: PASS -> FAIL +5
fi-skl-6770hq: PASS -> FAIL +5
fi-bxt-j4205: PASS -> FAIL +5
fi-kbl-7560u: PASS -> FAIL +5
fi-whl-u: PASS -> FAIL +5
fi-kbl-r: PASS -> FAIL +5
== Known issues ==
Here are the changes found in Patchwork_9932 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
fi-skl-6600u: PASS -> DMESG-FAIL (fdo#106560, fdo#107174)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
fi-cfl-s3: PASS -> FAIL (fdo#103481) +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
{fi-bsw-kefka}: PASS -> FAIL (fdo#106211) +3
{fi-cfl-8109u}: PASS -> FAIL (fdo#103481) +5
fi-cfl-guc: PASS -> FAIL (fdo#103481) +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-cnl-psr: PASS -> FAIL (fdo#106211) +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
fi-glk-j4005: PASS -> FAIL (fdo#106211) +5
fi-cfl-8700k: PASS -> FAIL (fdo#103481) +5
fi-bsw-n3050: PASS -> FAIL (fdo#106211) +1
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
fi-glk-dsi: PASS -> FAIL (fdo#106211) +5
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#106685, fdo#107174) -> PASS
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS
igt@kms_chamelium@dp-crc-fast:
fi-kbl-7500u: DMESG-FAIL (fdo#103841) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#106211 https://bugs.freedesktop.org/show_bug.cgi?id=106211
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
fdo#107139
[Intel-gfx] [PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback
This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.
Changes Since V1:
- update values_cnt in verify_crc_source
Cc: Haneen Mohammed
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/vkms/vkms_crc.c | 38 ++
drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..b2a484b4e2ad 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, );
}
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+ int ret = 0;
+
+ if (!src_name) {
+ *enabled = false;
+ } else if (strcmp(src_name, "auto") == 0) {
+ *enabled = true;
+ } else {
+ *enabled = false;
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+ size_t *values_cnt)
+{
+ bool enabled;
+
+ if (vkms_crc_parse_source(src_name, ) < 0) {
+ DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+ return -EINVAL;
+ }
+
+ *values_cnt = 1;
+
+ return 0;
+}
+
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt)
{
@@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
*src_name,
unsigned long flags;
int ret = 0;
- if (src_name && strcmp(src_name, "auto") == 0)
- enabled = true;
- else if (src_name)
- ret = -EINVAL;
+ ret = vkms_crc_parse_source(src_name, );
+ if (ret)
+ return ret;
*values_cnt = 1;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.enable_vblank = vkms_enable_vblank,
.disable_vblank = vkms_disable_vblank,
.set_crc_source = vkms_set_crc_source,
+ .verify_crc_source = vkms_verify_crc_source,
};
static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
/* CRC Support */
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+ size_t *values_cnt);
void vkms_crc_work_handle(struct work_struct *work);
#endif /* _VKMS_DRV_H_ */
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xserver] randr: fix RRCrtcDetachScanoutPixmap crash on server exit
The following crash was observed with xserver 1.20.1 on exiting xserver
after enabling a PRIME output source with the Intel driver:
Old value = (WindowPtr) 0x612000159dc0
New value = (WindowPtr) 0x0 // pWin->drawable.pScreen->root = NULL;
DeleteWindow (value=0x612000159dc0, wid=) at
dix/window.c:1112
1112dixFreeObjectWithPrivates(pWin, PRIVATE_WINDOW);
(gdb) bt
#0 DeleteWindow (value=0x612000159dc0, wid=) at
dix/window.c:1112
#1 0x557e7842535b in doFreeResource (res=0x6030ebf0,
skip=) at dix/resource.c:880
#2 0x557e784289ed in FreeClientResources (client=0x60e00040) at
dix/resource.c:1146
#3 0x557e78428c46 in FreeAllResources () at dix/resource.c:1161
#4 0x557e783c25d8 in dix_main (argc=, argv=, envp=) at dix/main.c:292
...
Thread 1 "Xorg" received signal SIGSEGV, Segmentation fault.
0x557e7841138c in PixmapStopDirtyTracking (src=0x0,
slave_dst=0x6112ea80) at dix/pixmap.c:251
251 ScreenPtr screen = src->pScreen;
(gdb) bt
#0 0x558e598a938c in PixmapStopDirtyTracking (src=0x0,
slave_dst=0x61138d00) at ../xserver/dix/pixmap.c:251
#1 0x558e5990ccd5 in RRCrtcDetachScanoutPixmap (crtc=0x61704680)
at ../xserver/randr/rrcrtc.c:413
#2 0x558e5990d001 in RRCrtcDestroyResource (value=0x61704680,
pid=) at ../xserver/randr/rrcrtc.c:900
#3 0x558e598bd35b in doFreeResource (res=0x6030a2a0,
skip=) at ../xserver/dix/resource.c:880
#4 0x558e598c09ed in FreeClientResources (client=0x60e00040) at
../xserver/dix/resource.c:1146
#5 0x558e598c0c46 in FreeAllResources () at
../xserver/dix/resource.c:1161
#6 0x558e5985a5d8 in dix_main (argc=, argv=, envp=) at ../xserver/dix/main.c:292
For some reason, the Window resource ends up being freed before a pixmap
when using the intel driver. It does not occur with modesetting (there
RRCrtcDestroyResource is called before deleting the root window).
Before "Make PixmapDirtyUpdateRec::src a DrawablePtr" the "src" argument
was "master->GetScreenPixmap(master)". After that commit, it becomes the
root window drawable which can be NULL as shown above.
Signed-off-by: Peter Wu
---
randr/rrcrtc.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d9026266..d5dc235b7 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -398,20 +398,22 @@ RRCrtcDetachScanoutPixmap(RRCrtcPtr crtc)
ScreenPtr master = crtc->pScreen->current_master;
DrawablePtr mrootdraw = >root->drawable;
-if (crtc->scanout_pixmap_back) {
-pScrPriv->rrDisableSharedPixmapFlipping(crtc);
+if (mrootdraw) {
+if (crtc->scanout_pixmap_back) {
+pScrPriv->rrDisableSharedPixmapFlipping(crtc);
-master->StopFlippingPixmapTracking(mrootdraw,
- crtc->scanout_pixmap,
- crtc->scanout_pixmap_back);
+master->StopFlippingPixmapTracking(mrootdraw,
+ crtc->scanout_pixmap,
+ crtc->scanout_pixmap_back);
-rrDestroySharedPixmap(crtc, crtc->scanout_pixmap_back);
-crtc->scanout_pixmap_back = NULL;
-}
-else {
-pScrPriv->rrCrtcSetScanoutPixmap(crtc, NULL);
-master->StopPixmapTracking(mrootdraw,
- crtc->scanout_pixmap);
+rrDestroySharedPixmap(crtc, crtc->scanout_pixmap_back);
+crtc->scanout_pixmap_back = NULL;
+}
+else {
+pScrPriv->rrCrtcSetScanoutPixmap(crtc, NULL);
+master->StopPixmapTracking(mrootdraw,
+ crtc->scanout_pixmap);
+}
}
rrDestroySharedPixmap(crtc, crtc->scanout_pixmap);
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel] SNA: fix PRIME output support since xserver 1.20
Since xorg-server 1.20, an external monitor would remain blank when used
in a PRIME output slave setup. Only a cursor was visible. The cause is
"Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the "src"
pointer might point to the root window (created by the server) instead
of a pixmap (as created by xf86-video-intel). Use get_drawable_pixmap to
handle both cases.
When built with -fsanitize=address, the following test will trigger a
heap-buffer-overflow error due to to_sna_from_pixmap receiving a window
instead of a pixmap.
Test on a hybrid graphics laptop (Intel + modesetting/nouveau):
xrandr --setprovideroutputsource modesetting Intel
xrandr --output DP-1-1 --mode 2560x1440 # should not crash
glxgears # should display gears on both screens
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
Signed-off-by: Peter Wu
---
Tested with xserver 1.20.1 with ASAN enabled. Survives multiple
resolution changes, works with a Plasma desktop session, it seems
stable. Something like this patch is required to make multi-monitor
setups usable in a hybrid graphics setting with Xorg 1.20.
---
src/sna/sna_accel.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 2f669bcf..80b116a3 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -17510,7 +17510,11 @@ static bool has_offload_slaves(struct sna *sna)
PixmapDirtyUpdatePtr dirty;
xorg_list_for_each_entry(dirty, >pixmap_dirty_list, ent) {
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ assert(dirty->src == >front->drawable);
+#else
assert(dirty->src == sna->front);
+#endif
if (RegionNotEmpty(DamageRegion(dirty->damage)))
return true;
}
@@ -17671,7 +17675,11 @@ static void sna_accel_post_damage(struct sna *sna)
if (RegionNil(damage))
continue;
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ src = get_drawable_pixmap(dirty->src);
+#else
src = dirty->src;
+#endif
dst = dirty->slave_dst->master_pixmap;
region.extents.x1 = dirty->x;
@@ -17922,9 +17930,15 @@ migrate_dirty_tracking(PixmapPtr old_front, PixmapPtr
new_front)
PixmapDirtyUpdatePtr dirty, safe;
xorg_list_for_each_entry_safe(dirty, safe, >pixmap_dirty_list,
ent) {
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ assert(dirty->src == _front->drawable);
+ if (dirty->src != _front->drawable)
+ continue;
+#else
assert(dirty->src == old_front);
if (dirty->src != old_front)
continue;
+#endif
DamageUnregister(>src->drawable, dirty->damage);
DamageDestroy(dirty->damage);
@@ -17939,7 +17953,11 @@ migrate_dirty_tracking(PixmapPtr old_front, PixmapPtr
new_front)
}
DamageRegister(_front->drawable, dirty->damage);
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ dirty->src = _front->drawable;
+#else
dirty->src = new_front;
+#endif
}
#endif
}
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/icl: Get DDI clock for ICL for MG PLL and TBT PLL
On Fri, 2018-07-27 at 13:04 -0700, Paulo Zanoni wrote:
> From: Manasi Navare
>
> PLLs are the source clocks for the DDIs so in order to determine the
> ddi clock we need to check the PLL configuration.
>
> For MG PHy Ports (C - F), depending on whether it is a TBT PLL or MG
> PLL the link lock can be obtained from the the PLL divisors based on
> the specification.
>
> v2 (from Paulo):
> * Make the algorithm look more like what's in the spec, also
> document
>where we differ form the spec and why.
> * Make the code a little more consistent with our coding style.
>
> Cc: James Ausmus
> Signed-off-by: Manasi Navare
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 ++
> drivers/gpu/drm/i915/intel_ddi.c | 81
> +++-
> 2 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index e04ac47d53db..cd37e76d4d19 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9458,6 +9458,7 @@ enum skl_power_gate {
> #define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5 (2 << 12)
> #define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7 (3 << 12)
> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x) ((x) << 8)
> +#define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT 8
> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK (0xf <<
> 8)
> #define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \
>_MG_CLKTOP2_HSCLKCTL_PORT1
> , \
> @@ -9468,7 +9469,10 @@ enum skl_power_gate {
> #define _MG_PLL_DIV0_PORT3 0x16AA00
> #define _MG_PLL_DIV0_PORT4 0x16BA00
> #define MG_PLL_DIV0_FRACNEN_H (1 <<
> 30)
> +#define MG_PLL_DIV0_FBDIV_FRAC_MASK(0x3fff
> ff << 8)
> +#define MG_PLL_DIV0_FBDIV_FRAC_SHIFT 8
> #define MG_PLL_DIV0_FBDIV_FRAC(x) ((x) << 8)
> +#define MG_PLL_DIV0_FBDIV_INT_MASK (0xff << 0)
> #define MG_PLL_DIV0_FBDIV_INT(x) ((x) << 0)
> #define MG_PLL_DIV0(port) _MMIO_PORT((port) - PORT_C,
> _MG_PLL_DIV0_PORT1, \
>_MG_PLL_DIV0_PORT2)
> @@ -9483,6 +9487,7 @@ enum skl_power_gate {
> #define MG_PLL_DIV1_DITHER_DIV_4 (2 << 12)
> #define MG_PLL_DIV1_DITHER_DIV_8 (3 << 12)
> #define MG_PLL_DIV1_NDIVRATIO(x) ((x) << 4)
> +#define MG_PLL_DIV1_FBPREDIV_MASK (0xf << 0)
> #define MG_PLL_DIV1_FBPREDIV(x)((x) << 0)
> #define MG_PLL_DIV1(port) _MMIO_PORT((port) - PORT_C,
> _MG_PLL_DIV1_PORT1, \
>_MG_PLL_DIV1_PORT2)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0adc043529f2..a18d57046bcc 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1427,6 +1427,81 @@ static int cnl_calc_wrpll_link(struct
> drm_i915_private *dev_priv,
> return dco_freq / (p0 * p1 * p2 * 5);
> }
>
> +static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv,
> + enum port port)
> +{
> + u32 val = I915_READ(DDI_CLK_SEL(port)) & DDI_CLK_SEL_MASK;
> +
> + switch (val) {
> + case DDI_CLK_SEL_NONE:
> + return 0;
> + case DDI_CLK_SEL_TBT_162:
> + return 162000;
> + case DDI_CLK_SEL_TBT_270:
> + return 27;
> + case DDI_CLK_SEL_TBT_540:
> + return 54;
> + case DDI_CLK_SEL_TBT_810:
> + return 81;
> + default:
> + MISSING_CASE(val);
> + return 0;
> + }
> +}
> +
> +static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv,
> + enum port port)
> +{
> + u32 mg_pll_div0, mg_clktop_hsclkctl;
> + u32 m1, m2_int, m2_frac, div1, div2, refclk;
> + u64 tmp;
> +
> + refclk = dev_priv->cdclk.hw.ref;
> +
> + mg_pll_div0 = I915_READ(MG_PLL_DIV0(port));
> + mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(port));
> +
> + m1 = I915_READ(MG_PLL_DIV1(port)) & MG_PLL_DIV1_FBPREDIV_MASK;
> + m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
> + m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ?
> + (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
> + MG_PLL_DIV0_FBDIV_FRAC_SHIFT : 0;
> +
> + switch (mg_clktop_hsclkctl &
> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
> + case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2:
> + div1 = 2;
> + break;
> + case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_3:
> + div1 = 3;
> + break;
> + case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5:
> + div1 = 5;
> + break;
> + case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7:
> + div1 = 7;
>
Re: [Intel-gfx] [PATCH 1/2] drm/i915/icl: Implement HSDIV_RATIO of MG_CLKTOP2_HSCLKCTL_PORT reg as separate divider value defines
On Fri, 2018-07-27 at 13:04 -0700, Paulo Zanoni wrote:
> From: Manasi Navare
>
> The register value of Divider Ratio for high speed divider
> (hsdiv_ratio) in MG_CLKTOP2_HSCLKCTL_PORT register is not same as the
> actual numerical value of the divider. So this patch implements
> separate divider value defines for that field.
> icl_mg_pll_find_divisors() can use these defines instead of magic
> register values.
>
> The new defines are going to be used in the next patch.
>
> v2 (from Paulo):
> * Rebase.
> * Make it look a little more like the rest of our code.
>
> Cc: James Ausmus
> Suggested-by: James Ausmus
> Signed-off-by: Manasi Navare
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 -
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 10 +-
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 5530c470f30d..e04ac47d53db 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9452,8 +9452,11 @@ enum skl_power_gate {
> #define MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK (0x1 << 16)
> #define MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL(x) ((x) << 14)
> #define MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK (0x3 << 14)
> -#define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO(x) ((x) << 12)
> #define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK (0x3 <<
> 12)
> +#define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2 (0 << 12)
> +#define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_3 (1 << 12)
> +#define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5 (2 << 12)
> +#define MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7 (3 << 12)
> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x) ((x) << 8)
> #define MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK (0xf <<
> 8)
> #define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 7e5e6eb5dfe2..300c374fc721 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2662,16 +2662,16 @@ static bool icl_mg_pll_find_divisors(int
> clock_khz, bool is_dp, bool use_ssc,
> MISSING_CASE(div1);
> /* fall through */
> case 2:
> - hsdiv = 0;
> + hsdiv =
> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2;
> break;
> case 3:
> - hsdiv = 1;
> + hsdiv =
> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_3;
> break;
> case 5:
> - hsdiv = 2;
> + hsdiv =
> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5;
> break;
> case 7:
> - hsdiv = 3;
> + hsdiv =
> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7;
I hsdiv should have its type changed to u32 as it is now a bit field
not a divider number, with that:
Reviewed-by: José Roberto de Souza
> break;
> }
>
> @@ -2685,7 +2685,7 @@ static bool icl_mg_pll_find_divisors(int
> clock_khz, bool is_dp, bool use_ssc,
> state->mg_clktop2_hsclkctl =
> MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL(tli
> nedrv) |
> MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL(input
> sel) |
> - MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO(hsdiv)
> |
> + hsdiv |
> MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(div2);
>
> return true;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Add pre_pll_enable hook for ICL and set DFLEXDPMLE in this hook
On Mon, Aug 13, 2018 at 12:38:00PM -0700, Souza, Jose wrote:
> On Tue, 2018-07-31 at 17:30 -0700, Manasi Navare wrote:
> > In case of Legacy DP connector on TypeC port (C, D, E or F), the
>
> Legacy DP connector(DisplayPort Alternative Mode)
Ok will reword this as DisplayPort Alternative Mode
>
> > flex IO DPMLE register is set to maximum number of lanes since
> > there is no muxing with other controllers in this case.
>
> Okay there is no muxing but driver could choose to use few lanes do
> save power.
>
> > While in case of the TypeC connector, it is set to the lane count
> > obained from DFLEXDPSP register.
>
> obtained
>
> > This needs to be programmed before enabling the shared PLLs hence
> > add a pre_pll_enable hook for ICL and add this programming in that
> > hook.
> >
> > v3:
> > * Call intel_dp_max_common_lane_count that gets lane count
> > common between sink, source, fia
> > v2:
> > * Add pre pll enable hook and move dflexdpmle programming
> > to that hook (Animesh)
> >
> > Cc: Jani Nikula
> > Cc: Animesh Manna
> > Cc: Anusha Srivatsa
> > Cc: Paulo Zanoni
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 18 ++
> > drivers/gpu/drm/i915/intel_dp.c | 20
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > 3 files changed, 39 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0adc043..eb00ac4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3193,6 +3193,22 @@ static void bxt_ddi_pre_pll_enable(struct
> > intel_encoder *encoder,
> > bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> > }
> >
> > +static void icl_ddi_pre_pll_enable(struct intel_encoder *encoder,
> > + const struct intel_crtc_state
> > *pipe_config,
> > + const struct drm_connector_state
> > *conn_state)
> > +{
> > + struct intel_dp *intel_dp = enc_to_intel_dp(>base);
> > + struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> > + enum tc_port tc_port = intel_port_to_tc(dev_priv,
> > + intel_dig_port-
> > >base.port);
> > +
> > + if (tc_port == PORT_TC_NONE || intel_dig_port->tc_type ==
> > TC_PORT_TBT)
> > + return;
> > +
> > + intel_dp_set_fia_lane_count(intel_dp);
> > +}
> > +
> > void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> > {
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > @@ -3709,6 +3725,8 @@ void intel_ddi_init(struct drm_i915_private
> > *dev_priv, enum port port)
> > intel_encoder->enable = intel_enable_ddi;
> > if (IS_GEN9_LP(dev_priv))
> > intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> > + if (IS_ICELAKE(dev_priv))
> > + intel_encoder->pre_pll_enable = icl_ddi_pre_pll_enable;
>
> pre_pll_enable is set initialy in intel_crt_init() for most of the
> gens, maybe those 4 lines should be moved there but that can be done in
> another patch.
I think the crt_init() only has thecrt specific functions for DDI platforms
like hsw_crt_compute_config() hsw_enable_crt() etc, all the other ddi function
pointers
are assigned in intel_ddi_init()
>
> > intel_encoder->pre_enable = intel_ddi_pre_enable;
> > intel_encoder->disable = intel_disable_ddi;
> > intel_encoder->post_disable = intel_ddi_post_disable;
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 49a3149..6683cdc 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2127,6 +2127,26 @@ void intel_dp_set_link_params(struct intel_dp
> > *intel_dp,
> > intel_dp->link_mst = link_mst;
> > }
> >
> > +void intel_dp_set_fia_lane_count(struct intel_dp *intel_dp)
> > +{
> > + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > + enum tc_port tc_port = intel_port_to_tc(dev_priv,
> > + intel_dig_port-
> > >base.port);
> > + u8 lane_count = intel_dp_max_common_lane_count(intel_dp);
>
> intel_dp->max_link_lane_count could be used instead as it is set on
> hotplug and don't change over time.
>
> But I guess this is also wrong, it should not enable the max lane
> count, it should enable the number of the lanes that the driver decides
> that is enough to the mode set to save power.
So I should be using crtc_state->lane_count here because thats what is the
optimum
lane count computed in intel_dp_compute_config() for legacy DP connector.
In case of Type-C connector also, DFLEXDPSP only has the max lane count but the
enable sequence
says:
"Dynamic - Program the
Re: [Intel-gfx] [PATCH] drm/i915/icl: Add pre_pll_enable hook for ICL and set DFLEXDPMLE in this hook
On Tue, 2018-07-31 at 17:30 -0700, Manasi Navare wrote:
> In case of Legacy DP connector on TypeC port (C, D, E or F), the
Legacy DP connector(DisplayPort Alternative Mode)
> flex IO DPMLE register is set to maximum number of lanes since
> there is no muxing with other controllers in this case.
Okay there is no muxing but driver could choose to use few lanes do
save power.
> While in case of the TypeC connector, it is set to the lane count
> obained from DFLEXDPSP register.
obtained
> This needs to be programmed before enabling the shared PLLs hence
> add a pre_pll_enable hook for ICL and add this programming in that
> hook.
>
> v3:
> * Call intel_dp_max_common_lane_count that gets lane count
> common between sink, source, fia
> v2:
> * Add pre pll enable hook and move dflexdpmle programming
> to that hook (Animesh)
>
> Cc: Jani Nikula
> Cc: Animesh Manna
> Cc: Anusha Srivatsa
> Cc: Paulo Zanoni
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 18 ++
> drivers/gpu/drm/i915/intel_dp.c | 20
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 3 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0adc043..eb00ac4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3193,6 +3193,22 @@ static void bxt_ddi_pre_pll_enable(struct
> intel_encoder *encoder,
> bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> }
>
> +static void icl_ddi_pre_pll_enable(struct intel_encoder *encoder,
> +const struct intel_crtc_state
> *pipe_config,
> +const struct drm_connector_state
> *conn_state)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(>base);
> + struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> >base.base.dev);
> + enum tc_port tc_port = intel_port_to_tc(dev_priv,
> + intel_dig_port-
> >base.port);
> +
> + if (tc_port == PORT_TC_NONE || intel_dig_port->tc_type ==
> TC_PORT_TBT)
> + return;
> +
> + intel_dp_set_fia_lane_count(intel_dp);
> +}
> +
> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> @@ -3709,6 +3725,8 @@ void intel_ddi_init(struct drm_i915_private
> *dev_priv, enum port port)
> intel_encoder->enable = intel_enable_ddi;
> if (IS_GEN9_LP(dev_priv))
> intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> + if (IS_ICELAKE(dev_priv))
> + intel_encoder->pre_pll_enable = icl_ddi_pre_pll_enable;
pre_pll_enable is set initialy in intel_crt_init() for most of the
gens, maybe those 4 lines should be moved there but that can be done in
another patch.
> intel_encoder->pre_enable = intel_ddi_pre_enable;
> intel_encoder->disable = intel_disable_ddi;
> intel_encoder->post_disable = intel_ddi_post_disable;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 49a3149..6683cdc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2127,6 +2127,26 @@ void intel_dp_set_link_params(struct intel_dp
> *intel_dp,
> intel_dp->link_mst = link_mst;
> }
>
> +void intel_dp_set_fia_lane_count(struct intel_dp *intel_dp)
> +{
> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> + enum tc_port tc_port = intel_port_to_tc(dev_priv,
> + intel_dig_port-
> >base.port);
> + u8 lane_count = intel_dp_max_common_lane_count(intel_dp);
intel_dp->max_link_lane_count could be used instead as it is set on
hotplug and don't change over time.
But I guess this is also wrong, it should not enable the max lane
count, it should enable the number of the lanes that the driver decides
that is enough to the mode set to save power.
> + u32 val;
> +
> + /* In case of legacy/static DP over Type-C port there is no
> muxing
> + * with other controllers so this is set to max lane count.
> + * In case of Type_C it is set to the DFLEXDPSP.DPX4TXLATC
> value.
> + */
> + val = I915_READ(PORT_TX_DFLEXDPMLE1);
> + val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> + val |= DFLEXDPMLE1_DPMLETC(tc_port, lane_count);
this macro is wrong, each bit set maps to a lane, so to enable all 4
lanes it should 0xf for the first TC.
From spec:
For example, in DP Pin Assignment C, the register
DFLEXDPSP1.DPX4TXLATC0 tells Display Driver that all the 4 TX Lane in
PHY can be used. However, Display Driver may choose to use only x1,
i.e. for ML0. Then Display Driver will program “0001b” to this
Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Remove wait_for_idle() for PSR2
On Mon, 2018-08-13 at 11:17 -0700, Tarun Vyas wrote:
> On Mon, Aug 13, 2018 at 11:10:00AM -0700, Pandiyan, Dhinakaran wrote:
> > On Mon, 2018-08-13 at 09:57 -0700, Rodrigo Vivi wrote:
> > > On Thu, Aug 09, 2018 at 05:41:35PM -0700, Dhinakaran Pandiyan
> > > wrote:
> > > > CI runs show PSR2 does not go to IDLE with selective update
> > > > enabled
> > > > on
> > > > all PSR exit triggers. Specifically, logs indicate the hardware
> > > > enters
> > > > "SLEEP Selective Update" and not "IDLE Reset state' like the
> > > > kernel
> > > > expects. This check was added for PSR1 but incorrectly extended
> > > > to
> > > > PSR2,
> > > > remove this check for PSR2 as there is a plan to test only PSR1
> > > > on
> > > > PSR2
> > > > panels.
> > > >
> > > > Also add bspec reference to the comment about idle timeout.
> > > >
> > > > Cc: Tarun Vyas
> > > > Cc: José Roberto de Souza
> > > > Cc: Rodrigo Vivi
> > > > Signed-off-by: Dhinakaran Pandiyan > > > om>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_psr.c | 39 --
> > > >
> > > > --
> > > > 1 file changed, 14 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 5686ddaa6a72..09be9bfee2be 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -722,37 +722,26 @@ int intel_psr_wait_for_idle(const struct
> > > > intel_crtc_state *new_crtc_state,
> > > > {
> > > > struct intel_crtc *crtc =
> > > > to_intel_crtc(new_crtc_state-
> > > > > base.crtc);
> > > >
> > > > struct drm_i915_private *dev_priv = to_i915(crtc-
> > > > > base.dev);
> > > >
> > > > - i915_reg_t reg;
> > > > - u32 mask;
> > > > -
> > > > - if (!new_crtc_state->has_psr)
> > > > - return 0;
> > > >
> > > > /*
> > > > -* The sole user right now is
> > > > intel_pipe_update_start(),
> > > > -* which won't race with psr_enable/disable, which is
> > > > -* where psr2_enabled is written to. So, we don't need
> > > > -* to acquire the psr.lock. More importantly, we want
> > > > the
> > > > -* latency inside intel_pipe_update_start() to be as
> > > > low
> > > > -* as possible, so no need to acquire psr.lock when it
> > > > is
> > > > -* not needed and will induce latencies in the atomic
> > > > -* update path.
> > > > +* The sole user right now is
> > > > intel_pipe_update_start(),
> > > > which won't
> > > > +* race with psr_enable/disable where psr2_enabled is
> > > > written to. So, we
> > > > +* don't need to acquire the psr.lock. More
> > > > importantly,
> > > > we want the
> > > > +* latency inside intel_pipe_update_start() to be as
> > > > low
> > > > as possible, so
> > > > +* no need to acquire psr.lock when it is not needed
> > > > and
> > > > will induce
> > > > +* latencies in the atomic update path.
> > > > */
> > >
> > > I think we shouldn't change this format here to keep patch
> > > cleaner...
> > > if there is any change here I couldn't see because it is changing
> > > all
> > > lines and if there is no change I think it is better not to touch
> > > because
> > > it removes the focus of the real changes.
> >
> > Okay.
> > >
> > > > - if (dev_priv->psr.psr2_enabled) {
> > > > - reg = EDP_PSR2_STATUS;
> > > > - mask = EDP_PSR2_STATUS_STATE_MASK;
> > > > - } else {
> > > > - reg = EDP_PSR_STATUS;
> > > > - mask = EDP_PSR_STATUS_STATE_MASK;
> > > > - }
> > > > + if (!new_crtc_state->has_psr || READ_ONCE(dev_priv-
> > > > > psr.psr2_enabled))
> > >
> > > I now see that we are removing psr2 of the picture, but I don't
> > > see
> > > how we are
> > > improving psr2 situation here.
> > > what am I missing?
> > >
> >
> > When the patch was written, we did not have sufficient tests to
> > tell us
> > the wait_for_idle condition was wrong for PSR2. It was not known
> > whether the wait was *necessary* for PSR2, think of this as a
> > partial
> > revert. Now that CI has pointed out, (and I checked with a PSR2
> > panel)
> > that the condition is wrong, we should be removing it for PSR2. If
> > you
> > think about it, it does improve PSR2 my removing irrelevant code.
>
> Do we have another way to ensure that we dont try to do a pipe update
> or rather
> check for the PIPE DSL when still in a PSR2 sleep state ?
I don't know, the PSR2 source states aren't documented well enough for
us to implement this change. The current check is clearly wrong, I
think we should remove and fix it correctly when we know it is needed.
> >
> >
> > > > + return 0;
> > > >
> > > > /*
> > > > -* Max time for PSR to idle = Inverse of the refresh
> > > > rate
> > > > +
> > > > -* 6 ms of exit training time + 1.5 ms of aux channel
> > >
Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Remove wait_for_idle() for PSR2
On Mon, Aug 13, 2018 at 11:10:00AM -0700, Pandiyan, Dhinakaran wrote:
> On Mon, 2018-08-13 at 09:57 -0700, Rodrigo Vivi wrote:
> > On Thu, Aug 09, 2018 at 05:41:35PM -0700, Dhinakaran Pandiyan wrote:
> > > CI runs show PSR2 does not go to IDLE with selective update enabled
> > > on
> > > all PSR exit triggers. Specifically, logs indicate the hardware
> > > enters
> > > "SLEEP Selective Update" and not "IDLE Reset state' like the kernel
> > > expects. This check was added for PSR1 but incorrectly extended to
> > > PSR2,
> > > remove this check for PSR2 as there is a plan to test only PSR1 on
> > > PSR2
> > > panels.
> > >
> > > Also add bspec reference to the comment about idle timeout.
> > >
> > > Cc: Tarun Vyas
> > > Cc: José Roberto de Souza
> > > Cc: Rodrigo Vivi
> > > Signed-off-by: Dhinakaran Pandiyan
> > > ---
> > > drivers/gpu/drm/i915/intel_psr.c | 39 --
> > > --
> > > 1 file changed, 14 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 5686ddaa6a72..09be9bfee2be 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -722,37 +722,26 @@ int intel_psr_wait_for_idle(const struct
> > > intel_crtc_state *new_crtc_state,
> > > {
> > > struct intel_crtc *crtc = to_intel_crtc(new_crtc_state-
> > > >base.crtc);
> > > struct drm_i915_private *dev_priv = to_i915(crtc-
> > > >base.dev);
> > > - i915_reg_t reg;
> > > - u32 mask;
> > > -
> > > - if (!new_crtc_state->has_psr)
> > > - return 0;
> > >
> > > /*
> > > - * The sole user right now is intel_pipe_update_start(),
> > > - * which won't race with psr_enable/disable, which is
> > > - * where psr2_enabled is written to. So, we don't need
> > > - * to acquire the psr.lock. More importantly, we want the
> > > - * latency inside intel_pipe_update_start() to be as low
> > > - * as possible, so no need to acquire psr.lock when it is
> > > - * not needed and will induce latencies in the atomic
> > > - * update path.
> > > + * The sole user right now is intel_pipe_update_start(),
> > > which won't
> > > + * race with psr_enable/disable where psr2_enabled is
> > > written to. So, we
> > > + * don't need to acquire the psr.lock. More importantly,
> > > we want the
> > > + * latency inside intel_pipe_update_start() to be as low
> > > as possible, so
> > > + * no need to acquire psr.lock when it is not needed and
> > > will induce
> > > + * latencies in the atomic update path.
> > >*/
> >
> > I think we shouldn't change this format here to keep patch cleaner...
> > if there is any change here I couldn't see because it is changing all
> > lines and if there is no change I think it is better not to touch
> > because
> > it removes the focus of the real changes.
>
> Okay.
> >
> > > - if (dev_priv->psr.psr2_enabled) {
> > > - reg = EDP_PSR2_STATUS;
> > > - mask = EDP_PSR2_STATUS_STATE_MASK;
> > > - } else {
> > > - reg = EDP_PSR_STATUS;
> > > - mask = EDP_PSR_STATUS_STATE_MASK;
> > > - }
> > > + if (!new_crtc_state->has_psr || READ_ONCE(dev_priv-
> > > >psr.psr2_enabled))
> >
> > I now see that we are removing psr2 of the picture, but I don't see
> > how we are
> > improving psr2 situation here.
> > what am I missing?
> >
> When the patch was written, we did not have sufficient tests to tell us
> the wait_for_idle condition was wrong for PSR2. It was not known
> whether the wait was *necessary* for PSR2, think of this as a partial
> revert. Now that CI has pointed out, (and I checked with a PSR2 panel)
> that the condition is wrong, we should be removing it for PSR2. If you
> think about it, it does improve PSR2 my removing irrelevant code.
Do we have another way to ensure that we dont try to do a pipe update or rather
check for the PIPE DSL when still in a PSR2 sleep state ?
>
>
> > > + return 0;
> > >
> > > /*
> > > - * Max time for PSR to idle = Inverse of the refresh rate
> > > +
> > > - * 6 ms of exit training time + 1.5 ms of aux channel
> > > - * handshake. 50 msec is defesive enough to cover
> > > everything.
> > > + * From Bspec Panel Self Refresh (BDW+):
> >
> > This is another case, if we didn't change the format only this line ^
> > would be in the patch and it would be cleaner and easier to review
> > the
> > changes.
> >
> > but my biggest concern with this patch is how do we check now
> > wait_psr2 idle
> >
> > > + * Max. time for PSR to idle = inverse of the refresh rate
> > > + 6 ms of
> > > + * exit training time + 1.5 ms of aux channel handshake.
> > > 50 ms is
> > > + * defensive enough to cover everything.
> > >*/
> > > -
> > > - return __intel_wait_for_register(dev_priv, reg, mask,
> > > + return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
> > > + EDP_PSR_STATUS_STATE_MASK
> > > ,
> > >
Re: [Intel-gfx] [PATCH] drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
On Mon, 2018-08-13 at 09:47 -0700, Rodrigo Vivi wrote:
> On Mon, Aug 13, 2018 at 03:47:20PM +0200, Maarten Lankhorst wrote:
> > Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> > > We print the last attempted entry and last exit timestamps only
> > > when
> > > IRQ debug is requested. This check was missed when new debug
> > > flags were
> > > added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> > > runtime through debugfs, v6")
> > >
> > > Cc: Maarten Lankhorst
> > > Signed-off-by: Dhinakaran Pandiyan > > >
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 26b7e5276b15..374b550d9a4f 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct
> > > seq_file *m, void *data)
> > > psr_source_status(dev_priv, m);
> > > mutex_unlock(_priv->psr.lock);
> > >
> > > - if (READ_ONCE(dev_priv->psr.debug)) {
> > > + if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ)
> > > {
> > > seq_printf(m, "Last attempted entry at: %lld\n",
> > > dev_priv->psr.last_entry_attempt);
> > > seq_printf(m, "Last exit at: %lld\n",
> >
> > Oops indeed.
> >
> > Reviewed-by: Maarten Lankhorst
>
> before pushing to dinq please check the compilation there..
> kbuild bot raised an issue...
>
> so apparently we will need a backmerge before pushing this...
The failures are on
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.18-rc8 next-20180810]
Is a back-merge expected to fix that? and who does that back-merge?
-DK
>
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Remove wait_for_idle() for PSR2
On Mon, 2018-08-13 at 09:57 -0700, Rodrigo Vivi wrote:
> On Thu, Aug 09, 2018 at 05:41:35PM -0700, Dhinakaran Pandiyan wrote:
> > CI runs show PSR2 does not go to IDLE with selective update enabled
> > on
> > all PSR exit triggers. Specifically, logs indicate the hardware
> > enters
> > "SLEEP Selective Update" and not "IDLE Reset state' like the kernel
> > expects. This check was added for PSR1 but incorrectly extended to
> > PSR2,
> > remove this check for PSR2 as there is a plan to test only PSR1 on
> > PSR2
> > panels.
> >
> > Also add bspec reference to the comment about idle timeout.
> >
> > Cc: Tarun Vyas
> > Cc: José Roberto de Souza
> > Cc: Rodrigo Vivi
> > Signed-off-by: Dhinakaran Pandiyan
> > ---
> > drivers/gpu/drm/i915/intel_psr.c | 39 --
> > --
> > 1 file changed, 14 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 5686ddaa6a72..09be9bfee2be 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -722,37 +722,26 @@ int intel_psr_wait_for_idle(const struct
> > intel_crtc_state *new_crtc_state,
> > {
> > struct intel_crtc *crtc = to_intel_crtc(new_crtc_state-
> > >base.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc-
> > >base.dev);
> > - i915_reg_t reg;
> > - u32 mask;
> > -
> > - if (!new_crtc_state->has_psr)
> > - return 0;
> >
> > /*
> > -* The sole user right now is intel_pipe_update_start(),
> > -* which won't race with psr_enable/disable, which is
> > -* where psr2_enabled is written to. So, we don't need
> > -* to acquire the psr.lock. More importantly, we want the
> > -* latency inside intel_pipe_update_start() to be as low
> > -* as possible, so no need to acquire psr.lock when it is
> > -* not needed and will induce latencies in the atomic
> > -* update path.
> > +* The sole user right now is intel_pipe_update_start(),
> > which won't
> > +* race with psr_enable/disable where psr2_enabled is
> > written to. So, we
> > +* don't need to acquire the psr.lock. More importantly,
> > we want the
> > +* latency inside intel_pipe_update_start() to be as low
> > as possible, so
> > +* no need to acquire psr.lock when it is not needed and
> > will induce
> > +* latencies in the atomic update path.
> > */
>
> I think we shouldn't change this format here to keep patch cleaner...
> if there is any change here I couldn't see because it is changing all
> lines and if there is no change I think it is better not to touch
> because
> it removes the focus of the real changes.
Okay.
>
> > - if (dev_priv->psr.psr2_enabled) {
> > - reg = EDP_PSR2_STATUS;
> > - mask = EDP_PSR2_STATUS_STATE_MASK;
> > - } else {
> > - reg = EDP_PSR_STATUS;
> > - mask = EDP_PSR_STATUS_STATE_MASK;
> > - }
> > + if (!new_crtc_state->has_psr || READ_ONCE(dev_priv-
> > >psr.psr2_enabled))
>
> I now see that we are removing psr2 of the picture, but I don't see
> how we are
> improving psr2 situation here.
> what am I missing?
>
When the patch was written, we did not have sufficient tests to tell us
the wait_for_idle condition was wrong for PSR2. It was not known
whether the wait was *necessary* for PSR2, think of this as a partial
revert. Now that CI has pointed out, (and I checked with a PSR2 panel)
that the condition is wrong, we should be removing it for PSR2. If you
think about it, it does improve PSR2 my removing irrelevant code.
> > + return 0;
> >
> > /*
> > -* Max time for PSR to idle = Inverse of the refresh rate
> > +
> > -* 6 ms of exit training time + 1.5 ms of aux channel
> > -* handshake. 50 msec is defesive enough to cover
> > everything.
> > +* From Bspec Panel Self Refresh (BDW+):
>
> This is another case, if we didn't change the format only this line ^
> would be in the patch and it would be cleaner and easier to review
> the
> changes.
>
> but my biggest concern with this patch is how do we check now
> wait_psr2 idle
>
> > +* Max. time for PSR to idle = inverse of the refresh rate
> > + 6 ms of
> > +* exit training time + 1.5 ms of aux channel handshake.
> > 50 ms is
> > +* defensive enough to cover everything.
> > */
> > -
> > - return __intel_wait_for_register(dev_priv, reg, mask,
> > + return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
> > +EDP_PSR_STATUS_STATE_MASK
> > ,
> > EDP_PSR_STATUS_STATE_IDLE
> > , 2, 50,
> > out_value);
> > }
> > --
> > 2.17.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Remove wait_for_idle() for PSR2
On Thu, Aug 09, 2018 at 05:41:35PM -0700, Dhinakaran Pandiyan wrote:
> CI runs show PSR2 does not go to IDLE with selective update enabled on
> all PSR exit triggers. Specifically, logs indicate the hardware enters
> "SLEEP Selective Update" and not "IDLE Reset state' like the kernel
> expects. This check was added for PSR1 but incorrectly extended to PSR2,
> remove this check for PSR2 as there is a plan to test only PSR1 on PSR2
> panels.
>
> Also add bspec reference to the comment about idle timeout.
>
> Cc: Tarun Vyas
> Cc: José Roberto de Souza
> Cc: Rodrigo Vivi
> Signed-off-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/i915/intel_psr.c | 39
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 5686ddaa6a72..09be9bfee2be 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -722,37 +722,26 @@ int intel_psr_wait_for_idle(const struct
> intel_crtc_state *new_crtc_state,
> {
> struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - i915_reg_t reg;
> - u32 mask;
> -
> - if (!new_crtc_state->has_psr)
> - return 0;
>
> /*
> - * The sole user right now is intel_pipe_update_start(),
> - * which won't race with psr_enable/disable, which is
> - * where psr2_enabled is written to. So, we don't need
> - * to acquire the psr.lock. More importantly, we want the
> - * latency inside intel_pipe_update_start() to be as low
> - * as possible, so no need to acquire psr.lock when it is
> - * not needed and will induce latencies in the atomic
> - * update path.
> + * The sole user right now is intel_pipe_update_start(), which won't
> + * race with psr_enable/disable where psr2_enabled is written to. So, we
> + * don't need to acquire the psr.lock. More importantly, we want the
> + * latency inside intel_pipe_update_start() to be as low as possible, so
> + * no need to acquire psr.lock when it is not needed and will induce
> + * latencies in the atomic update path.
>*/
I think we shouldn't change this format here to keep patch cleaner...
if there is any change here I couldn't see because it is changing all
lines and if there is no change I think it is better not to touch because
it removes the focus of the real changes.
> - if (dev_priv->psr.psr2_enabled) {
> - reg = EDP_PSR2_STATUS;
> - mask = EDP_PSR2_STATUS_STATE_MASK;
> - } else {
> - reg = EDP_PSR_STATUS;
> - mask = EDP_PSR_STATUS_STATE_MASK;
> - }
> + if (!new_crtc_state->has_psr || READ_ONCE(dev_priv->psr.psr2_enabled))
I now see that we are removing psr2 of the picture, but I don't see how we are
improving psr2 situation here.
what am I missing?
> + return 0;
>
> /*
> - * Max time for PSR to idle = Inverse of the refresh rate +
> - * 6 ms of exit training time + 1.5 ms of aux channel
> - * handshake. 50 msec is defesive enough to cover everything.
> + * From Bspec Panel Self Refresh (BDW+):
This is another case, if we didn't change the format only this line ^
would be in the patch and it would be cleaner and easier to review the
changes.
but my biggest concern with this patch is how do we check now wait_psr2 idle
> + * Max. time for PSR to idle = inverse of the refresh rate + 6 ms of
> + * exit training time + 1.5 ms of aux channel handshake. 50 ms is
> + * defensive enough to cover everything.
>*/
> -
> - return __intel_wait_for_register(dev_priv, reg, mask,
> + return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
> + EDP_PSR_STATUS_STATE_MASK,
>EDP_PSR_STATUS_STATE_IDLE, 2, 50,
>out_value);
> }
> --
> 2.17.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
On Mon, Aug 13, 2018 at 03:47:20PM +0200, Maarten Lankhorst wrote:
> Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> > We print the last attempted entry and last exit timestamps only when
> > IRQ debug is requested. This check was missed when new debug flags were
> > added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> > runtime through debugfs, v6")
> >
> > Cc: Maarten Lankhorst
> > Signed-off-by: Dhinakaran Pandiyan
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 26b7e5276b15..374b550d9a4f 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct seq_file *m,
> > void *data)
> > psr_source_status(dev_priv, m);
> > mutex_unlock(_priv->psr.lock);
> >
> > - if (READ_ONCE(dev_priv->psr.debug)) {
> > + if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
> > seq_printf(m, "Last attempted entry at: %lld\n",
> >dev_priv->psr.last_entry_attempt);
> > seq_printf(m, "Last exit at: %lld\n",
>
> Oops indeed.
>
> Reviewed-by: Maarten Lankhorst
before pushing to dinq please check the compilation there..
kbuild bot raised an issue...
so apparently we will need a backmerge before pushing this...
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
On Wed, 2018-08-08 at 10:53 +0200, Jan-Marek Glogowski wrote:
> This re-applies the workaround for "some DP sinks, [which] are a
> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> quality check unconditionally during long pulse").
> It makes the secondary AOC E2460P monitor connected via DP to an
> acer Veriton N4640G usable again.
>
> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> DP link retraining into the ->post_hotplug() hook")
>
> Signed-off-by: Jan-Marek Glogowski
Just as a heads up I haven't forgotten about this! Coincidentally; it turns
out a real life friend of mine is seeing an issue that is very likely fixed by
this patch :), so I am going to have them do some checks for me to make sure
this fix is OK (it looks fine to me, but you can't be too careful)
> ---
> drivers/gpu/drm/i915/intel_dp.c | 33 +++--
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 8e0e14b..22b2452 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> *intel_dp)
> return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
> }
>
> -/*
> - * If display is now connected check links status,
> - * there has been known issues of link loss triggering
> - * long pulse.
> - *
> - * Some sinks (eg. ASUS PB287Q) seem to perform some
> - * weird HPD ping pong during modesets. So we can apparently
> - * end up with HPD going low during a modeset, and then
> - * going back up soon after. And once that happens we must
> - * retrain the link to get a picture. That's in case no
> - * userspace component reacted to intermittent HPD dip.
> - */
> int intel_dp_retrain_link(struct intel_encoder *encoder,
> struct drm_modeset_acquire_ctx *ctx)
> {
> @@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> }
>
> static int
> -intel_dp_long_pulse(struct intel_connector *connector)
> +intel_dp_long_pulse(struct intel_connector *connector,
> + struct drm_modeset_acquire_ctx *ctx)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_dp *intel_dp = intel_attached_dp(>base);
> @@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector
> *connector)
>*/
> status = connector_status_disconnected;
> goto out;
> + } else {
> + /*
> + * If display is now connected check links status,
> + * there has been known issues of link loss triggering
> + * long pulse.
> + *
> + * Some sinks (eg. ASUS PB287Q) seem to perform some
> + * weird HPD ping pong during modesets. So we can apparently
> + * end up with HPD going low during a modeset, and then
> + * going back up soon after. And once that happens we must
> + * retrain the link to get a picture. That's in case no
> + * userspace component reacted to intermittent HPD dip.
> + */
> + struct intel_encoder *encoder = _to_dig_port(intel_dp)-
> >base;
> +
> + intel_dp_retrain_link(encoder, ctx);
> }
>
> /*
> @@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector *connector,
> return ret;
> }
>
> - status = intel_dp_long_pulse(intel_dp->attached_connector);
> + status = intel_dp_long_pulse(intel_dp->attached_connector,
> ctx);
> }
>
> intel_dp->detect_done = false;
--
Cheers,
Lyude Paul
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/kms_cursor_crc: Open DRM device with DRIVER_ANY
So that this test can be run in drivers other than i915.
Remove devid and only check it if the driver is i915.
Signed-off-by: Haneen Mohammed
---
tests/kms_cursor_crc.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index a164839e..870d7fec 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -59,7 +59,6 @@ typedef struct {
int curw, curh; /* cursor size */
int cursor_max_w, cursor_max_h;
igt_pipe_crc_t *pipe_crc;
- uint32_t devid;
unsigned flags;
} data_t;
@@ -108,6 +107,13 @@ static void cursor_disable(data_t *data)
static bool chv_cursor_broken(data_t *data, int x)
{
+ uint32_t devid;
+
+ if (!is_i915_device(data->drm_fd))
+ return false;
+
+ devid = intel_get_drm_devid(data->drm_fd);
+
/*
* CHV gets a FIFO underrun on pipe C when cursor x coordinate
* is negative and the cursor visible.
@@ -121,7 +127,7 @@ static bool chv_cursor_broken(data_t *data, int x)
if (x >= 0)
return false;
- return IS_CHERRYVIEW(data->devid) && data->pipe == PIPE_C;
+ return IS_CHERRYVIEW(devid) && data->pipe == PIPE_C;
}
static bool cursor_visible(data_t *data, int x, int y)
@@ -459,8 +465,15 @@ static void create_cursor_fb(data_t *data, int cur_w, int
cur_h)
igt_put_cairo_ctx(data->drm_fd, >fb, cr);
}
-static bool has_nonsquare_cursors(uint32_t devid)
+static bool has_nonsquare_cursors(data_t *data)
{
+ uint32_t devid;
+
+ if (!is_i915_device(data->drm_fd))
+ return true;
+
+ devid = intel_get_drm_devid(data->drm_fd);
+
/*
* Test non-square cursors a bit on the platforms
* that support such things.
@@ -616,19 +629,19 @@ static void run_test_generic(data_t *data)
/* Using created cursor FBs to test cursor support */
igt_subtest_f("cursor-%dx%d-onscreen", w, h) {
- igt_require(has_nonsquare_cursors(data->devid));
+ igt_require(has_nonsquare_cursors(data));
run_test(data, test_crc_onscreen, w, h);
}
igt_subtest_f("cursor-%dx%d-offscreen", w, h) {
- igt_require(has_nonsquare_cursors(data->devid));
+ igt_require(has_nonsquare_cursors(data));
run_test(data, test_crc_offscreen, w, h);
}
igt_subtest_f("cursor-%dx%d-sliding", w, h) {
- igt_require(has_nonsquare_cursors(data->devid));
+ igt_require(has_nonsquare_cursors(data));
run_test(data, test_crc_sliding, w, h);
}
igt_subtest_f("cursor-%dx%d-random", w, h) {
- igt_require(has_nonsquare_cursors(data->devid));
+ igt_require(has_nonsquare_cursors(data));
run_test(data, test_crc_random, w, h);
}
@@ -647,9 +660,7 @@ igt_main
igt_skip_on_simulation();
igt_fixture {
- data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
-
- data.devid = intel_get_drm_devid(data.drm_fd);
+ data.drm_fd = drm_open_driver_master(DRIVER_ANY);
ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH,
_width);
igt_assert(ret == 0 || errno == EINVAL);
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Wait for vblank between disabling planes and disabling crtc
== Series Details == Series: drm/i915: Wait for vblank between disabling planes and disabling crtc URL : https://patchwork.freedesktop.org/series/48113/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4662_full -> Patchwork_9930_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9930_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9930_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9930_full: === IGT changes === Warnings igt@kms_draw_crc@draw-method-rgb565-pwrite-xtiled: shard-snb: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_9930_full that come from known issues: === IGT changes === Issues hit igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render: shard-snb: NOTRUN -> INCOMPLETE (fdo#105411) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) Possible fixes igt@kms_flip@flip-vs-expired-vblank: shard-kbl: FAIL (fdo#102887, fdo#105363) -> PASS igt@perf_pmu@busy-accuracy-50-bcs0: shard-snb: INCOMPLETE (fdo#105411) -> SKIP fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4662 -> Patchwork_9930 CI_DRM_4662: 3010d040760de7eb87c6eb54985ba6220447bbba @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9930: a46c5d644c7f46d5c153a8503fa499e2ee7488b2 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9930/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/14] drm/i915: Restrict gen6_reset_rps_interrupts to gen6+
Chris Wilson writes: > Do not call gen6_reset_rps_interrupts() when we know the registers do not > exist. > > Signed-off-by: Chris Wilson This makes me wonder about adding some known gen6+ debug register range assertions to I915_WRITE when gen < 6. i915.mmio_debug for all (gens) :) Reviewed-by: Mika Kuoppala > --- > 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 03654f5f68c3..9a01560c5bd1 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -8260,7 +8260,7 @@ void intel_sanitize_gt_powersave(struct > drm_i915_private *dev_priv) > > if (INTEL_GEN(dev_priv) >= 11) > gen11_reset_rps_interrupts(dev_priv); > - else > + else if (INTEL_GEN(dev_priv) >= 6) > gen6_reset_rps_interrupts(dev_priv); > } > > -- > 2.18.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/14] drm/i915: Cleanup gt powerstate from gem
Chris Wilson writes: > Since the gt powerstate is allocated by i915_gem_init, clean it from > i915_gem_fini for symmetry and to correct the imbalance on error. > Was first confused about the allocation as it is embedded. But vlv really does allocate an underlying power context object. And the asymmetry could lead to freeing an not allocated object on error path. > Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Improve crc-core driver interface (rev11)
== Series Details ==
Series: Improve crc-core driver interface (rev11)
URL : https://patchwork.freedesktop.org/series/45246/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4663 -> Patchwork_9931 =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_9931 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9931, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/45246/revisions/11/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9931:
=== IGT changes ===
Possible regressions
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
fi-ivb-3520m: PASS -> FAIL +5
{fi-icl-u}: PASS -> FAIL +5
fi-skl-6700hq: PASS -> FAIL +5
fi-skl-guc: PASS -> FAIL +5
fi-blb-e6850: PASS -> FAIL +3
fi-byt-j1900: PASS -> FAIL +3
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-ilk-650: PASS -> FAIL +3
fi-elk-e7500: PASS -> FAIL +3
fi-byt-n2820: PASS -> FAIL +3
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
fi-bdw-5557u: PASS -> FAIL +5
fi-pnv-d510:PASS -> FAIL +3
fi-skl-6600u: PASS -> FAIL +5
{fi-byt-clapper}: PASS -> FAIL +3
fi-bxt-dsi: PASS -> FAIL +5
fi-hsw-4770:PASS -> FAIL +5
fi-ivb-3770:PASS -> FAIL +5
fi-bwr-2160:PASS -> FAIL +3
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
{fi-bdw-samus}: PASS -> FAIL +5
fi-hsw-peppy: PASS -> FAIL +5
fi-bdw-gvtdvm: PASS -> FAIL +5
fi-gdg-551: NOTRUN -> FAIL +3
fi-kbl-7500u: PASS -> FAIL +5
fi-snb-2600:PASS -> FAIL +3
fi-hsw-4770r: PASS -> FAIL +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
fi-kbl-7567u: PASS -> FAIL +5
fi-skl-6260u: PASS -> FAIL +5
fi-skl-6700k2: PASS -> FAIL +5
fi-skl-gvtdvm: PASS -> FAIL +5
{fi-skl-iommu}: PASS -> FAIL +5
fi-skl-6770hq: PASS -> FAIL +5
fi-bxt-j4205: PASS -> FAIL +5
fi-kbl-7560u: PASS -> FAIL +5
fi-whl-u: PASS -> FAIL +5
fi-kbl-r: PASS -> FAIL +5
== Known issues ==
Here are the changes found in Patchwork_9931 that come from known issues:
=== IGT changes ===
Issues hit
igt@debugfs_test@read_all_entries:
fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713)
igt@drv_selftest@live_hangcheck:
fi-cfl-s3: PASS -> DMESG-FAIL (fdo#106560)
igt@drv_selftest@live_workarounds:
fi-skl-6700k2: PASS -> DMESG-FAIL (fdo#107292)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
fi-skl-guc: PASS -> FAIL (fdo#103191)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
fi-cfl-s3: PASS -> FAIL (fdo#103481) +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
{fi-bsw-kefka}: PASS -> FAIL (fdo#106211) +3
{fi-cfl-8109u}: PASS -> FAIL (fdo#103481) +5
fi-cfl-guc: PASS -> FAIL (fdo#103481) +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-cnl-psr: PASS -> FAIL (fdo#106211) +5
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
fi-glk-j4005: PASS -> FAIL (fdo#106211) +5
fi-cfl-8700k: PASS -> FAIL (fdo#103481) +5
fi-bsw-n3050: PASS -> FAIL (fdo#106211) +1
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
fi-glk-dsi: PASS -> FAIL (fdo#106211) +5
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
{fi-byt-clapper}: PASS -> FAIL (fdo#103191, fdo#107362)
Possible fixes
igt@gem_exec_suspend@basic-s3:
{fi-cfl-8109u}: FAIL (fdo#103375) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#106211 https://bugs.freedesktop.org/show_bug.cgi?id=106211
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107292
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Expose retry count to per gen reset logic (rev4)
== Series Details == Series: series starting with [1/2] drm/i915: Expose retry count to per gen reset logic (rev4) URL : https://patchwork.freedesktop.org/series/48019/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4662_full -> Patchwork_9929_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9929_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@2x-flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render: shard-snb: NOTRUN -> INCOMPLETE (fdo#105411) igt@kms_rotation_crc@sprite-rotation-180: shard-hsw: PASS -> FAIL (fdo#103925) Possible fixes igt@kms_flip@flip-vs-expired-vblank: shard-kbl: FAIL (fdo#102887, fdo#105363) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS igt@perf_pmu@busy-accuracy-50-bcs0: shard-snb: INCOMPLETE (fdo#105411) -> SKIP fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4662 -> Patchwork_9929 CI_DRM_4662: 3010d040760de7eb87c6eb54985ba6220447bbba @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9929: 871b39c1a7e41c61b09fe26c6dd445a59ba4477b @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9929/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/3] Revert "drm: crc: Wait for a frame before returning from open()"
This reverts commit e8fa5671183c80342d520ad81d14fa79a9d4a680. Don't wait for first CRC during crtc_crc_open. It avoids one frame wait during open. If application want to wait after read call, it can use poll/read blocking read() call. Suggested-by: Ville Syrjälä Signed-off-by: Mahesh Kumar Cc: dri-de...@lists.freedesktop.org Cc: Tomeu Vizoso Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/drm_debugfs_crc.c | 16 1 file changed, 16 deletions(-) diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c index 3e0a2cfaa35c..00e743153e94 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -228,24 +228,8 @@ static int crtc_crc_open(struct inode *inode, struct file *filep) if (ret) goto err; - spin_lock_irq(>lock); - /* -* Only return once we got a first frame, so userspace doesn't have to -* guess when this particular piece of HW will be ready to start -* generating CRCs. -*/ - ret = wait_event_interruptible_lock_irq(crc->wq, - crtc_crc_data_count(crc), - crc->lock); - spin_unlock_irq(>lock); - - if (ret) - goto err_disable; - return 0; -err_disable: - crtc->funcs->set_crc_source(crtc, NULL); err: spin_lock_irq(>lock); crtc_crc_cleanup(crc); -- 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v1 1/3] drm/vkms/crc: Implement verify_crc_source callback
This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.
Cc: dri-de...@lists.freedesktop.org
Cc: Haneen Mohammed
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/vkms/vkms_crc.c | 36
drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..ce5ee0461f80 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,35 @@ void vkms_crc_work_handle(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, );
}
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+ int ret = 0;
+
+ if (!src_name) {
+ *enabled = false;
+ } else if (strcmp(src_name, "auto") == 0) {
+ *enabled = true;
+ } else {
+ *enabled = false;
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+ size_t *values_cnt)
+{
+ bool enabled;
+
+ if (vkms_crc_parse_source(src_name, ) < 0) {
+ DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt)
{
@@ -78,10 +107,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
*src_name,
unsigned long flags;
int ret = 0;
- if (src_name && strcmp(src_name, "auto") == 0)
- enabled = true;
- else if (src_name)
- ret = -EINVAL;
+ ret = vkms_crc_parse_source(src_name, );
+ if (ret)
+ return ret;
*values_cnt = 1;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.enable_vblank = vkms_enable_vblank,
.disable_vblank = vkms_disable_vblank,
.set_crc_source = vkms_set_crc_source,
+ .verify_crc_source = vkms_verify_crc_source,
};
static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
/* CRC Support */
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+ size_t *values_cnt);
void vkms_crc_work_handle(struct work_struct *work);
#endif /* _VKMS_DRV_H_ */
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 2/3] drm/crc: Cleanup crtc_crc_open function
This patch make changes to allocate crc-entries buffer before
enabling CRC generation.
It moves all the failure check early in the function before setting
the source or memory allocation.
Now set_crc_source takes only two variable inputs, values_cnt we
already gets as part of verify_crc_source.
Changes since V1:
- refactor code to use single spin lock
Changes since V2:
- rebase
Changes since V3:
- rebase on top of VKMS driver
Signed-off-by: Mahesh Kumar
Cc: dri-de...@lists.freedesktop.org
Cc: Laurent Pinchart
Cc: Haneen Mohammed
Reviewed-by: Maarten Lankhorst (V2)
Acked-by: Leo Li (V2)
Reviewed-by: Laurent Pinchart (V3)
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 4 +-
drivers/gpu/drm/drm_debugfs_crc.c | 61 ++
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pipe_crc.c | 4 +-
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c| 6 +--
drivers/gpu/drm/vkms/vkms_crc.c| 5 +-
drivers/gpu/drm/vkms/vkms_drv.h| 3 +-
include/drm/drm_crtc.h | 3 +-
10 files changed, 39 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index e43ed064dc46..54056d180003 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -258,8 +258,7 @@ amdgpu_dm_remove_sink_from_freesync_module(struct
drm_connector *connector);
/* amdgpu_dm_crc.c */
#ifdef CONFIG_DEBUG_FS
-int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
- size_t *values_cnt);
+int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name);
int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc,
const char *src_name,
size_t *values_cnt);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
index dfcca594d52a..e7ad528f5853 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
@@ -62,8 +62,7 @@ amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const
char *src_name,
return 0;
}
-int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
- size_t *values_cnt)
+int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
{
struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state);
struct dc_stream_state *stream_state = crtc_state->stream;
@@ -99,7 +98,6 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc,
const char *src_name,
return -EINVAL;
}
- *values_cnt = 3;
/* Reset crc_skipped on dm state */
crtc_state->crc_skip_count = 0;
return 0;
diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c
index d7e626331eca..3e0a2cfaa35c 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -127,11 +127,9 @@ static ssize_t crc_control_write(struct file *file, const
char __user *ubuf,
if (source[len] == '\n')
source[len] = '\0';
- if (crtc->funcs->verify_crc_source) {
- ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
- if (ret)
- return ret;
- }
+ ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
+ if (ret)
+ return ret;
spin_lock_irq(>lock);
@@ -197,40 +195,40 @@ static int crtc_crc_open(struct inode *inode, struct file
*filep)
return ret;
}
+ ret = crtc->funcs->verify_crc_source(crtc, crc->source, _cnt);
+ if (ret)
+ return ret;
+
+ if (WARN_ON(values_cnt > DRM_MAX_CRC_NR))
+ return -EINVAL;
+
+ if (WARN_ON(values_cnt == 0))
+ return -EINVAL;
+
+ entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
+ if (!entries)
+ return -ENOMEM;
+
spin_lock_irq(>lock);
- if (!crc->opened)
+ if (!crc->opened) {
crc->opened = true;
- else
+ crc->entries = entries;
+ crc->values_cnt = values_cnt;
+ } else {
ret = -EBUSY;
+ }
spin_unlock_irq(>lock);
- if (ret)
+ if (ret) {
+ kfree(entries);
return ret;
+ }
- ret = crtc->funcs->set_crc_source(crtc, crc->source, _cnt);
+ ret = crtc->funcs->set_crc_source(crtc, crc->source);
if (ret)
goto
[Intel-gfx] [PATCH v5 0/3] Improve crc-core driver interface
This series improves crc-core <-> driver interface. This series adds following functionality in the crc-core - Now control node will print all the available sources if implemented by driver along with current source. - Setting of sorce will fail if provided source is not supported - cleanup of crtc_crc_open function first allocate memory before enabling CRC generation - Don't block open() call instead wait in crc read call. Following IGT will fail due to crc-core <-> driver interface change igt@kms_pipe_crc_basic@bad-source ig@kms_pipe_crc_basic@nonblocking-crc-pipe-X ig@kms_pipe_crc_basic@nonblocking-crc-pipe-X-frame-sequence In nonblocking crc tests we'll get lesser crc's due to skipping crc AMD/Rockchip/rcar code path is not validated and may need inputs Changes: - Rebase on top of VKMS driver Cc: dri-de...@lists.freedesktop.org Mahesh Kumar (3): drm/vkms/crc: Implement verify_crc_source callback drm/crc: Cleanup crtc_crc_open function Revert "drm: crc: Wait for a frame before returning from open()" drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 4 +- drivers/gpu/drm/drm_debugfs_crc.c | 71 -- drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_pipe_crc.c | 4 +- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c| 6 +- drivers/gpu/drm/vkms/vkms_crc.c| 41 ++--- drivers/gpu/drm/vkms/vkms_crtc.c | 1 + drivers/gpu/drm/vkms/vkms_drv.h| 5 +- include/drm/drm_crtc.h | 3 +- 11 files changed, 71 insertions(+), 74 deletions(-) -- 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/2] drm/i915: Adding YUV444 packed format(DRM_FORMAT_XYUV) support.
On 13-Aug-18 7:46 PM, Sharma, Swati2 wrote:
Hi Stanlis,
Won't we defining XYUV PF in skl_update_scaler_plane? Else scaler
won't be updated for XYUV PF.
On 10-Aug-18 6:50 PM, StanLis wrote:
From: Stanislav Lisovskiy
PLANE_CTL_FORMAT_AYUV is already supported, according to hardware
specification.
v2: Edited commit message, removed redundant whitespaces.
v3: Fixed fallthrough logic for the format switch cases.
v4: Yet again fixed fallthrough logic, to reuse code from other case
labels.
v5: Started to use XYUV instead of AYUV, as we don't use alpha.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/intel_display.c | 7 +++
drivers/gpu/drm/i915/intel_sprite.c | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 56818a45181c..8a4f763c7ca3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_YVYU,
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
+ DRM_FORMAT_XYUV,
};
static const uint32_t skl_pri_planar_formats[] = {
@@ -102,6 +103,7 @@ static const uint32_t skl_pri_planar_formats[] = {
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
DRM_FORMAT_NV12,
+ DRM_FORMAT_XYUV,
};
static const uint64_t skl_format_modifiers_noccs[] = {
@@ -3497,6 +3499,8 @@ static u32 skl_plane_ctl_format(uint32_t
pixel_format)
return PLANE_CTL_FORMAT_XRGB_2101010;
case DRM_FORMAT_XBGR2101010:
return PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
+ case DRM_FORMAT_XYUV:
+ return PLANE_CTL_FORMAT_AYUV;
case DRM_FORMAT_YUYV:
return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
case DRM_FORMAT_YVYU:
@@ -13371,6 +13375,7 @@ static bool
skl_plane_format_mod_supported(struct drm_plane *_plane,
}
switch (format) {
+
case DRM_FORMAT_XRGB:
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ARGB:
@@ -13387,6 +13392,7 @@ static bool
skl_plane_format_mod_supported(struct drm_plane *_plane,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_XYUV:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
/* fall through */
@@ -14510,6 +14516,7 @@ static int intel_framebuffer_init(struct
intel_framebuffer *intel_fb,
goto err;
}
break;
+ case DRM_FORMAT_XYUV:
case DRM_FORMAT_YUYV:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 344c0e709b19..812e4e8afe2b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1421,6 +1421,7 @@ static bool
skl_plane_format_mod_supported(struct drm_plane *_plane,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_XYUV:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
/* fall through */
--
Thanks and Regards,
Swati
___
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: Wait for vblank between disabling planes and disabling crtc
== Series Details ==
Series: drm/i915: Wait for vblank between disabling planes and disabling crtc
URL : https://patchwork.freedesktop.org/series/48113/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4662 -> Patchwork_9930 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48113/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9930 that come from known issues:
=== IGT changes ===
Issues hit
{igt@amdgpu/amd_prime@amd-to-i915}:
{fi-kbl-8809g}: NOTRUN -> FAIL (fdo#107341)
igt@drv_module_reload@basic-reload:
fi-ilk-650: PASS -> DMESG-WARN (fdo#106387)
Possible fixes
{igt@amdgpu/amd_basic@userptr}:
{fi-kbl-8809g}: INCOMPLETE (fdo#107402) -> PASS
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: FAIL (fdo#103167) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341
fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
== Participating hosts (54 -> 49) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4662 -> Patchwork_9930
CI_DRM_4662: 3010d040760de7eb87c6eb54985ba6220447bbba @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9930: a46c5d644c7f46d5c153a8503fa499e2ee7488b2 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a46c5d644c7f drm/i915: Wait for vblank between disabling planes and disabling
crtc
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9930/issues.html
___
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: Expose retry count to per gen reset logic
Chris Wilson writes: > Quoting Mika Kuoppala (2018-08-10 15:00:35) >> There is a possibility for per gen reset logic to >> be more nasty if the softer approach on resetting does >> not bear fruit. >> >> Expose retry count to per gen reset logic if it >> wants to take such tough measures. >> >> Cc: Chris Wilson >> Signed-off-by: Mika Kuoppala > Reviewed-by: Chris Wilson Pushed patches 1&2, thanks for review! -Mika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Wait for vblank between disabling planes and disabling crtc
== Series Details == Series: drm/i915: Wait for vblank between disabling planes and disabling crtc URL : https://patchwork.freedesktop.org/series/48113/ State : warning == Summary == $ dim checkpatch origin/drm-tip a46c5d644c7f drm/i915: Wait for vblank between disabling planes and disabling crtc -:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #13: References: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/877396/ total: 0 errors, 1 warnings, 0 checks, 9 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Expose retry count to per gen reset logic (rev4)
== Series Details ==
Series: series starting with [1/2] drm/i915: Expose retry count to per gen
reset logic (rev4)
URL : https://patchwork.freedesktop.org/series/48019/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4662 -> Patchwork_9929 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48019/revisions/4/mbox/
== Known issues ==
Here are the changes found in Patchwork_9929 that come from known issues:
=== IGT changes ===
Issues hit
{igt@amdgpu/amd_prime@amd-to-i915}:
{fi-kbl-8809g}: NOTRUN -> FAIL (fdo#107341)
igt@drv_selftest@live_hangcheck:
fi-skl-guc: PASS -> DMESG-FAIL (fdo#107174)
igt@drv_selftest@live_requests:
fi-bsw-n3050: PASS -> INCOMPLETE (fdo#105876)
igt@drv_selftest@live_workarounds:
fi-bsw-n3050: PASS -> DMESG-FAIL (fdo#107292)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362)
Possible fixes
{igt@amdgpu/amd_basic@userptr}:
{fi-kbl-8809g}: INCOMPLETE (fdo#107402) -> PASS
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: FAIL (fdo#103167) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105876 https://bugs.freedesktop.org/show_bug.cgi?id=105876
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
== Participating hosts (54 -> 48) ==
Missing(6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_4662 -> Patchwork_9929
CI_DRM_4662: 3010d040760de7eb87c6eb54985ba6220447bbba @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9929: 871b39c1a7e41c61b09fe26c6dd445a59ba4477b @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
871b39c1a7e4 drm/i915: Force reset on unready engine
b0f40a9d4041 drm/i915: Expose retry count to per gen reset logic
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9929/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
Op 11-08-18 om 02:50 schreef Dhinakaran Pandiyan:
> We print the last attempted entry and last exit timestamps only when
> IRQ debug is requested. This check was missed when new debug flags were
> added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
> runtime through debugfs, v6")
>
> Cc: Maarten Lankhorst
> Signed-off-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 26b7e5276b15..374b550d9a4f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct seq_file *m, void
> *data)
> psr_source_status(dev_priv, m);
> mutex_unlock(_priv->psr.lock);
>
> - if (READ_ONCE(dev_priv->psr.debug)) {
> + if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
> seq_printf(m, "Last attempted entry at: %lld\n",
> dev_priv->psr.last_entry_attempt);
> seq_printf(m, "Last exit at: %lld\n",
Oops indeed.
Reviewed-by: Maarten Lankhorst
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Wait for vblank between disabling planes and disabling crtc
Disabling cursor does not take effect until the next vblank, but if the pipe is shut down too quickly after a cursor disable, we will see random corruption from where the mouse cursor used to be when the pipe is enabled again. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106478 References: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/877396/ Signed-off-by: Maarten Lankhorst Cc: Sean Paul Cc: Kristian H. Kristensen --- drivers/gpu/drm/i915/intel_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a2aed7e3ed0d..a02c77a3bc7e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12673,6 +12673,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) */ intel_crtc_disable_pipe_crc(intel_crtc); + /* Wait a vblank for planes to actually be turned off */ + intel_wait_for_vblank(dev_priv, intel_crtc->pipe); + dev_priv->display.crtc_disable(to_intel_crtc_state(old_crtc_state), state); intel_crtc->active = false; intel_fbc_disable(intel_crtc); -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option
Joonas, sorry for interfering; could you please explain more regarding the options for tracing scheduling events better than tracepoints? After scheduling moves to GuC tools will have to switch to something like GuC-logging; but while kmd does scheduling isn't kernel-tracing the best solution? I know gpuvis is not the only attempt to use tracepoints for the same purpose. (there're trace.pl and S.E.A. and of course VTune though it probably is not considered to be existing as it's not open source). And assuming this movement towards GuC is it not too late to invent a completely new way to provide tools with scheduling info from kmd? Could we just improve the existing way and let it live its last years\months? gpuvis works w\o modifying kernel for AMDgpu showing HW queue and HW execution; it cosplays Microsoft GPUView which works out-of-the-box on Windows too. Thus it appears that intel gfx on linux is the most closed platform, not bothering of observability (or even bothering about how to forbid observability). Not long ago the MediaSDK team diagnosed a problem with their workloads looking at VTune timelines - seeing the difference between the time request came to kmd and time it went runnable & comparing the queues on 2 engines they understood that their requests have dependencies that were definitely unexpected. MediaSDK reported the problem to driver people and it was fixed. I can add Dmitry Rogozhkin to discussion if the usefulness of scheduling timeline in tools is questionable, as far as I remember this wasn't the only use case they had, I'm sure he can add more. Thank you, Svetlana -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Joonas Lahtinen Sent: Monday, August 13, 2018 12:55 PM To: Chris Wilson ; Intel-gfx@lists.freedesktop.org; Tvrtko Ursulin ; Tvrtko Ursulin Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option Quoting Tvrtko Ursulin (2018-08-08 15:56:01) > On 08/08/2018 13:42, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-08-08 13:13:08) > This is true, no disagreement. My point simply was that we can provide > this info easily to anyone. There is a little bit of analogy with perf > scheduler tracing/map etc. > > > I don't see any value in giving the information away, just the cost. > > If you can convince Joonas of its merit, and if we can define just > > exactly what ABI it constitutes, then I'd be happy to be the one who > > says "I told you so" in the future for a change. > > I think Joonas was okay in principle that we soft-commit to _trying_ > to keep _some_ tracepoint stable-ish (where it makes sense and after > some discussion for each) if IGT also materializes which auto-pings us > (via > CI) when we break one of them. But I may be misremembering so Joonas > please comment. Currently gpuvis, using these, seems to be only packaged in one AUR repo, and they do make a not in the wiki how you need to configure kernel for debugging. And there's been no apparent demand for them to have it in stock kernel. And even when we do get demand for having gpuvis or another tool working from vanilla kernel, tracepoints being a rather tricky subject, I would start the discussion by going through alternative means of providing the information the tool needs and considering those. So lets still keep this option as it was introduced. The whole "tracepoints as stable uAPI" idea is a can of worms which is only dug into when other options are exhausted. Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx Joint Stock Company Intel A/O Registered legal address: Krylatsky Hills Business Park, 17 Krylatskaya Str., Bldg 4, Moscow 121614, Russian Federation This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
If engine reports that it is not ready for reset, we
give up. Evidence shows that forcing a per engine reset
on an engine which is not reporting to be ready for reset,
can bring it back into a working order. There is risk that
we corrupt the context image currently executing on that
engine. But that is a risk worth taking as if we unblock
the engine, we prevent a whole device wedging in a case
of full gpu reset.
Reset individual engine even if it reports that it is not
prepared for reset, but only if we aim for full gpu reset
and not on first reset attempt.
v2: force reset only on later attempts, readability (Chris)
v3: simplify with adequate caffeine levels (Chris)
v4: comment about risks and migitations (Chris)
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_uncore.c | 50 +
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 027d14574bfa..20f2f5ad9c3f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2085,7 +2085,7 @@ int __intel_wait_for_register(struct drm_i915_private
*dev_priv,
return ret;
}
-static int gen8_reset_engine_start(struct intel_engine_cs *engine)
+static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
int ret;
@@ -2105,7 +2105,7 @@ static int gen8_reset_engine_start(struct intel_engine_cs
*engine)
return ret;
}
-static void gen8_reset_engine_cancel(struct intel_engine_cs *engine)
+static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
@@ -2113,29 +2113,50 @@ static void gen8_reset_engine_cancel(struct
intel_engine_cs *engine)
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
+static int reset_engines(struct drm_i915_private *i915,
+unsigned int engine_mask,
+unsigned int retry)
+{
+ if (INTEL_GEN(i915) >= 11)
+ return gen11_reset_engines(i915, engine_mask);
+ else
+ return gen6_reset_engines(i915, engine_mask, retry);
+}
+
static int gen8_reset_engines(struct drm_i915_private *dev_priv,
unsigned int engine_mask,
unsigned int retry)
{
struct intel_engine_cs *engine;
+ const bool reset_non_ready = retry >= 1;
unsigned int tmp;
int ret;
for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
- if (gen8_reset_engine_start(engine)) {
- ret = -EIO;
- goto not_ready;
- }
+ ret = gen8_engine_reset_prepare(engine);
+ if (ret && !reset_non_ready)
+ goto skip_reset;
+
+ /*
+* If this is not the first failed attempt to prepare,
+* we decide to proceed anyway.
+*
+* By doing so we risk context corruption and with
+* some gens (kbl), possible system hang if reset
+* happens during active bb execution.
+*
+* We rather take context corruption instead of
+* failed reset with a wedged driver/gpu. And
+* active bb execution case should be covered by
+* i915_stop_engines we have before the reset.
+*/
}
- if (INTEL_GEN(dev_priv) >= 11)
- ret = gen11_reset_engines(dev_priv, engine_mask);
- else
- ret = gen6_reset_engines(dev_priv, engine_mask, retry);
+ ret = reset_engines(dev_priv, engine_mask, retry);
-not_ready:
+skip_reset:
for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
- gen8_reset_engine_cancel(engine);
+ gen8_engine_reset_cancel(engine);
return ret;
}
@@ -2164,12 +2185,15 @@ static reset_func intel_get_gpu_reset(struct
drm_i915_private *dev_priv)
return NULL;
}
-int intel_gpu_reset(struct drm_i915_private *dev_priv, unsigned int
engine_mask)
+int intel_gpu_reset(struct drm_i915_private *dev_priv,
+ const unsigned int engine_mask)
{
reset_func reset = intel_get_gpu_reset(dev_priv);
unsigned int retry;
int ret;
+ GEM_BUG_ON(!engine_mask);
+
/*
* We want to perform per-engine reset from atomic context (e.g.
* softirq), which imposes the constraint that we cannot sleep.
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Bump priority of clean up work
Quoting Mika Kuoppala (2018-08-13 13:49:43) > Chris Wilson writes: > > > Quoting Chris Wilson (2018-07-12 12:57:29) > >> We require that we keep the list of outstanding work short so that we do > >> not "leak" memory while pageflipping under stress. However that system > >> stress may delay kernel workers virtually indefinitely, which incurs the > >> pageflips stall and eventually hit a timeout waiting for the cleanup. > >> > >> Try to combat CPU starvation of our short-lived cleanup workers by > >> switching to a high priority workqueue. > >> > >> Testcase: igt/kms_cursor_legacy/all-pipes-torture-move > >> References: https://bugs.freedesktop.org/show_bug.cgi?id=107122 > >> Signed-off-by: Chris Wilson > >> Cc: Daniel Vetter > >> --- > >> Not sure if highpri is enough to combat our RT torture... > > > > CI thinks it is. > > Not so familiar with the atomic commit. But it makes > sense and now there is evidence supporting it. Proof is indeed in the pudding; the CI stress case is about the worst it can be so, with any luck the starvation issue is prevented. Thanks, pushed. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Expose retry count to per gen reset logic (rev3)
== Series Details == Series: series starting with [1/2] drm/i915: Expose retry count to per gen reset logic (rev3) URL : https://patchwork.freedesktop.org/series/48019/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4660_full -> Patchwork_9928_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9928_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@fence-restore-untiled: shard-glk: PASS -> FAIL (fdo#103375) igt@drv_suspend@shrink: shard-snb: PASS -> FAIL (fdo#106886) shard-glk: PASS -> FAIL (fdo#106886) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) igt@perf@polling: shard-hsw: PASS -> FAIL (fdo#102252) Possible fixes igt@gem_eio@reset-stress: shard-apl: FAIL -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4660 -> Patchwork_9928 CI_DRM_4660: 9cd882754c1017e68ed9d2c8d57dc326530522fe @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9928: 43b30337b551eda6a92a54cd873bcd4256da34cd @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9928/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Bump priority of clean up work
Chris Wilson writes: > Quoting Chris Wilson (2018-07-12 12:57:29) >> We require that we keep the list of outstanding work short so that we do >> not "leak" memory while pageflipping under stress. However that system >> stress may delay kernel workers virtually indefinitely, which incurs the >> pageflips stall and eventually hit a timeout waiting for the cleanup. >> >> Try to combat CPU starvation of our short-lived cleanup workers by >> switching to a high priority workqueue. >> >> Testcase: igt/kms_cursor_legacy/all-pipes-torture-move >> References: https://bugs.freedesktop.org/show_bug.cgi?id=107122 >> Signed-off-by: Chris Wilson >> Cc: Daniel Vetter >> --- >> Not sure if highpri is enough to combat our RT torture... > > CI thinks it is. Not so familiar with the atomic commit. But it makes sense and now there is evidence supporting it. Reviewed-by: Mika Kuoppala > -Chris > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V6 10/10] drm/rcar-du/crc: Implement get_crc_sources callback
Hey, Op 08-08-18 om 17:26 schreef Mahesh Kumar: > This patch implements get_crc_sources callback, which returns list of > all the crc sources supported by driver in current platform. > > Changes Since V1: > - move sources list per-crtc > - init sources-list only for gen3 > Changes Since V2: > - Adopt to driver style > - Address other review comments from Laurent Pinchart > Changes Since V3/4/5: (Laurent Pinchart review) > - s/rcar_du_crtc_crc_sources_list_init/rcar_du_crtc_crc_init > - s/rcar_du_crtc_crc_sources_list_uninit/rcar_du_crtc_crc_cleanup > - other cleanup > > Signed-off-by: Mahesh Kumar > Cc: dri-de...@lists.freedesktop.org > Cc: Laurent Pinchart > Reviewed-by: Laurent Pinchart Thanks, I've pushed this and patch 1-7 to drm-misc-next. 8/ needs to be rebased because the VKMS driver appears to be able to generate CRC's now. Didn't yet push 9/ because of the polling change needed in igt. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/2] drm: Introduce new DRM_FORMAT_XYUV
On Fri, 2018-08-10 at 19:01 +0100, Alexandru-Cosmin Gheorghe wrote:
> Hi Stanislav,
>
> FYI, we are trying to add same format under a slightly different
> name.
> See https://lists.freedesktop.org/archives/dri-devel/2018-
> July/184598.html
So we probably just need to decide, if this should be DRM_FORMAT_XYUV
or DRM_FORMAT_XYUV :)
>
> On Fri, Aug 10, 2018 at 04:19:03PM +0300, StanLis wrote:
> > From: Stanislav Lisovskiy
> >
> > v5: This is YUV444 packed format same as AYUV, but without alpha,
> > as supported by i915.
> >
> > Signed-off-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/drm_fourcc.c | 1 +
> > include/uapi/drm/drm_fourcc.h | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_fourcc.c
> > b/drivers/gpu/drm/drm_fourcc.c
> > index 5ca6395cd4d3..5bde1b20a098 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -173,6 +173,7 @@ const struct drm_format_info
> > *__drm_format_info(u32 format)
> > { .format = DRM_FORMAT_UYVY,.depth
> > = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> > { .format = DRM_FORMAT_VYUY,.depth
> > = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> > { .format = DRM_FORMAT_AYUV,.depth
> > = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1,
> > .has_alpha = true },
> > + { .format = DRM_FORMAT_XYUV,.depth
> > = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1,
> > .has_alpha = false },
>
> I would rather drop the .has_alpha = false, since these lines are
> long
> enough already.
Sounds reasonable.
>
> > };
> >
> > unsigned int i;
> > diff --git a/include/uapi/drm/drm_fourcc.h
> > b/include/uapi/drm/drm_fourcc.h
> > index d5e52350a3aa..c4eb69468eda 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -112,6 +112,7 @@ extern "C" {
> > #define DRM_FORMAT_VYUYfourcc_code('V', 'Y', 'U',
> > 'Y') /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
> >
> > #define DRM_FORMAT_AYUVfourcc_code('A', 'Y', 'U',
> > 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
> > +#define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U',
> > 'V') /* [31:0] X:Y:Cb:Cr 8:8:8:8 little endian */
> >
> > /*
> > * 2 plane RGB + A
> > --
> > 2.17.0
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
--
Best Regards,
Lisovskiy Stanislav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Expose retry count to per gen reset logic (rev3)
== Series Details ==
Series: series starting with [1/2] drm/i915: Expose retry count to per gen
reset logic (rev3)
URL : https://patchwork.freedesktop.org/series/48019/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4660 -> Patchwork_9928 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48019/revisions/3/mbox/
== Known issues ==
Here are the changes found in Patchwork_9928 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
fi-kbl-7560u: PASS -> DMESG-FAIL (fdo#106947, fdo#106560)
igt@drv_selftest@live_workarounds:
{fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#107292)
fi-cfl-s3: PASS -> DMESG-FAIL (fdo#107292)
fi-cnl-psr: PASS -> DMESG-FAIL (fdo#107292)
fi-kbl-7560u: PASS -> DMESG-FAIL (fdo#107292)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
{fi-byt-clapper}: FAIL (fdo#107362) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-skl-guc: FAIL (fdo#103191) -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
== Participating hosts (54 -> 49) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4660 -> Patchwork_9928
CI_DRM_4660: 9cd882754c1017e68ed9d2c8d57dc326530522fe @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9928: 43b30337b551eda6a92a54cd873bcd4256da34cd @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
43b30337b551 drm/i915: Force reset on unready engine
b6a14da11f65 drm/i915: Expose retry count to per gen reset logic
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9928/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Quoting Mika Kuoppala (2018-08-13 12:02:51) > Chris Wilson writes: > > > Quoting Mika Kuoppala (2018-08-13 11:42:42) > >> If engine reports that it is not ready for reset, we > >> give up. Evidence shows that forcing a per engine reset > >> on an engine which is not reporting to be ready for reset, > >> can bring it back into a working order. There is risk that > >> we corrupt the context image currently executing on that > >> engine. But that is a risk worth taking as if we unblock > >> the engine, we prevent a whole device wedging in a case > >> of full gpu reset. > >> > >> Reset individual engine even if it reports that it is not > >> prepared for reset, but only if we aim for full gpu reset > >> and not on first reset attempt. > >> > >> v2: force reset only on later attempts, readability (Chris) > >> v3: simplify with adequate caffeine levels (Chris) > >> > >> Cc: Chris Wilson > >> Signed-off-by: Mika Kuoppala > > Reviewed-by: Chris Wilson > > > > One last thing, you said you recalled one of the reasons for its > > existence was to prevent machine lockups on kbl. Is the recollection > > true? Do we want to leave a comment in case of fire? > > We got machine lockups if we did reset a non stopped, active > engine inside a batchbuffer. i915_stop_engines() arise from that > and we have a comment in intel_gpu_reset explaining it. That lockup > did apparently happen regardless of ready-to-reset ack. > > How I read it is that we got ready-to-reset acks on > active engines, which then died if we proceed. So this > patch should not make things worse as i915_stop_engines > have hold water. Ok, I still think a comment about the danger for last-chance with a reference back to i915_stop_engines will be useful for future archaeology. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Chris Wilson writes: > Quoting Mika Kuoppala (2018-08-13 11:42:42) >> If engine reports that it is not ready for reset, we >> give up. Evidence shows that forcing a per engine reset >> on an engine which is not reporting to be ready for reset, >> can bring it back into a working order. There is risk that >> we corrupt the context image currently executing on that >> engine. But that is a risk worth taking as if we unblock >> the engine, we prevent a whole device wedging in a case >> of full gpu reset. >> >> Reset individual engine even if it reports that it is not >> prepared for reset, but only if we aim for full gpu reset >> and not on first reset attempt. >> >> v2: force reset only on later attempts, readability (Chris) >> v3: simplify with adequate caffeine levels (Chris) >> >> Cc: Chris Wilson >> Signed-off-by: Mika Kuoppala > Reviewed-by: Chris Wilson > > One last thing, you said you recalled one of the reasons for its > existence was to prevent machine lockups on kbl. Is the recollection > true? Do we want to leave a comment in case of fire? We got machine lockups if we did reset a non stopped, active engine inside a batchbuffer. i915_stop_engines() arise from that and we have a comment in intel_gpu_reset explaining it. That lockup did apparently happen regardless of ready-to-reset ack. How I read it is that we got ready-to-reset acks on active engines, which then died if we proceed. So this patch should not make things worse as i915_stop_engines have hold water. -Mika *knocks on wood* ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v3] drm/i915: Add detection of changing of edid on between suspend and resume
On Fri, 2018-08-10 at 13:02 +, Lisovskiy, Stanislav wrote:
> On Thu, 2018-08-09 at 14:13 +0300, Gwan-gyeong Mun wrote:
> >
> > The hotplug detection routine of i915 uses
> > drm_helper_hpd_irq_event(). This
> > helper can detect changing of status of connector, but it can not
> > detect
> > changing of edid.
> >
> > Following scenario requires detection of changing of edid.
> >
> > 1) plug display device to a connector
> > 2) system suspend
> > 3) unplug 1)'s display device and plug the other display device to
> > a
> > connector
> > 4) system resume
> >
> > It adds edid check routine when a connector status still remains as
> > "connector_status_connected".
> >
> > v2: Add NULL check before comparing of EDIDs.
> >
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> > Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
> >
> > Signed-off-by: Gwan-gyeong Mun
> > ---
> > drivers/gpu/drm/i915/intel_hotplug.c | 84
> > +++-
> > 1 file changed, 83 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 648a13c6043c..965f2d771fc0 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -507,6 +507,88 @@ void intel_hpd_init(struct drm_i915_private
> > *dev_priv)
> > }
> > }
> >
> > +/**
> > + * intel_hpd_irq_event - hotplug processing
> > + * @dev: drm_device
> > + *
> > + * Drivers can use this function to run a detect cycle on all
> > connectors which
> > + * have the DRM_CONNECTOR_POLL_HPD flag set in their
> > member.
> > All other
> > + * connectors are ignored, which is useful to avoid reprobing
> > fixed
> > panels.
> > + *
> > + * This function is useful for drivers which can't or don't track
> > hotplug interrupts
> > + * for each connector. This function is based on
> > drm_helper_hpd_irq_event() helper
> > + * function and besides it adds edid check routine when a
> > connector
> > status still
> > + * remains as "connector_status_connected".
> > + *
> > + * Following scenario requires detection of changing of edid.
> > + * 1) plug display device to a connector
> > + * 2) system suspend
> > + * 3) unplug 1)'s display device and plug the other display
> > device
> > to a connector
> > + * 4) system resume
> > +
> > + * This function must be called from process context with no mode
> > + * setting locks held.
> > + *
> > + * Note that a connector can be both polled and probed from the
> > hotplug handler,
> > + * in case the hotplug interrupt is known to be unreliable.
> > + */
> > +static bool intel_hpd_irq_event(struct drm_device *dev)
> > +{
> > + struct drm_connector *connector;
> > + struct drm_connector_list_iter conn_iter;
> > + enum drm_connector_status old_status, cur_status;
> > + struct edid *old_edid;
> > + bool changed = false;
> > +
> > + if (!dev->mode_config.poll_enabled)
> > + return false;
> > +
> > + mutex_lock(>mode_config.mutex);
> > + drm_connector_list_iter_begin(dev, _iter);
> > + drm_for_each_connector_iter(connector, _iter) {
> > + /* Only handle HPD capable connectors. */
> > + if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> > + continue;
> > +
> > + old_status = connector->status;
> > + old_edid = to_intel_connector(connector)-
> > >
> > > detect_edid;
> > +
> > + cur_status = drm_helper_probe_detect(connector,
> > NULL, false);
> > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated
> > from
> > %s to %s\n",
> > + connector->base.id, connector->name,
> > + drm_get_connector_status_name(old_st
> > at
> > us),
> > + drm_get_connector_status_name(cur_st
> > at
> > us));
> > +
> > + if (old_status != cur_status)
> > + changed = true;
> > +
> > + /* Check changing of edid when a connector status
> > still remains
> > + * as "connector_status_connected".
> > + */
> > + if (old_status == cur_status &&
> > + cur_status == connector_status_connected) {
> > + struct edid *cur_edid =
> > to_intel_connector(connector)->detect_edid;
> > +
> > + if (!old_edid || !cur_edid)
> > + continue;
> > +
> > + if (memcmp(old_edid, cur_edid,
> > sizeof(*cur_edid))) {
> > + changed = true;
> > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]
> > edid updated\n",
> > + connector->base.id,
> > + connector->name);
> > + }
> > + }
> > + }
> > + drm_connector_list_iter_end(_iter);
> > +
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Quoting Mika Kuoppala (2018-08-13 11:42:42) > If engine reports that it is not ready for reset, we > give up. Evidence shows that forcing a per engine reset > on an engine which is not reporting to be ready for reset, > can bring it back into a working order. There is risk that > we corrupt the context image currently executing on that > engine. But that is a risk worth taking as if we unblock > the engine, we prevent a whole device wedging in a case > of full gpu reset. > > Reset individual engine even if it reports that it is not > prepared for reset, but only if we aim for full gpu reset > and not on first reset attempt. > > v2: force reset only on later attempts, readability (Chris) > v3: simplify with adequate caffeine levels (Chris) > > Cc: Chris Wilson > Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson One last thing, you said you recalled one of the reasons for its existence was to prevent machine lockups on kbl. Is the recollection true? Do we want to leave a comment in case of fire? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Bump priority of clean up work
Quoting Chris Wilson (2018-07-12 12:57:29) > We require that we keep the list of outstanding work short so that we do > not "leak" memory while pageflipping under stress. However that system > stress may delay kernel workers virtually indefinitely, which incurs the > pageflips stall and eventually hit a timeout waiting for the cleanup. > > Try to combat CPU starvation of our short-lived cleanup workers by > switching to a high priority workqueue. > > Testcase: igt/kms_cursor_legacy/all-pipes-torture-move > References: https://bugs.freedesktop.org/show_bug.cgi?id=107122 > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > --- > Not sure if highpri is enough to combat our RT torture... CI thinks it is. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
If engine reports that it is not ready for reset, we
give up. Evidence shows that forcing a per engine reset
on an engine which is not reporting to be ready for reset,
can bring it back into a working order. There is risk that
we corrupt the context image currently executing on that
engine. But that is a risk worth taking as if we unblock
the engine, we prevent a whole device wedging in a case
of full gpu reset.
Reset individual engine even if it reports that it is not
prepared for reset, but only if we aim for full gpu reset
and not on first reset attempt.
v2: force reset only on later attempts, readability (Chris)
v3: simplify with adequate caffeine levels (Chris)
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 36 ++---
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 027d14574bfa..7cba14ae82a1 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2085,7 +2085,7 @@ int __intel_wait_for_register(struct drm_i915_private
*dev_priv,
return ret;
}
-static int gen8_reset_engine_start(struct intel_engine_cs *engine)
+static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
int ret;
@@ -2105,7 +2105,7 @@ static int gen8_reset_engine_start(struct intel_engine_cs
*engine)
return ret;
}
-static void gen8_reset_engine_cancel(struct intel_engine_cs *engine)
+static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
@@ -2113,29 +2113,36 @@ static void gen8_reset_engine_cancel(struct
intel_engine_cs *engine)
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
+static int reset_engines(struct drm_i915_private *i915,
+unsigned int engine_mask,
+unsigned int retry)
+{
+ if (INTEL_GEN(i915) >= 11)
+ return gen11_reset_engines(i915, engine_mask);
+ else
+ return gen6_reset_engines(i915, engine_mask, retry);
+}
+
static int gen8_reset_engines(struct drm_i915_private *dev_priv,
unsigned int engine_mask,
unsigned int retry)
{
struct intel_engine_cs *engine;
+ const bool reset_non_ready = retry >= 1;
unsigned int tmp;
int ret;
for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
- if (gen8_reset_engine_start(engine)) {
- ret = -EIO;
- goto not_ready;
- }
+ ret = gen8_engine_reset_prepare(engine);
+ if (ret && !reset_non_ready)
+ goto skip_reset;
}
- if (INTEL_GEN(dev_priv) >= 11)
- ret = gen11_reset_engines(dev_priv, engine_mask);
- else
- ret = gen6_reset_engines(dev_priv, engine_mask, retry);
+ ret = reset_engines(dev_priv, engine_mask, retry);
-not_ready:
+skip_reset:
for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
- gen8_reset_engine_cancel(engine);
+ gen8_engine_reset_cancel(engine);
return ret;
}
@@ -2164,12 +2171,15 @@ static reset_func intel_get_gpu_reset(struct
drm_i915_private *dev_priv)
return NULL;
}
-int intel_gpu_reset(struct drm_i915_private *dev_priv, unsigned int
engine_mask)
+int intel_gpu_reset(struct drm_i915_private *dev_priv,
+ const unsigned int engine_mask)
{
reset_func reset = intel_get_gpu_reset(dev_priv);
unsigned int retry;
int ret;
+ GEM_BUG_ON(!engine_mask);
+
/*
* We want to perform per-engine reset from atomic context (e.g.
* softirq), which imposes the constraint that we cannot sleep.
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Bump priority of clean up work
== Series Details == Series: drm/i915: Bump priority of clean up work URL : https://patchwork.freedesktop.org/series/46390/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4660_full -> Patchwork_9927_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9927_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@shrink: shard-kbl: PASS -> INCOMPLETE (fdo#106886, fdo#103665) igt@kms_flip@flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363, fdo#102887) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) Possible fixes igt@gem_eio@reset-stress: shard-apl: FAIL -> PASS igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: INCOMPLETE (fdo#106023, fdo#103665) -> PASS igt@kms_cursor_legacy@all-pipes-torture-move: shard-snb: DMESG-WARN (fdo#107122) -> PASS shard-glk: DMESG-WARN (fdo#107122) -> PASS shard-apl: DMESG-WARN (fdo#107122) -> PASS shard-hsw: DMESG-WARN (fdo#107122) -> PASS fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#107122 https://bugs.freedesktop.org/show_bug.cgi?id=107122 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4660 -> Patchwork_9927 CI_DRM_4660: 9cd882754c1017e68ed9d2c8d57dc326530522fe @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9927: fef13c75ab23b211457cd09728351190018c11f7 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9927/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Quoting Mika Kuoppala (2018-08-13 10:58:10)
> Chris Wilson writes:
>
> > Quoting Mika Kuoppala (2018-08-13 09:18:07)
> >> Chris Wilson writes:
> >>
> >> > Quoting Mika Kuoppala (2018-08-10 15:00:36)
> >> >> If engine reports that it is not ready for reset, we
> >> >> give up. Evidence shows that forcing a per engine reset
> >> >> on an engine which is not reporting to be ready for reset,
> >> >> can bring it back into a working order. There is risk that
> >> >> we corrupt the context image currently executing on that
> >> >> engine. But that is a risk worth taking as if we unblock
> >> >> the engine, we prevent a whole device wedging in a case
> >> >> of full gpu reset.
> >> >>
> >> >> Reset individual engine even if it reports that it is not
> >> >> prepared for reset, but only if we aim for full gpu reset
> >> >> and not on first reset attempt.
> >> >>
> >> >> v2: force reset only on later attempts, readability (Chris)
> >> >>
> >> >> Cc: Chris Wilson
> >> >> Signed-off-by: Mika Kuoppala
> >> >> ---
> >> >> drivers/gpu/drm/i915/intel_uncore.c | 49 +++--
> >> >> 1 file changed, 39 insertions(+), 10 deletions(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> >> >> b/drivers/gpu/drm/i915/intel_uncore.c
> >> >> index 027d14574bfa..d24026839b17 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_uncore.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> >> >> @@ -2099,9 +2099,6 @@ static int gen8_reset_engine_start(struct
> >> >> intel_engine_cs *engine)
> >> >>RESET_CTL_READY_TO_RESET,
> >> >>700, 0,
> >> >>NULL);
> >> >> - if (ret)
> >> >> - DRM_ERROR("%s: reset request timeout\n", engine->name);
> >> >> -
> >> >> return ret;
> >> >> }
> >> >>
> >> >> @@ -2113,6 +2110,42 @@ static void gen8_reset_engine_cancel(struct
> >> >> intel_engine_cs *engine)
> >> >> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> >> >> }
> >> >>
> >> >> +static int reset_engines(struct drm_i915_private *i915,
> >> >> +unsigned int engine_mask,
> >> >> +unsigned int retry)
> >> >> +{
> >> >> + if (INTEL_GEN(i915) >= 11)
> >> >> + return gen11_reset_engines(i915, engine_mask);
> >> >> + else
> >> >> + return gen6_reset_engines(i915, engine_mask, retry);
> >> >> +}
> >> >> +
> >> >> +static int gen8_prepare_engine_for_reset(struct intel_engine_cs
> >> >> *engine,
> >> >> +unsigned int retry)
> >> >> +{
> >> >> + const bool force_reset_on_non_ready = retry >= 1;
> >> >> + int ret;
> >> >> +
> >> >> + ret = gen8_reset_engine_start(engine);
> >> >> +
> >> >> + if (ret && force_reset_on_non_ready) {
> >> >> + /*
> >> >> +* Try to unblock a single non-ready engine by risking
> >> >> +* context corruption.
> >> >> +*/
> >> >> + ret = reset_engines(engine->i915,
> >> >> + intel_engine_flag(engine),
> >> >> + retry);
> >> >> + if (!ret)
> >> >> + ret = gen8_reset_engine_start(engine);
> >> >> +
> >> >> + DRM_ERROR("%s: reset request timeout, forcing reset
> >> >> (%d)\n",
> >> >> + engine->name, ret);
> >> >
> >> > This looks dubious now ;)
> >> >
> >> > After the force you then do a reset in the caller. Twice the reset for
> >> > twice the unpreparedness.
> >>
> >> It is intentional. First we make the engine unstuck and then do
> >> a full cycle with ready to reset involved. I don't know if
> >> it really matters tho. It could be that the engine is already
> >> in pristine condition after first.
> >
> > Looks extremely weird way of going about it. If you want to do a double
> > reset after the first try fails, try
>
> The crux is: failing to reset or failing to preparing to reset.
> It is the ready for reset dance not working that we are trying to
> by doing extra per engine reset. So trying in the higher lever,
> again with the same preconditions would not help.
No, you made it into two.
The first is that we don't want to skip the reset even if we fail to
prepare (as a last resort).
The second is that you added the double reset.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for Reviewed perf cleanups
== Series Details == Series: Reviewed perf cleanups URL : https://patchwork.freedesktop.org/series/48100/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4660_full -> Patchwork_9926_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9926_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@fence-restore-untiled: shard-glk: PASS -> FAIL (fdo#103375) igt@drv_suspend@shrink: shard-apl: PASS -> FAIL (fdo#106886) igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) Possible fixes igt@gem_eio@reset-stress: shard-apl: FAIL -> PASS fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4660 -> Patchwork_9926 CI_DRM_4660: 9cd882754c1017e68ed9d2c8d57dc326530522fe @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9926: fbf730a0a4a717a5f3a151e398f50ca6b9de1757 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9926/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Chris Wilson writes:
> Quoting Mika Kuoppala (2018-08-13 09:18:07)
>> Chris Wilson writes:
>>
>> > Quoting Mika Kuoppala (2018-08-10 15:00:36)
>> >> If engine reports that it is not ready for reset, we
>> >> give up. Evidence shows that forcing a per engine reset
>> >> on an engine which is not reporting to be ready for reset,
>> >> can bring it back into a working order. There is risk that
>> >> we corrupt the context image currently executing on that
>> >> engine. But that is a risk worth taking as if we unblock
>> >> the engine, we prevent a whole device wedging in a case
>> >> of full gpu reset.
>> >>
>> >> Reset individual engine even if it reports that it is not
>> >> prepared for reset, but only if we aim for full gpu reset
>> >> and not on first reset attempt.
>> >>
>> >> v2: force reset only on later attempts, readability (Chris)
>> >>
>> >> Cc: Chris Wilson
>> >> Signed-off-by: Mika Kuoppala
>> >> ---
>> >> drivers/gpu/drm/i915/intel_uncore.c | 49 +++--
>> >> 1 file changed, 39 insertions(+), 10 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> >> b/drivers/gpu/drm/i915/intel_uncore.c
>> >> index 027d14574bfa..d24026839b17 100644
>> >> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> >> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> >> @@ -2099,9 +2099,6 @@ static int gen8_reset_engine_start(struct
>> >> intel_engine_cs *engine)
>> >>RESET_CTL_READY_TO_RESET,
>> >>700, 0,
>> >>NULL);
>> >> - if (ret)
>> >> - DRM_ERROR("%s: reset request timeout\n", engine->name);
>> >> -
>> >> return ret;
>> >> }
>> >>
>> >> @@ -2113,6 +2110,42 @@ static void gen8_reset_engine_cancel(struct
>> >> intel_engine_cs *engine)
>> >> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
>> >> }
>> >>
>> >> +static int reset_engines(struct drm_i915_private *i915,
>> >> +unsigned int engine_mask,
>> >> +unsigned int retry)
>> >> +{
>> >> + if (INTEL_GEN(i915) >= 11)
>> >> + return gen11_reset_engines(i915, engine_mask);
>> >> + else
>> >> + return gen6_reset_engines(i915, engine_mask, retry);
>> >> +}
>> >> +
>> >> +static int gen8_prepare_engine_for_reset(struct intel_engine_cs *engine,
>> >> +unsigned int retry)
>> >> +{
>> >> + const bool force_reset_on_non_ready = retry >= 1;
>> >> + int ret;
>> >> +
>> >> + ret = gen8_reset_engine_start(engine);
>> >> +
>> >> + if (ret && force_reset_on_non_ready) {
>> >> + /*
>> >> +* Try to unblock a single non-ready engine by risking
>> >> +* context corruption.
>> >> +*/
>> >> + ret = reset_engines(engine->i915,
>> >> + intel_engine_flag(engine),
>> >> + retry);
>> >> + if (!ret)
>> >> + ret = gen8_reset_engine_start(engine);
>> >> +
>> >> + DRM_ERROR("%s: reset request timeout, forcing reset
>> >> (%d)\n",
>> >> + engine->name, ret);
>> >
>> > This looks dubious now ;)
>> >
>> > After the force you then do a reset in the caller. Twice the reset for
>> > twice the unpreparedness.
>>
>> It is intentional. First we make the engine unstuck and then do
>> a full cycle with ready to reset involved. I don't know if
>> it really matters tho. It could be that the engine is already
>> in pristine condition after first.
>
> Looks extremely weird way of going about it. If you want to do a double
> reset after the first try fails, try
The crux is: failing to reset or failing to preparing to reset.
It is the ready for reset dance not working that we are trying to
by doing extra per engine reset. So trying in the higher lever,
again with the same preconditions would not help.
-Mika
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c
> b/drivers/gpu/drm/i915/i915_reset.c
> index 4e5826045cbf..6fe137d7d455 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -450,6 +450,8 @@ int intel_gpu_reset(struct drm_i915_private *i915,
> unsigned int engine_mask)
> GEM_TRACE("engine_mask=%x\n", engine_mask);
> preempt_disable();
> ret = reset(i915, engine_mask);
> + if (retry > 0 && !ret) /* Double check reset worked */
> + ret = reset(i915, engine_mask);
> preempt_enable();
> }
> if (ret != -ETIMEDOUT || engine_mask != ALL_ENGINES)
>
> as a separate patch.
>
> P.S. you really need to review the i915_reset.c patch ;)
> -Chris
___
Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option
Quoting Tvrtko Ursulin (2018-08-08 15:56:01) > On 08/08/2018 13:42, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-08-08 13:13:08) > This is true, no disagreement. My point simply was that we can provide > this info easily to anyone. There is a little bit of analogy with perf > scheduler tracing/map etc. > > > I don't see any value in giving the information away, just the cost. If > > you can convince Joonas of its merit, and if we can define just exactly > > what ABI it constitutes, then I'd be happy to be the one who says "I > > told you so" in the future for a change. > > I think Joonas was okay in principle that we soft-commit to _trying_ to > keep _some_ tracepoint stable-ish (where it makes sense and after some > discussion for each) if IGT also materializes which auto-pings us (via > CI) when we break one of them. But I may be misremembering so Joonas > please comment. Currently gpuvis, using these, seems to be only packaged in one AUR repo, and they do make a not in the wiki how you need to configure kernel for debugging. And there's been no apparent demand for them to have it in stock kernel. And even when we do get demand for having gpuvis or another tool working from vanilla kernel, tracepoints being a rather tricky subject, I would start the discussion by going through alternative means of providing the information the tool needs and considering those. So lets still keep this option as it was introduced. The whole "tracepoints as stable uAPI" idea is a can of worms which is only dug into when other options are exhausted. Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] igt/perf_pmu: Aim for a fixed number of iterations for calibrating accuracy
On 10/08/2018 14:25, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-08-09 12:54:41)
On 08/08/2018 15:59, Chris Wilson wrote:
Our observation is that the systematic error is proportional to the
number of iterations we perform; the suspicion is that it directly
correlates with the number of sleeps. Reduce the number of iterations,
to try and keep the error in check.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
tests/perf_pmu.c | 34 +-
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 9a20abb6b..5a26d5272 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1521,14 +1521,13 @@ static void __rearm_spin_batch(igt_spin_t *spin)
static void
accuracy(int gem_fd, const struct intel_execution_engine2 *e,
- unsigned long target_busy_pct)
+ unsigned long target_busy_pct,
+ unsigned long target_iters)
{
- unsigned long busy_us = 1 - 100 * (1 + abs(50 - target_busy_pct));
- unsigned long idle_us = 100 * (busy_us - target_busy_pct *
- busy_us / 100) / target_busy_pct;
const unsigned long min_test_us = 1e6;
- const unsigned long pwm_calibration_us = min_test_us;
- const unsigned long test_us = min_test_us;
+ unsigned long pwm_calibration_us;
+ unsigned long test_us;
+ unsigned long cycle_us, busy_us, idle_us;
double busy_r, expected;
uint64_t val[2];
uint64_t ts[2];
@@ -1538,18 +1537,27 @@ accuracy(int gem_fd, const struct
intel_execution_engine2 *e,
/* Sampling platforms cannot reach the high accuracy criteria. */
igt_require(gem_has_execlists(gem_fd));
- while (idle_us < 2500) {
+ /* Aim for approximately 100 iterations for calibration */
+ cycle_us = min_test_us / target_iters;
+ busy_us = cycle_us * target_busy_pct / 100;
+ idle_us = cycle_us - busy_us;
2% load, 1s / 10 iters
cycles_us = 100ms
busy_us = 2ms
idle_us = 98ms
...
+
+ while (idle_us < 2500 || busy_us < 2500) {
busy_us *= 2;
idle_us *= 2;
...
busy_us = 4ms
idle_us = 196ms
Currently it is 250ms per 98:2 cycle and about 20ms per 50:50 cycle. So
we are only doing 4 and 50 iterations respectively.
10 cycles is strictly an improvement :-p
Hmm indeed. It seems I misremembered how it works. I'll re-read your
patches.
Regards,
Tvrtko
___
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: Bump priority of clean up work
== Series Details ==
Series: drm/i915: Bump priority of clean up work
URL : https://patchwork.freedesktop.org/series/46390/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4660 -> Patchwork_9927 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46390/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9927 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_workarounds:
{fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#107292)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: FAIL (fdo#103167) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
{fi-byt-clapper}: FAIL (fdo#107362) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-skl-guc: FAIL (fdo#103191) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
== Participating hosts (54 -> 49) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4660 -> Patchwork_9927
CI_DRM_4660: 9cd882754c1017e68ed9d2c8d57dc326530522fe @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9927: fef13c75ab23b211457cd09728351190018c11f7 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
fef13c75ab23 drm/i915: Bump priority of clean up work
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9927/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 2/2] drm/i915/perf: reuse intel_lrc ctx regs macro
Quoting Tvrtko Ursulin (2018-08-13 10:11:44)
>
> On 13/08/2018 09:16, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-08-13 09:02:18)
> >> From: Lionel Landwerlin
> >>
> >> Abstract the context image access a bit.
> >>
> >> Signed-off-by: Lionel Landwerlin
> >> Reviewed-by: Tvrtko Ursulin
> >> ---
> >> drivers/gpu/drm/i915/i915_perf.c | 34 +++-
> >> 1 file changed, 16 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_perf.c
> >> b/drivers/gpu/drm/i915/i915_perf.c
> >> index 49597cf31707..ccb20230df2c 100644
> >> --- a/drivers/gpu/drm/i915/i915_perf.c
> >> +++ b/drivers/gpu/drm/i915/i915_perf.c
> >> @@ -210,6 +210,7 @@
> >> #include "i915_oa_cflgt3.h"
> >> #include "i915_oa_cnl.h"
> >> #include "i915_oa_icl.h"
> >> +#include "intel_lrc_reg.h"
> >>
> >> /* HW requires this to be a power of two, between 128k and 16M, though
> >> driver
> >>* is currently generally designed assuming the largest 16M size is used
> >> such
> >> @@ -1636,27 +1637,25 @@ static void gen8_update_reg_state_unlocked(struct
> >> i915_gem_context *ctx,
> >> u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> >> u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> >> /* The MMIO offsets for Flex EU registers aren't contiguous */
> >> - u32 flex_mmio[] = {
> >> - i915_mmio_reg_offset(EU_PERF_CNTL0),
> >> - i915_mmio_reg_offset(EU_PERF_CNTL1),
> >> - i915_mmio_reg_offset(EU_PERF_CNTL2),
> >> - i915_mmio_reg_offset(EU_PERF_CNTL3),
> >> - i915_mmio_reg_offset(EU_PERF_CNTL4),
> >> - i915_mmio_reg_offset(EU_PERF_CNTL5),
> >> - i915_mmio_reg_offset(EU_PERF_CNTL6),
> >> + i915_reg_t flex_regs[] = {
> >> + EU_PERF_CNTL0,
> >> + EU_PERF_CNTL1,
> >> + EU_PERF_CNTL2,
> >> + EU_PERF_CNTL3,
> >> + EU_PERF_CNTL4,
> >> + EU_PERF_CNTL5,
> >> + EU_PERF_CNTL6,
> >> };
> >> int i;
> >>
> >> - reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
> >> - reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
> >> - GEN8_OA_TIMER_PERIOD_SHIFT) |
> >> -(dev_priv->perf.oa.periodic ?
> >> - GEN8_OA_TIMER_ENABLE : 0) |
> >> -GEN8_OA_COUNTER_RESUME;
> >> + CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
> >> + (dev_priv->perf.oa.period_exponent <<
> >> GEN8_OA_TIMER_PERIOD_SHIFT) |
> >> + (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> >> + GEN8_OA_COUNTER_RESUME);
> >
> > I'll be honest but, I don't think it's CTX_REG() that helps improve the
> > readability here.
> >
> > The really odd part is that this sticks itself into a bare part of the
> > register state not surrounded by any LRI and after a BB_END. This
> > routine can only work for established contexts, it should not work for
> > execlists_init_reg_state.
>
> Unless I am missing something change is completely mechanical, so any
> question marks you have are already there, right? What do you suggest is
> the action here?
Sure, the only thing I question of this patch itself is whether
CTX_REG() is simply too much horrible obfuscating magic.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 2/2] drm/i915/perf: reuse intel_lrc ctx regs macro
On 13/08/2018 09:16, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-08-13 09:02:18)
From: Lionel Landwerlin
Abstract the context image access a bit.
Signed-off-by: Lionel Landwerlin
Reviewed-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_perf.c | 34 +++-
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 49597cf31707..ccb20230df2c 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -210,6 +210,7 @@
#include "i915_oa_cflgt3.h"
#include "i915_oa_cnl.h"
#include "i915_oa_icl.h"
+#include "intel_lrc_reg.h"
/* HW requires this to be a power of two, between 128k and 16M, though driver
* is currently generally designed assuming the largest 16M size is used such
@@ -1636,27 +1637,25 @@ static void gen8_update_reg_state_unlocked(struct
i915_gem_context *ctx,
u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
/* The MMIO offsets for Flex EU registers aren't contiguous */
- u32 flex_mmio[] = {
- i915_mmio_reg_offset(EU_PERF_CNTL0),
- i915_mmio_reg_offset(EU_PERF_CNTL1),
- i915_mmio_reg_offset(EU_PERF_CNTL2),
- i915_mmio_reg_offset(EU_PERF_CNTL3),
- i915_mmio_reg_offset(EU_PERF_CNTL4),
- i915_mmio_reg_offset(EU_PERF_CNTL5),
- i915_mmio_reg_offset(EU_PERF_CNTL6),
+ i915_reg_t flex_regs[] = {
+ EU_PERF_CNTL0,
+ EU_PERF_CNTL1,
+ EU_PERF_CNTL2,
+ EU_PERF_CNTL3,
+ EU_PERF_CNTL4,
+ EU_PERF_CNTL5,
+ EU_PERF_CNTL6,
};
int i;
- reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
- reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
- GEN8_OA_TIMER_PERIOD_SHIFT) |
-(dev_priv->perf.oa.periodic ?
- GEN8_OA_TIMER_ENABLE : 0) |
-GEN8_OA_COUNTER_RESUME;
+ CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
+ (dev_priv->perf.oa.period_exponent <<
GEN8_OA_TIMER_PERIOD_SHIFT) |
+ (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
+ GEN8_OA_COUNTER_RESUME);
I'll be honest but, I don't think it's CTX_REG() that helps improve the
readability here.
The really odd part is that this sticks itself into a bare part of the
register state not surrounded by any LRI and after a BB_END. This
routine can only work for established contexts, it should not work for
execlists_init_reg_state.
Unless I am missing something change is completely mechanical, so any
question marks you have are already there, right? What do you suggest is
the action here?
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Reviewed perf cleanups
== Series Details ==
Series: Reviewed perf cleanups
URL : https://patchwork.freedesktop.org/series/48100/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4660 -> Patchwork_9926 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48100/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9926 that come from known issues:
=== IGT changes ===
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
{fi-byt-clapper}: FAIL (fdo#107362) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-skl-guc: FAIL (fdo#103191) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
== Participating hosts (54 -> 49) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4660 -> Patchwork_9926
CI_DRM_4660: 9cd882754c1017e68ed9d2c8d57dc326530522fe @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9926: fbf730a0a4a717a5f3a151e398f50ca6b9de1757 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
fbf730a0a4a7 drm/i915/perf: reuse intel_lrc ctx regs macro
fecf631c038e drm/i915/perf: simplify configure all context function
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9926/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Quoting Mika Kuoppala (2018-08-13 09:18:07)
> Chris Wilson writes:
>
> > Quoting Mika Kuoppala (2018-08-10 15:00:36)
> >> If engine reports that it is not ready for reset, we
> >> give up. Evidence shows that forcing a per engine reset
> >> on an engine which is not reporting to be ready for reset,
> >> can bring it back into a working order. There is risk that
> >> we corrupt the context image currently executing on that
> >> engine. But that is a risk worth taking as if we unblock
> >> the engine, we prevent a whole device wedging in a case
> >> of full gpu reset.
> >>
> >> Reset individual engine even if it reports that it is not
> >> prepared for reset, but only if we aim for full gpu reset
> >> and not on first reset attempt.
> >>
> >> v2: force reset only on later attempts, readability (Chris)
> >>
> >> Cc: Chris Wilson
> >> Signed-off-by: Mika Kuoppala
> >> ---
> >> drivers/gpu/drm/i915/intel_uncore.c | 49 +++--
> >> 1 file changed, 39 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> >> b/drivers/gpu/drm/i915/intel_uncore.c
> >> index 027d14574bfa..d24026839b17 100644
> >> --- a/drivers/gpu/drm/i915/intel_uncore.c
> >> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> >> @@ -2099,9 +2099,6 @@ static int gen8_reset_engine_start(struct
> >> intel_engine_cs *engine)
> >>RESET_CTL_READY_TO_RESET,
> >>700, 0,
> >>NULL);
> >> - if (ret)
> >> - DRM_ERROR("%s: reset request timeout\n", engine->name);
> >> -
> >> return ret;
> >> }
> >>
> >> @@ -2113,6 +2110,42 @@ static void gen8_reset_engine_cancel(struct
> >> intel_engine_cs *engine)
> >> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> >> }
> >>
> >> +static int reset_engines(struct drm_i915_private *i915,
> >> +unsigned int engine_mask,
> >> +unsigned int retry)
> >> +{
> >> + if (INTEL_GEN(i915) >= 11)
> >> + return gen11_reset_engines(i915, engine_mask);
> >> + else
> >> + return gen6_reset_engines(i915, engine_mask, retry);
> >> +}
> >> +
> >> +static int gen8_prepare_engine_for_reset(struct intel_engine_cs *engine,
> >> +unsigned int retry)
> >> +{
> >> + const bool force_reset_on_non_ready = retry >= 1;
> >> + int ret;
> >> +
> >> + ret = gen8_reset_engine_start(engine);
> >> +
> >> + if (ret && force_reset_on_non_ready) {
> >> + /*
> >> +* Try to unblock a single non-ready engine by risking
> >> +* context corruption.
> >> +*/
> >> + ret = reset_engines(engine->i915,
> >> + intel_engine_flag(engine),
> >> + retry);
> >> + if (!ret)
> >> + ret = gen8_reset_engine_start(engine);
> >> +
> >> + DRM_ERROR("%s: reset request timeout, forcing reset
> >> (%d)\n",
> >> + engine->name, ret);
> >
> > This looks dubious now ;)
> >
> > After the force you then do a reset in the caller. Twice the reset for
> > twice the unpreparedness.
>
> It is intentional. First we make the engine unstuck and then do
> a full cycle with ready to reset involved. I don't know if
> it really matters tho. It could be that the engine is already
> in pristine condition after first.
Looks extremely weird way of going about it. If you want to do a double
reset after the first try fails, try
diff --git a/drivers/gpu/drm/i915/i915_reset.c
b/drivers/gpu/drm/i915/i915_reset.c
index 4e5826045cbf..6fe137d7d455 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -450,6 +450,8 @@ int intel_gpu_reset(struct drm_i915_private *i915, unsigned
int engine_mask)
GEM_TRACE("engine_mask=%x\n", engine_mask);
preempt_disable();
ret = reset(i915, engine_mask);
+ if (retry > 0 && !ret) /* Double check reset worked */
+ ret = reset(i915, engine_mask);
preempt_enable();
}
if (ret != -ETIMEDOUT || engine_mask != ALL_ENGINES)
as a separate patch.
P.S. you really need to review the i915_reset.c patch ;)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Chris Wilson writes:
> Quoting Mika Kuoppala (2018-08-10 15:00:36)
>> If engine reports that it is not ready for reset, we
>> give up. Evidence shows that forcing a per engine reset
>> on an engine which is not reporting to be ready for reset,
>> can bring it back into a working order. There is risk that
>> we corrupt the context image currently executing on that
>> engine. But that is a risk worth taking as if we unblock
>> the engine, we prevent a whole device wedging in a case
>> of full gpu reset.
>>
>> Reset individual engine even if it reports that it is not
>> prepared for reset, but only if we aim for full gpu reset
>> and not on first reset attempt.
>>
>> v2: force reset only on later attempts, readability (Chris)
>>
>> Cc: Chris Wilson
>> Signed-off-by: Mika Kuoppala
>> ---
>> drivers/gpu/drm/i915/intel_uncore.c | 49 +++--
>> 1 file changed, 39 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index 027d14574bfa..d24026839b17 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -2099,9 +2099,6 @@ static int gen8_reset_engine_start(struct
>> intel_engine_cs *engine)
>>RESET_CTL_READY_TO_RESET,
>>700, 0,
>>NULL);
>> - if (ret)
>> - DRM_ERROR("%s: reset request timeout\n", engine->name);
>> -
>> return ret;
>> }
>>
>> @@ -2113,6 +2110,42 @@ static void gen8_reset_engine_cancel(struct
>> intel_engine_cs *engine)
>> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
>> }
>>
>> +static int reset_engines(struct drm_i915_private *i915,
>> +unsigned int engine_mask,
>> +unsigned int retry)
>> +{
>> + if (INTEL_GEN(i915) >= 11)
>> + return gen11_reset_engines(i915, engine_mask);
>> + else
>> + return gen6_reset_engines(i915, engine_mask, retry);
>> +}
>> +
>> +static int gen8_prepare_engine_for_reset(struct intel_engine_cs *engine,
>> +unsigned int retry)
>> +{
>> + const bool force_reset_on_non_ready = retry >= 1;
>> + int ret;
>> +
>> + ret = gen8_reset_engine_start(engine);
>> +
>> + if (ret && force_reset_on_non_ready) {
>> + /*
>> +* Try to unblock a single non-ready engine by risking
>> +* context corruption.
>> +*/
>> + ret = reset_engines(engine->i915,
>> + intel_engine_flag(engine),
>> + retry);
>> + if (!ret)
>> + ret = gen8_reset_engine_start(engine);
>> +
>> + DRM_ERROR("%s: reset request timeout, forcing reset (%d)\n",
>> + engine->name, ret);
>
> This looks dubious now ;)
>
> After the force you then do a reset in the caller. Twice the reset for
> twice the unpreparedness.
It is intentional. First we make the engine unstuck and then do
a full cycle with ready to reset involved. I don't know if
it really matters tho. It could be that the engine is already
in pristine condition after first.
I considered the engine reset here to only be the hammer,
and then the reset afterwards to be the validation.
-Mika
>
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int gen8_reset_engines(struct drm_i915_private *dev_priv,
>> unsigned int engine_mask,
>> unsigned int retry)
>> @@ -2122,16 +2155,12 @@ static int gen8_reset_engines(struct
>> drm_i915_private *dev_priv,
>> int ret;
>>
>> for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
>> - if (gen8_reset_engine_start(engine)) {
>> - ret = -EIO;
>> + ret = gen8_prepare_engine_for_reset(engine, retry);
>> + if (ret)
>
> if (ret && !force_reset_unread)
>
>> goto not_ready;
>> - }
>> }
>>
>> - if (INTEL_GEN(dev_priv) >= 11)
>> - ret = gen11_reset_engines(dev_priv, engine_mask);
>> - else
>> - ret = gen6_reset_engines(dev_priv, engine_mask, retry);
>> + ret = reset_engines(dev_priv, engine_mask, retry);
>>
>> not_ready:
>> for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
>> --
>> 2.17.1
>>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 2/2] drm/i915/perf: reuse intel_lrc ctx regs macro
Quoting Tvrtko Ursulin (2018-08-13 09:02:18)
> From: Lionel Landwerlin
>
> Abstract the context image access a bit.
>
> Signed-off-by: Lionel Landwerlin
> Reviewed-by: Tvrtko Ursulin
> ---
> drivers/gpu/drm/i915/i915_perf.c | 34 +++-
> 1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c
> b/drivers/gpu/drm/i915/i915_perf.c
> index 49597cf31707..ccb20230df2c 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -210,6 +210,7 @@
> #include "i915_oa_cflgt3.h"
> #include "i915_oa_cnl.h"
> #include "i915_oa_icl.h"
> +#include "intel_lrc_reg.h"
>
> /* HW requires this to be a power of two, between 128k and 16M, though driver
> * is currently generally designed assuming the largest 16M size is used such
> @@ -1636,27 +1637,25 @@ static void gen8_update_reg_state_unlocked(struct
> i915_gem_context *ctx,
> u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> /* The MMIO offsets for Flex EU registers aren't contiguous */
> - u32 flex_mmio[] = {
> - i915_mmio_reg_offset(EU_PERF_CNTL0),
> - i915_mmio_reg_offset(EU_PERF_CNTL1),
> - i915_mmio_reg_offset(EU_PERF_CNTL2),
> - i915_mmio_reg_offset(EU_PERF_CNTL3),
> - i915_mmio_reg_offset(EU_PERF_CNTL4),
> - i915_mmio_reg_offset(EU_PERF_CNTL5),
> - i915_mmio_reg_offset(EU_PERF_CNTL6),
> + i915_reg_t flex_regs[] = {
> + EU_PERF_CNTL0,
> + EU_PERF_CNTL1,
> + EU_PERF_CNTL2,
> + EU_PERF_CNTL3,
> + EU_PERF_CNTL4,
> + EU_PERF_CNTL5,
> + EU_PERF_CNTL6,
> };
> int i;
>
> - reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
> - reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
> - GEN8_OA_TIMER_PERIOD_SHIFT) |
> -(dev_priv->perf.oa.periodic ?
> - GEN8_OA_TIMER_ENABLE : 0) |
> -GEN8_OA_COUNTER_RESUME;
> + CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
> + (dev_priv->perf.oa.period_exponent <<
> GEN8_OA_TIMER_PERIOD_SHIFT) |
> + (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> + GEN8_OA_COUNTER_RESUME);
I'll be honest but, I don't think it's CTX_REG() that helps improve the
readability here.
The really odd part is that this sticks itself into a bare part of the
register state not surrounded by any LRI and after a BB_END. This
routine can only work for established contexts, it should not work for
execlists_init_reg_state.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 1/2] drm/i915/perf: simplify configure all context function
From: Lionel Landwerlin
We don't need any special treatment on error so just return as soon as
possible.
Signed-off-by: Lionel Landwerlin
Reviewed-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_perf.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 0376338d1f8d..49597cf31707 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1819,7 +1819,7 @@ static int gen8_configure_all_contexts(struct
drm_i915_private *dev_priv,
/* Switch away from any user context. */
ret = gen8_switch_to_updated_kernel_context(dev_priv, oa_config);
if (ret)
- goto out;
+ return ret;
/*
* The OA register config is setup through the context image. This image
@@ -1838,7 +1838,7 @@ static int gen8_configure_all_contexts(struct
drm_i915_private *dev_priv,
wait_flags,
MAX_SCHEDULE_TIMEOUT);
if (ret)
- goto out;
+ return ret;
/* Update all contexts now that we've stalled the submission. */
list_for_each_entry(ctx, _priv->contexts.list, link) {
@@ -1850,10 +1850,8 @@ static int gen8_configure_all_contexts(struct
drm_i915_private *dev_priv,
continue;
regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
- if (IS_ERR(regs)) {
- ret = PTR_ERR(regs);
- goto out;
- }
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
ce->state->obj->mm.dirty = true;
regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
@@ -1863,7 +1861,6 @@ static int gen8_configure_all_contexts(struct
drm_i915_private *dev_priv,
i915_gem_object_unpin_map(ce->state->obj);
}
- out:
return ret;
}
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 0/2] Reviewed perf cleanups
From: Tvrtko Ursulin Two reviewed simple cleanup patches, extracted from dynamic SSEU series. Lionel Landwerlin (2): drm/i915/perf: simplify configure all context function drm/i915/perf: reuse intel_lrc ctx regs macro drivers/gpu/drm/i915/i915_perf.c | 45 ++-- 1 file changed, 20 insertions(+), 25 deletions(-) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 2/2] drm/i915/perf: reuse intel_lrc ctx regs macro
From: Lionel Landwerlin
Abstract the context image access a bit.
Signed-off-by: Lionel Landwerlin
Reviewed-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_perf.c | 34 +++-
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 49597cf31707..ccb20230df2c 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -210,6 +210,7 @@
#include "i915_oa_cflgt3.h"
#include "i915_oa_cnl.h"
#include "i915_oa_icl.h"
+#include "intel_lrc_reg.h"
/* HW requires this to be a power of two, between 128k and 16M, though driver
* is currently generally designed assuming the largest 16M size is used such
@@ -1636,27 +1637,25 @@ static void gen8_update_reg_state_unlocked(struct
i915_gem_context *ctx,
u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
/* The MMIO offsets for Flex EU registers aren't contiguous */
- u32 flex_mmio[] = {
- i915_mmio_reg_offset(EU_PERF_CNTL0),
- i915_mmio_reg_offset(EU_PERF_CNTL1),
- i915_mmio_reg_offset(EU_PERF_CNTL2),
- i915_mmio_reg_offset(EU_PERF_CNTL3),
- i915_mmio_reg_offset(EU_PERF_CNTL4),
- i915_mmio_reg_offset(EU_PERF_CNTL5),
- i915_mmio_reg_offset(EU_PERF_CNTL6),
+ i915_reg_t flex_regs[] = {
+ EU_PERF_CNTL0,
+ EU_PERF_CNTL1,
+ EU_PERF_CNTL2,
+ EU_PERF_CNTL3,
+ EU_PERF_CNTL4,
+ EU_PERF_CNTL5,
+ EU_PERF_CNTL6,
};
int i;
- reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
- reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
- GEN8_OA_TIMER_PERIOD_SHIFT) |
-(dev_priv->perf.oa.periodic ?
- GEN8_OA_TIMER_ENABLE : 0) |
-GEN8_OA_COUNTER_RESUME;
+ CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
+ (dev_priv->perf.oa.period_exponent <<
GEN8_OA_TIMER_PERIOD_SHIFT) |
+ (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
+ GEN8_OA_COUNTER_RESUME);
- for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
+ for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
u32 state_offset = ctx_flexeu0 + i * 2;
- u32 mmio = flex_mmio[i];
+ u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
/*
* This arbitrary default will select the 'EU FPU0 Pipeline
@@ -1676,8 +1675,7 @@ static void gen8_update_reg_state_unlocked(struct
i915_gem_context *ctx,
}
}
- reg_state[state_offset] = mmio;
- reg_state[state_offset+1] = value;
+ CTX_REG(reg_state, state_offset, flex_regs[i], value);
}
}
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
