[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
== Series Details == Series: series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc URL : https://patchwork.freedesktop.org/series/48695/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4704_full -> Patchwork_10015_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10015_full that come from known issues: === IGT changes === Issues hit igt@gem_ctx_isolation@bcs0-s3: shard-kbl: PASS -> INCOMPLETE (fdo#103665) igt@gem_exec_await@wide-contexts: shard-kbl: PASS -> FAIL (fdo#105900) igt@gem_exec_big: shard-hsw: PASS -> INCOMPLETE (fdo#103540) Possible fixes igt@kms_cursor_legacy@cursor-vs-flip-toggle: shard-hsw: FAIL (fdo#103355) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS igt@kms_vblank@pipe-a-ts-continuation-suspend: shard-hsw: FAIL (fdo#104894) -> PASS fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894 fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900 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_4704 -> Patchwork_10015 CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10015: 7855b6acf8946464612d3eb13a6a65aade8612dd @ 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_10015/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
== Series Details ==
Series: series starting with [v3,1/2] drm: drm/i915: Add connector property to
limit max bpc
URL : https://patchwork.freedesktop.org/series/48695/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4704 -> Patchwork_10015 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48695/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10015:
=== IGT changes ===
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-WARN -> DMESG-FAIL
== Known issues ==
Here are the changes found in Patchwork_10015 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
fi-kbl-guc: PASS -> DMESG-FAIL (fdo#106947)
igt@kms_chamelium@dp-edid-read:
fi-kbl-7500u: PASS -> FAIL (fdo#103841)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362, fdo#103191)
Possible fixes
igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
fi-skl-guc: FAIL (fdo#103191) -> PASS +1
{fi-byt-clapper}: FAIL (fdo#107362, fdo#103191) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-skl-6260u: INCOMPLETE (fdo#104108) -> PASS
{igt@pm_rpm@module-reload}:
fi-cnl-psr: WARN (fdo#107602) -> 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#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
== 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_4704 -> Patchwork_10015
CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10015: 7855b6acf8946464612d3eb13a6a65aade8612dd @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
7855b6acf894 drm/i915: Allow "max bpc" property to limit pipe_bpp
cf2501ba38fa drm: drm/i915: Add connector property to limit max bpc
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10015/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
== Series Details ==
Series: series starting with [v3,1/2] drm: drm/i915: Add connector property to
limit max bpc
URL : https://patchwork.freedesktop.org/series/48695/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
cf2501ba38fa drm: drm/i915: Add connector property to limit max bpc
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#17:
connector property to make sure that bpc does not exceed the configured value.
-:125: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!prop"
#125: FILE: drivers/gpu/drm/i915/intel_modes.c:146:
+ if (prop == NULL) {
-:127: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!prop"
#127: FILE: drivers/gpu/drm/i915/intel_modes.c:148:
+ if (prop == NULL)
total: 0 errors, 1 warnings, 2 checks, 102 lines checked
7855b6acf894 drm/i915: Allow "max bpc" property to limit pipe_bpp
-:30: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#30: FILE: drivers/gpu/drm/i915/intel_display.c:10789:
+connected_sink_max_bpp(struct drm_connector_state *conn_state,
+struct intel_crtc_state *pipe_config)
-:35: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#35: FILE: drivers/gpu/drm/i915/intel_display.c:10794:
+ pipe_config->pipe_bpp = 8*3;
^
-:39: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#39: FILE: drivers/gpu/drm/i915/intel_display.c:10798:
+ pipe_config->pipe_bpp = 10*3;
^
-:42: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#42: FILE: drivers/gpu/drm/i915/intel_display.c:10801:
+ pipe_config->pipe_bpp = 12*3;
^
total: 0 errors, 0 warnings, 4 checks, 37 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc
At times 12bpc HDMI cannot be driven due to faulty cables, dongles
level shifters etc. To workaround them we may need to drive the output
at a lower bpc. Currently the user space does not have a way to limit
the bpc. The default bpc to be programmed is decided by the driver and
is run against connector limitations.
Creating a new connector property "max bpc" in order to limit the bpc
with which the pixels are scanned out. xrandr can make use of this
connector property to make sure that bpc does not exceed the configured value.
This property can be used by userspace to set the bpc.
V2: Initialize max_bpc to satisfy kms_properties
V3: Move the property to drm_connector
Cc: Ville Syrjälä
Cc: Kishore Kadiyala
Cc: Rodrigo Vivi
Cc: Manasi Navare
Cc: Stanislav Lisovskiy
Signed-off-by: Radhakrishna Sripada
---
drivers/gpu/drm/drm_atomic.c| 4
drivers/gpu/drm/drm_atomic_helper.c | 4
drivers/gpu/drm/i915/intel_drv.h| 2 ++
drivers/gpu/drm/i915/intel_hdmi.c | 11 +++
drivers/gpu/drm/i915/intel_modes.c | 20
include/drm/drm_connector.h | 6 ++
include/drm/drm_mode_config.h | 5 +
7 files changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3eb061e11e2e..461dde0c2c10 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
return set_out_fence_for_connector(state->state, connector,
fence_ptr);
+ } else if (property == config->max_bpc_property) {
+ state->max_bpc = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector
*connector,
*val = 0;
} else if (property == config->writeback_out_fence_ptr_property) {
*val = 0;
+ } else if (property == config->max_bpc_property) {
+ *val = state->max_bpc;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 38ce9a375ffb..82caac8d1432 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -638,6 +638,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (old_connector_state->link_status !=
new_connector_state->link_status)
new_crtc_state->connectors_changed = true;
+
+ if (old_connector_state->max_bpc !=
+ new_connector_state->max_bpc)
+ new_crtc_state->connectors_changed = true;
}
if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1b78de838c18..209eb1798238 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1862,6 +1862,8 @@ int intel_ddc_get_modes(struct drm_connector *c, struct
i2c_adapter *adapter);
void intel_attach_force_audio_property(struct drm_connector *connector);
void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
void intel_attach_aspect_ratio_property(struct drm_connector *connector);
+void intel_attach_max_bpc_property(struct drm_connector *connector, int min,
int
+ max);
/* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a1799b5c12bb..82739f342246 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2097,11 +2097,22 @@ static const struct drm_encoder_funcs
intel_hdmi_enc_funcs = {
static void
intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector
*connector)
{
+ struct drm_i915_private *dev_priv = to_i915(connector->dev);
+
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
intel_attach_aspect_ratio_property(connector);
drm_connector_attach_content_type_property(connector);
connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
+ if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
+ IS_CHERRYVIEW(dev_priv)) {
+ intel_attach_max_bpc_property(connector, 8, 10);
+ connector->state->max_bpc = 10;
+ } else if (INTEL_GEN(dev_priv) >= 5) {
+ intel_attach_max_bpc_property(connector, 8, 12);
+ connector->state->max_bpc = 12;
+
[Intel-gfx] [PATCH v3 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp
From: "Sripada, Radhakrishna"
Use the newly added "max bpc" connector property to limit pipe bpp.
V3: Use drm_connector_state to access the "max bpc" property
Cc: Ville Syrjälä
Cc: Rodrigo Vivi
Cc: Kishore Kadiyala
Cc: Manasi Navare
Cc: Stanislav Lisovskiy
Signed-off-by: Radhakrishna Sripada
---
drivers/gpu/drm/i915/intel_display.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 30fdfd1a3037..8506cd1634fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10784,6 +10784,28 @@ connected_sink_compute_bpp(struct intel_connector
*connector,
}
}
+static void
+connected_sink_max_bpp(struct drm_connector_state *conn_state,
+struct intel_crtc_state *pipe_config)
+{
+ switch (conn_state->max_bpc) {
+ case 8:
+ case 9:
+ pipe_config->pipe_bpp = 8*3;
+ break;
+ case 10:
+ case 11:
+ pipe_config->pipe_bpp = 10*3;
+ break;
+ case 12:
+ pipe_config->pipe_bpp = 12*3;
+ break;
+ default:
+ break;
+ }
+ DRM_DEBUG_KMS("Limiting display bpp to %d\n", pipe_config->pipe_bpp);
+}
+
static int
compute_baseline_pipe_bpp(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config)
@@ -10812,6 +10834,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
if (connector_state->crtc != >base)
continue;
+ if (connector_state->max_bpc)
+ connected_sink_max_bpp(connector_state, pipe_config);
+
connected_sink_compute_bpp(to_intel_connector(connector),
pipe_config);
}
--
2.9.3
___
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/psr: Remove wait_for_idle() for PSR2
== Series Details == Series: series starting with [1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 URL : https://patchwork.freedesktop.org/series/48691/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4704_full -> Patchwork_10014_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10014_full that come from known issues: === IGT changes === Issues hit igt@gem_ctx_isolation@bcs0-s3: shard-kbl: PASS -> INCOMPLETE (fdo#103665) +1 igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) Possible fixes igt@kms_cursor_legacy@cursor-vs-flip-toggle: shard-hsw: FAIL (fdo#103355) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS igt@kms_vblank@pipe-a-ts-continuation-suspend: shard-hsw: FAIL (fdo#104894) -> PASS fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 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_4704 -> Patchwork_10014 CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10014: 45eabe700f63f7160911b083c0775e251871bc0e @ 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_10014/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 1/4] intel: add IS_GENX() generic macro
This will allow platforms to reuse kernel IDs instead of manually
keeping them in sync. In most of the cases we only need to extend
IS_9XX(). Current platforms that fit this requirement can be ported
over to use this macro.
The i915_pciids.h header is in sync with kernel tree on
drm-tip 2018y-08m-20d-21h-41m-11s.
Signed-off-by: Lucas De Marchi
---
intel/i915_pciids.h | 461 ++
intel/intel_chipset.h | 20 ++
2 files changed, 481 insertions(+)
create mode 100644 intel/i915_pciids.h
diff --git a/intel/i915_pciids.h b/intel/i915_pciids.h
new file mode 100644
index ..fd965ffb
--- /dev/null
+++ b/intel/i915_pciids.h
@@ -0,0 +1,461 @@
+/*
+ * Copyright 2013 Intel Corporation
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+#ifndef _I915_PCIIDS_H
+#define _I915_PCIIDS_H
+
+/*
+ * A pci_device_id struct {
+ * __u32 vendor, device;
+ * __u32 subvendor, subdevice;
+ * __u32 class, class_mask;
+ * kernel_ulong_t driver_data;
+ * };
+ * Don't use C99 here because "class" is reserved and we want to
+ * give userspace flexibility.
+ */
+#define INTEL_VGA_DEVICE(id, info) { \
+ 0x8086, id, \
+ ~0, ~0, \
+ 0x03, 0xff, \
+ (unsigned long) info }
+
+#define INTEL_QUANTA_VGA_DEVICE(info) {\
+ 0x8086, 0x16a, \
+ 0x152d, 0x8990, \
+ 0x03, 0xff, \
+ (unsigned long) info }
+
+#define INTEL_I810_IDS(info) \
+ INTEL_VGA_DEVICE(0x7121, info), /* I810 */ \
+ INTEL_VGA_DEVICE(0x7123, info), /* I810_DC100 */\
+ INTEL_VGA_DEVICE(0x7125, info) /* I810_E */
+
+#define INTEL_I815_IDS(info) \
+ INTEL_VGA_DEVICE(0x1132, info) /* I815*/
+
+#define INTEL_I830_IDS(info) \
+ INTEL_VGA_DEVICE(0x3577, info)
+
+#define INTEL_I845G_IDS(info) \
+ INTEL_VGA_DEVICE(0x2562, info)
+
+#define INTEL_I85X_IDS(info) \
+ INTEL_VGA_DEVICE(0x3582, info), /* I855_GM */ \
+ INTEL_VGA_DEVICE(0x358e, info)
+
+#define INTEL_I865G_IDS(info) \
+ INTEL_VGA_DEVICE(0x2572, info) /* I865_G */
+
+#define INTEL_I915G_IDS(info) \
+ INTEL_VGA_DEVICE(0x2582, info), /* I915_G */ \
+ INTEL_VGA_DEVICE(0x258a, info) /* E7221_G */
+
+#define INTEL_I915GM_IDS(info) \
+ INTEL_VGA_DEVICE(0x2592, info) /* I915_GM */
+
+#define INTEL_I945G_IDS(info) \
+ INTEL_VGA_DEVICE(0x2772, info) /* I945_G */
+
+#define INTEL_I945GM_IDS(info) \
+ INTEL_VGA_DEVICE(0x27a2, info), /* I945_GM */ \
+ INTEL_VGA_DEVICE(0x27ae, info) /* I945_GME */
+
+#define INTEL_I965G_IDS(info) \
+ INTEL_VGA_DEVICE(0x2972, info), /* I946_GZ */ \
+ INTEL_VGA_DEVICE(0x2982, info), /* G35_G */ \
+ INTEL_VGA_DEVICE(0x2992, info), /* I965_Q */\
+ INTEL_VGA_DEVICE(0x29a2, info) /* I965_G */
+
+#define INTEL_G33_IDS(info)\
+ INTEL_VGA_DEVICE(0x29b2, info), /* Q35_G */ \
+ INTEL_VGA_DEVICE(0x29c2, info), /* G33_G */ \
+ INTEL_VGA_DEVICE(0x29d2, info) /* Q33_G */
+
+#define INTEL_I965GM_IDS(info) \
+ INTEL_VGA_DEVICE(0x2a02, info), /* I965_GM */ \
+ INTEL_VGA_DEVICE(0x2a12, info) /* I965_GME */
+
+#define INTEL_GM45_IDS(info) \
+ INTEL_VGA_DEVICE(0x2a42, info) /* GM45_G */
+
+#define INTEL_G45_IDS(info)\
+ INTEL_VGA_DEVICE(0x2e02, info), /*
[Intel-gfx] [PATCH libdrm 4/4] intel: make gen9 use generic gen macro
The 2 PCI IDs that are used for the command line overrid mechanism were left defined. The rest can be gone and then we just use the kernel defines. Signed-off-by: Lucas De Marchi --- intel/intel_chipset.h | 188 +- 1 file changed, 3 insertions(+), 185 deletions(-) diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h index 82f3b5a0..6887605e 100644 --- a/intel/intel_chipset.h +++ b/intel/intel_chipset.h @@ -165,86 +165,8 @@ #define PCI_CHIP_CHERRYVIEW_2 0x22b2 #define PCI_CHIP_CHERRYVIEW_3 0x22b3 -#define PCI_CHIP_SKYLAKE_DT_GT10x1902 -#define PCI_CHIP_SKYLAKE_ULT_GT1 0x1906 -#define PCI_CHIP_SKYLAKE_SRV_GT1 0x190A /* Reserved */ -#define PCI_CHIP_SKYLAKE_H_GT1 0x190B -#define PCI_CHIP_SKYLAKE_ULX_GT1 0x190E /* Reserved */ #define PCI_CHIP_SKYLAKE_DT_GT20x1912 -#define PCI_CHIP_SKYLAKE_FUSED0_GT20x1913 /* Reserved */ -#define PCI_CHIP_SKYLAKE_FUSED1_GT20x1915 /* Reserved */ -#define PCI_CHIP_SKYLAKE_ULT_GT2 0x1916 -#define PCI_CHIP_SKYLAKE_FUSED2_GT20x1917 /* Reserved */ -#define PCI_CHIP_SKYLAKE_SRV_GT2 0x191A /* Reserved */ -#define PCI_CHIP_SKYLAKE_HALO_GT2 0x191B -#define PCI_CHIP_SKYLAKE_WKS_GT2 0x191D -#define PCI_CHIP_SKYLAKE_ULX_GT2 0x191E -#define PCI_CHIP_SKYLAKE_MOBILE_GT20x1921 /* Reserved */ -#define PCI_CHIP_SKYLAKE_ULT_GT3_0 0x1923 -#define PCI_CHIP_SKYLAKE_ULT_GT3_1 0x1926 -#define PCI_CHIP_SKYLAKE_ULT_GT3_2 0x1927 -#define PCI_CHIP_SKYLAKE_SRV_GT4 0x192A -#define PCI_CHIP_SKYLAKE_HALO_GT3 0x192B /* Reserved */ -#define PCI_CHIP_SKYLAKE_SRV_GT3 0x192D -#define PCI_CHIP_SKYLAKE_DT_GT40x1932 -#define PCI_CHIP_SKYLAKE_SRV_GT4X 0x193A -#define PCI_CHIP_SKYLAKE_H_GT4 0x193B -#define PCI_CHIP_SKYLAKE_WKS_GT4 0x193D - -#define PCI_CHIP_KABYLAKE_ULT_GT2 0x5916 -#define PCI_CHIP_KABYLAKE_ULT_GT1_50x5913 -#define PCI_CHIP_KABYLAKE_ULT_GT1 0x5906 -#define PCI_CHIP_KABYLAKE_ULT_GT3_00x5923 -#define PCI_CHIP_KABYLAKE_ULT_GT3_10x5926 -#define PCI_CHIP_KABYLAKE_ULT_GT3_20x5927 -#define PCI_CHIP_KABYLAKE_ULT_GT2F 0x5921 -#define PCI_CHIP_KABYLAKE_ULX_GT1_50x5915 -#define PCI_CHIP_KABYLAKE_ULX_GT1 0x590E -#define PCI_CHIP_KABYLAKE_ULX_GT2_00x591E #define PCI_CHIP_KABYLAKE_DT_GT2 0x5912 -#define PCI_CHIP_KABYLAKE_M_GT20x5917 -#define PCI_CHIP_KABYLAKE_DT_GT1 0x5902 -#define PCI_CHIP_KABYLAKE_HALO_GT2 0x591B -#define PCI_CHIP_KABYLAKE_HALO_GT4 0x593B -#define PCI_CHIP_KABYLAKE_HALO_GT1_0 0x5908 -#define PCI_CHIP_KABYLAKE_HALO_GT1_1 0x590B -#define PCI_CHIP_KABYLAKE_SRV_GT2 0x591A -#define PCI_CHIP_KABYLAKE_SRV_GT1 0x590A -#define PCI_CHIP_KABYLAKE_WKS_GT2 0x591D - -#define PCI_CHIP_AMBERLAKE_ULX_GT2_1 0x591C -#define PCI_CHIP_AMBERLAKE_ULX_GT2_2 0x87C0 - -#define PCI_CHIP_BROXTON_0 0x0A84 -#define PCI_CHIP_BROXTON_1 0x1A84 -#define PCI_CHIP_BROXTON_2 0x5A84 -#define PCI_CHIP_BROXTON_3 0x1A85 -#define PCI_CHIP_BROXTON_4 0x5A85 - -#define PCI_CHIP_GLK 0x3184 -#define PCI_CHIP_GLK_2X6 0x3185 - -#define PCI_CHIP_COFFEELAKE_S_GT1_1 0x3E90 -#define PCI_CHIP_COFFEELAKE_S_GT1_2 0x3E93 -#define PCI_CHIP_COFFEELAKE_S_GT1_3 0x3E99 -#define PCI_CHIP_COFFEELAKE_S_GT2_1 0x3E91 -#define PCI_CHIP_COFFEELAKE_S_GT2_2 0x3E92 -#define PCI_CHIP_COFFEELAKE_S_GT2_3 0x3E96 -#define PCI_CHIP_COFFEELAKE_S_GT2_4 0x3E98 -#define PCI_CHIP_COFFEELAKE_S_GT2_5 0x3E9A -#define PCI_CHIP_COFFEELAKE_H_GT2_1 0x3E9B -#define PCI_CHIP_COFFEELAKE_H_GT2_2 0x3E94 -#define PCI_CHIP_COFFEELAKE_U_GT2_1 0x3EA9 -#define PCI_CHIP_COFFEELAKE_U_GT3_1 0x3EA5 -#define PCI_CHIP_COFFEELAKE_U_GT3_2 0x3EA6 -#define PCI_CHIP_COFFEELAKE_U_GT3_3 0x3EA7 -#define PCI_CHIP_COFFEELAKE_U_GT3_4 0x3EA8 - -#define PCI_CHIP_WHISKEYLAKE_U_GT1_1 0x3EA1 -#define PCI_CHIP_WHISKEYLAKE_U_GT2_1 0x3EA0 -#define PCI_CHIP_WHISKEYLAKE_U_GT3_1 0x3EA2 -#define PCI_CHIP_WHISKEYLAKE_U_GT3_2 0x3EA3 -#define PCI_CHIP_WHISKEYLAKE_U_GT3_3 0x3EA4 #define IS_MOBILE(devid) ((devid) == PCI_CHIP_I855_GM || \ (devid) == PCI_CHIP_I915_GM || \ @@ -405,113 +327,6 @@ #define IS_GEN8(devid) (IS_BROADWELL(devid) || \ IS_CHERRYVIEW(devid)) -#define IS_SKL_GT1(devid) ((devid) == PCI_CHIP_SKYLAKE_DT_GT1 || \ -(devid) == PCI_CHIP_SKYLAKE_ULT_GT1|| \ -(devid) == PCI_CHIP_SKYLAKE_SRV_GT1|| \ -(devid) == PCI_CHIP_SKYLAKE_H_GT1 || \ -(devid) == PCI_CHIP_SKYLAKE_ULX_GT1) - -#define IS_SKL_GT2(devid) ((devid) == PCI_CHIP_SKYLAKE_DT_GT2 || \ -
[Intel-gfx] [PATCH libdrm 2/4] intel: make gen11 use generic gen macro
Signed-off-by: Lucas De Marchi
---
intel/intel_chipset.h | 26 ++
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index 8a0e3e76..163fb536 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -261,16 +261,6 @@
#define PCI_CHIP_CANNONLAKE_12 0x5A44
#define PCI_CHIP_CANNONLAKE_13 0x5A4C
-#define PCI_CHIP_ICELAKE_11_0 0x8A50
-#define PCI_CHIP_ICELAKE_11_1 0x8A51
-#define PCI_CHIP_ICELAKE_11_2 0x8A5C
-#define PCI_CHIP_ICELAKE_11_3 0x8A5D
-#define PCI_CHIP_ICELAKE_11_4 0x8A52
-#define PCI_CHIP_ICELAKE_11_5 0x8A5A
-#define PCI_CHIP_ICELAKE_11_6 0x8A5B
-#define PCI_CHIP_ICELAKE_11_7 0x8A71
-#define PCI_CHIP_ICELAKE_11_8 0x8A70
-
#define IS_MOBILE(devid) ((devid) == PCI_CHIP_I855_GM || \
(devid) == PCI_CHIP_I915_GM || \
(devid) == PCI_CHIP_I945_GM || \
@@ -554,20 +544,6 @@
#define IS_GEN10(devid)(IS_CANNONLAKE(devid))
-#define IS_ICELAKE_11(devid) ((devid) == PCI_CHIP_ICELAKE_11_0 || \
-(devid) == PCI_CHIP_ICELAKE_11_1 || \
-(devid) == PCI_CHIP_ICELAKE_11_2 || \
-(devid) == PCI_CHIP_ICELAKE_11_3 || \
-(devid) == PCI_CHIP_ICELAKE_11_4 || \
-(devid) == PCI_CHIP_ICELAKE_11_5 || \
-(devid) == PCI_CHIP_ICELAKE_11_6 || \
-(devid) == PCI_CHIP_ICELAKE_11_7 || \
-(devid) == PCI_CHIP_ICELAKE_11_8)
-
-#define IS_ICELAKE(devid) (IS_ICELAKE_11(devid))
-
-#define IS_GEN11(devid)(IS_ICELAKE_11(devid))
-
/* New platforms use kernel pci ids */
#include "i915_pciids.h"
@@ -587,6 +563,8 @@ struct pci_device_id {
break; \
__i < __n;})
+#define IS_GEN11(devid) IS_GENX(ICL_11, devid)
+
/* all platforms */
#define IS_9XX(dev)(IS_GEN3(dev) || \
IS_GEN4(dev) || \
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 0/4] intel: rework how we add PCI IDs
Adding PCI IDs to different projects is a boring manual task that motivated me to create this series. The idea is to centralize the IDs in the kernel header and let other projects copy it. Initially my plan was to convert all gens, back to gen2, but that proved slightly difficult since there are some corner cases to cover and I didn't want to block the important part, i.e.: for recent gens, there's no risk of missing a PCI ID. It does increase the size a little bit, but doesn't explode. Size diff below showing ~3k: - 36533176 40 367498f8d build/intel/intel@@drm_intel@sha/intel_bufmgr_gem.c.o - 66237 1384 24 67645 1083d build/intel/intel@@drm_intel@sha/intel_decode.c.o + 39362176 40 395789a9a build/intel/intel@@drm_intel@sha/intel_bufmgr_gem.c.o + 68935 1384 24 70343 112c7 build/intel/intel@@drm_intel@sha/intel_decode.c.o Let me know what you think. Lucas De Marchi (4): intel: add IS_GENX() generic macro intel: make gen11 use generic gen macro intel: make gen10 use generic gen macro intel: make gen9 use generic gen macro intel/i915_pciids.h | 461 ++ intel/intel_chipset.h | 267 +++- 2 files changed, 487 insertions(+), 241 deletions(-) create mode 100644 intel/i915_pciids.h -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 3/4] intel: make gen10 use generic gen macro
Signed-off-by: Lucas De Marchi
---
intel/intel_chipset.h | 33 +
1 file changed, 1 insertion(+), 32 deletions(-)
diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index 163fb536..82f3b5a0 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -246,21 +246,6 @@
#define PCI_CHIP_WHISKEYLAKE_U_GT3_2 0x3EA3
#define PCI_CHIP_WHISKEYLAKE_U_GT3_3 0x3EA4
-#define PCI_CHIP_CANNONLAKE_0 0x5A51
-#define PCI_CHIP_CANNONLAKE_1 0x5A59
-#define PCI_CHIP_CANNONLAKE_2 0x5A41
-#define PCI_CHIP_CANNONLAKE_3 0x5A49
-#define PCI_CHIP_CANNONLAKE_4 0x5A52
-#define PCI_CHIP_CANNONLAKE_5 0x5A5A
-#define PCI_CHIP_CANNONLAKE_6 0x5A42
-#define PCI_CHIP_CANNONLAKE_7 0x5A4A
-#define PCI_CHIP_CANNONLAKE_8 0x5A50
-#define PCI_CHIP_CANNONLAKE_9 0x5A40
-#define PCI_CHIP_CANNONLAKE_10 0x5A54
-#define PCI_CHIP_CANNONLAKE_11 0x5A5C
-#define PCI_CHIP_CANNONLAKE_12 0x5A44
-#define PCI_CHIP_CANNONLAKE_13 0x5A4C
-
#define IS_MOBILE(devid) ((devid) == PCI_CHIP_I855_GM || \
(devid) == PCI_CHIP_I915_GM || \
(devid) == PCI_CHIP_I945_GM || \
@@ -527,23 +512,6 @@
IS_GEMINILAKE(devid) || \
IS_COFFEELAKE(devid))
-#define IS_CANNONLAKE(devid) ((devid) == PCI_CHIP_CANNONLAKE_0 || \
-(devid) == PCI_CHIP_CANNONLAKE_1 || \
-(devid) == PCI_CHIP_CANNONLAKE_2 || \
-(devid) == PCI_CHIP_CANNONLAKE_3 || \
-(devid) == PCI_CHIP_CANNONLAKE_4 || \
-(devid) == PCI_CHIP_CANNONLAKE_5 || \
-(devid) == PCI_CHIP_CANNONLAKE_6 || \
-(devid) == PCI_CHIP_CANNONLAKE_7 || \
-(devid) == PCI_CHIP_CANNONLAKE_8 || \
-(devid) == PCI_CHIP_CANNONLAKE_9 || \
-(devid) == PCI_CHIP_CANNONLAKE_10 || \
-(devid) == PCI_CHIP_CANNONLAKE_11 || \
-(devid) == PCI_CHIP_CANNONLAKE_12 || \
-(devid) == PCI_CHIP_CANNONLAKE_13)
-
-#define IS_GEN10(devid)(IS_CANNONLAKE(devid))
-
/* New platforms use kernel pci ids */
#include "i915_pciids.h"
@@ -563,6 +531,7 @@ struct pci_device_id {
break; \
__i < __n;})
+#define IS_GEN10(devid) IS_GENX(CNL, devid)
#define IS_GEN11(devid) IS_GENX(ICL_11, devid)
/* all platforms */
--
2.17.1
___
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: Clean up skl_plane_has_planar()
On Fri, 2018-08-24 at 16:16 -0700, Rodrigo Vivi wrote:
> On Fri, Aug 24, 2018 at 10:27:09PM +, Pandiyan, Dhinakaran wrote:
> >
> >
> > On Fri, 2018-08-24 at 15:15 -0700, Rodrigo Vivi wrote:
> > > On Fri, Aug 24, 2018 at 01:38:55PM -0700, Dhinakaran Pandiyan
> > > wrote:
> > > > skl_plane_has_planar is hard to read, simplify the logic by
> > > > checking for
> > > > support in the order of platform, pipe and plane.
> > > >
> > > > No change in functionality intended.
> > > >
> > > > Cc: Ville Syrjälä
> > > > Signed-off-by: Dhinakaran Pandiyan > > > om>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 27 +---
> > > >
> > > > ---
> > > > 1 file changed, 9 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 30fdfd1a3037..7e18bd8b21b8 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct
> > > > drm_i915_private *dev_priv,
> > > > bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
> > > > enum pipe pipe, enum plane_id
> > > > plane_id)
> > > > {
> > > > - if (plane_id == PLANE_PRIMARY) {
> > > > - if (IS_SKYLAKE(dev_priv) ||
> > > > IS_BROXTON(dev_priv))
> > > > - return false;
> > > > - else if ((INTEL_GEN(dev_priv) == 9 && pipe ==
> > > > PIPE_C) &&
> > > > -!IS_GEMINILAKE(dev_priv))
> > > > - return false;
> > > > - } else if (plane_id >= PLANE_SPRITE0) {
> > > > - if (plane_id == PLANE_CURSOR)
> > > > - return false;
> > > > - if (IS_GEMINILAKE(dev_priv) ||
> > > > INTEL_GEN(dev_priv)
> > > > == 10) {
> > > > - if (plane_id != PLANE_SPRITE0)
> > > > - return false;
> > > > - } else {
> > > > - if (plane_id != PLANE_SPRITE0 || pipe
> > > > ==
> > > > PIPE_C ||
> > > > - IS_SKYLAKE(dev_priv) ||
> > > > IS_BROXTON(dev_priv))
> > > > - return false;
> >
> > ^ Here it does, or am I not seeing the paranthesis correctly.
>
> dam... it is so bad that I got confused when drawing a
> table here...
>
> nip: The name of the function seems wrong since "skl_" always
> return false
Looks like the first versions of the patch implementing nv12 did
include SKL, not clear what happened later.
> but I don't have a better suggestion ... maybe just "intel_"
> and can be in a follow up patch...
>
> so,
>
> Reviewed-by: Rodrigo Vivi
Thanks!
-DK
>
> >
> > > > - }
> > > > - }
> > > > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > > + return false;
> > >
> > > The current code is indeed ugly, but I'm afraid it doesn't always
> > > return
> > > false for these platforms.
> > >
> > > for instance plane_id = PLANE_SPRITE0
> > >
> > > > +
> > > > + if (INTEL_GEN(dev_priv) == 9 &&
> > > > !IS_GEMINILAKE(dev_priv)
> > > > && pipe == PIPE_C)
> > > > + return false;
> > > > +
> > > > + if (plane_id == PLANE_CURSOR || plane_id !=
> > > > PLANE_SPRITE0)
> > > > + return false;
> > > > +
> > > > return true;
> > > > }
> > > >
> > > > --
> > > > 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
___
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/psr: Remove wait_for_idle() for PSR2
== Series Details ==
Series: series starting with [1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
URL : https://patchwork.freedesktop.org/series/48691/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4704 -> Patchwork_10014 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48691/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10014:
=== IGT changes ===
Warnings
{igt@kms_psr@cursor_plane_move}:
fi-cnl-psr: DMESG-FAIL -> FAIL
== Known issues ==
Here are the changes found in Patchwork_10014 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
{fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#106560)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
Possible fixes
igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
fi-skl-guc: FAIL (fdo#103191) -> PASS +1
{fi-byt-clapper}: FAIL (fdo#107362, fdo#103191) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-skl-6260u: INCOMPLETE (fdo#104108) -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN -> PASS +1
{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#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
== Participating hosts (54 -> 48) ==
Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_4704 -> Patchwork_10014
CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10014: 45eabe700f63f7160911b083c0775e251871bc0e @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
45eabe700f63 drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle()
62024ccae1a4 drm/i915/psr: Remove wait_for_idle() for PSR2
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10014/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/psr: Rewrite comments in intel_psr_wait_for_idle()
On Fri, Aug 24, 2018 at 04:08:44PM -0700, Dhinakaran Pandiyan wrote:
> Added bspec reference, aligned text and documented the function.
>
> Cc: Rodrigo Vivi
> Signed-off-by: Dhinakaran Pandiyan
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_psr.c | 28 ++--
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 2cb931f3019b..aee64aee18fe 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -766,6 +766,16 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> cancel_work_sync(_priv->psr.work);
> }
>
> +/**
> + * intel_psr_wait_for_idle - wait for PSR1 to idle
> + * @new_crtc_state: new CRTC state
> + * @out_value: PSR status in case of failure
> + *
> + * This function is expected to be called from pipe_update_start() where it
> is
> + * not expected to race with PSR enable or disable.
> + *
> + * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
> + */
> int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> u32 *out_value)
> {
> @@ -775,25 +785,15 @@ int intel_psr_wait_for_idle(const struct
> intel_crtc_state *new_crtc_state,
> if (!dev_priv->psr.enabled || !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.
> - */
> -
> /* FIXME: Update this for PSR2 if we need to wait for idle */
> if (READ_ONCE(dev_priv->psr.psr2_enabled))
> 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+)
> + * 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, EDP_PSR_STATUS,
> --
> 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 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
On Fri, Aug 24, 2018 at 04:08:43PM -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, when vblank interrupts are enabled. This check was added for PSR1
> but incorrectly extended to PSR2, remove the check as it breaks tests
> and prints out misleading error messages.
>
> v2: Split out non-code changes (Rodrigo)
>
> Cc: Tarun Vyas
> Cc: José Roberto de Souza
> Cc: Rodrigo Vivi
> Fixes: c43dbcbbcc8c ("drm/i915/psr: Lockless version of psr_wait_for_idle")
> Signed-off-by: Dhinakaran Pandiyan
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_psr.c | 16 ++--
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index da583a45e942..2cb931f3019b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -771,8 +771,6 @@ 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 (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
> return 0;
> @@ -787,13 +785,10 @@ int intel_psr_wait_for_idle(const struct
> intel_crtc_state *new_crtc_state,
>* not needed and will induce latencies in the atomic
>* update path.
>*/
> - 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;
> - }
> +
> + /* FIXME: Update this for PSR2 if we need to wait for idle */
> + if (READ_ONCE(dev_priv->psr.psr2_enabled))
> + return 0;
>
> /*
>* Max time for PSR to idle = Inverse of the refresh rate +
> @@ -801,7 +796,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state
> *new_crtc_state,
>* handshake. 50 msec is defesive 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 1/2] drm/i915: Clean up skl_plane_has_planar()
On Fri, Aug 24, 2018 at 10:27:09PM +, Pandiyan, Dhinakaran wrote:
>
>
> On Fri, 2018-08-24 at 15:15 -0700, Rodrigo Vivi wrote:
> > On Fri, Aug 24, 2018 at 01:38:55PM -0700, Dhinakaran Pandiyan wrote:
> > > skl_plane_has_planar is hard to read, simplify the logic by
> > > checking for
> > > support in the order of platform, pipe and plane.
> > >
> > > No change in functionality intended.
> > >
> > > Cc: Ville Syrjälä
> > > Signed-off-by: Dhinakaran Pandiyan
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 27 +---
> > > ---
> > > 1 file changed, 9 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 30fdfd1a3037..7e18bd8b21b8 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct
> > > drm_i915_private *dev_priv,
> > > bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
> > > enum pipe pipe, enum plane_id plane_id)
> > > {
> > > - if (plane_id == PLANE_PRIMARY) {
> > > - if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > - return false;
> > > - else if ((INTEL_GEN(dev_priv) == 9 && pipe ==
> > > PIPE_C) &&
> > > - !IS_GEMINILAKE(dev_priv))
> > > - return false;
> > > - } else if (plane_id >= PLANE_SPRITE0) {
> > > - if (plane_id == PLANE_CURSOR)
> > > - return false;
> > > - if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
> > > == 10) {
> > > - if (plane_id != PLANE_SPRITE0)
> > > - return false;
> > > - } else {
> > > - if (plane_id != PLANE_SPRITE0 || pipe ==
> > > PIPE_C ||
> > > - IS_SKYLAKE(dev_priv) ||
> > > IS_BROXTON(dev_priv))
> > > - return false;
> ^ Here it does, or am I not seeing the paranthesis correctly.
dam... it is so bad that I got confused when drawing a
table here...
nip: The name of the function seems wrong since "skl_" always
return false
but I don't have a better suggestion ... maybe just "intel_"
and can be in a follow up patch...
so,
Reviewed-by: Rodrigo Vivi
>
> > > - }
> > > - }
> > > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > + return false;
> >
> > The current code is indeed ugly, but I'm afraid it doesn't always
> > return
> > false for these platforms.
> >
> > for instance plane_id = PLANE_SPRITE0
> >
> > > +
> > > + if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)
> > > && pipe == PIPE_C)
> > > + return false;
> > > +
> > > + if (plane_id == PLANE_CURSOR || plane_id != PLANE_SPRITE0)
> > > + return false;
> > > +
> > > return true;
> > > }
> > >
> > > --
> > > 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
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, when vblank interrupts are enabled. This check was added for PSR1
but incorrectly extended to PSR2, remove the check as it breaks tests
and prints out misleading error messages.
v2: Split out non-code changes (Rodrigo)
Cc: Tarun Vyas
Cc: José Roberto de Souza
Cc: Rodrigo Vivi
Fixes: c43dbcbbcc8c ("drm/i915/psr: Lockless version of psr_wait_for_idle")
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/intel_psr.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index da583a45e942..2cb931f3019b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -771,8 +771,6 @@ 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 (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
return 0;
@@ -787,13 +785,10 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state
*new_crtc_state,
* not needed and will induce latencies in the atomic
* update path.
*/
- 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;
- }
+
+ /* FIXME: Update this for PSR2 if we need to wait for idle */
+ if (READ_ONCE(dev_priv->psr.psr2_enabled))
+ return 0;
/*
* Max time for PSR to idle = Inverse of the refresh rate +
@@ -801,7 +796,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state
*new_crtc_state,
* handshake. 50 msec is defesive 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] [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle()
Added bspec reference, aligned text and documented the function.
Cc: Rodrigo Vivi
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/intel_psr.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2cb931f3019b..aee64aee18fe 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -766,6 +766,16 @@ void intel_psr_disable(struct intel_dp *intel_dp,
cancel_work_sync(_priv->psr.work);
}
+/**
+ * intel_psr_wait_for_idle - wait for PSR1 to idle
+ * @new_crtc_state: new CRTC state
+ * @out_value: PSR status in case of failure
+ *
+ * This function is expected to be called from pipe_update_start() where it is
+ * not expected to race with PSR enable or disable.
+ *
+ * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
+ */
int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
u32 *out_value)
{
@@ -775,25 +785,15 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state
*new_crtc_state,
if (!dev_priv->psr.enabled || !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.
-*/
-
/* FIXME: Update this for PSR2 if we need to wait for idle */
if (READ_ONCE(dev_priv->psr.psr2_enabled))
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+)
+* 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, EDP_PSR_STATUS,
--
2.17.1
___
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: Clean up skl_plane_has_planar()
On Fri, 2018-08-24 at 15:15 -0700, Rodrigo Vivi wrote:
> On Fri, Aug 24, 2018 at 01:38:55PM -0700, Dhinakaran Pandiyan wrote:
> > skl_plane_has_planar is hard to read, simplify the logic by
> > checking for
> > support in the order of platform, pipe and plane.
> >
> > No change in functionality intended.
> >
> > Cc: Ville Syrjälä
> > Signed-off-by: Dhinakaran Pandiyan
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 27 +---
> > ---
> > 1 file changed, 9 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 30fdfd1a3037..7e18bd8b21b8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct
> > drm_i915_private *dev_priv,
> > bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
> > enum pipe pipe, enum plane_id plane_id)
> > {
> > - if (plane_id == PLANE_PRIMARY) {
> > - if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > - return false;
> > - else if ((INTEL_GEN(dev_priv) == 9 && pipe ==
> > PIPE_C) &&
> > -!IS_GEMINILAKE(dev_priv))
> > - return false;
> > - } else if (plane_id >= PLANE_SPRITE0) {
> > - if (plane_id == PLANE_CURSOR)
> > - return false;
> > - if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
> > == 10) {
> > - if (plane_id != PLANE_SPRITE0)
> > - return false;
> > - } else {
> > - if (plane_id != PLANE_SPRITE0 || pipe ==
> > PIPE_C ||
> > - IS_SKYLAKE(dev_priv) ||
> > IS_BROXTON(dev_priv))
> > - return false;
^ Here it does, or am I not seeing the paranthesis correctly.
> > - }
> > - }
> > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > + return false;
>
> The current code is indeed ugly, but I'm afraid it doesn't always
> return
> false for these platforms.
>
> for instance plane_id = PLANE_SPRITE0
>
> > +
> > + if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)
> > && pipe == PIPE_C)
> > + return false;
> > +
> > + if (plane_id == PLANE_CURSOR || plane_id != PLANE_SPRITE0)
> > + return false;
> > +
> > return true;
> > }
> >
> > --
> > 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 1/2] drm/i915: Clean up skl_plane_has_planar()
On Fri, Aug 24, 2018 at 01:38:55PM -0700, Dhinakaran Pandiyan wrote:
> skl_plane_has_planar is hard to read, simplify the logic by checking for
> support in the order of platform, pipe and plane.
>
> No change in functionality intended.
>
> Cc: Ville Syrjälä
> Signed-off-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/i915/intel_display.c | 27 +--
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 30fdfd1a3037..7e18bd8b21b8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct
> drm_i915_private *dev_priv,
> bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
> enum pipe pipe, enum plane_id plane_id)
> {
> - if (plane_id == PLANE_PRIMARY) {
> - if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> - return false;
> - else if ((INTEL_GEN(dev_priv) == 9 && pipe == PIPE_C) &&
> - !IS_GEMINILAKE(dev_priv))
> - return false;
> - } else if (plane_id >= PLANE_SPRITE0) {
> - if (plane_id == PLANE_CURSOR)
> - return false;
> - if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) == 10) {
> - if (plane_id != PLANE_SPRITE0)
> - return false;
> - } else {
> - if (plane_id != PLANE_SPRITE0 || pipe == PIPE_C ||
> - IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> - return false;
> - }
> - }
> + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> + return false;
The current code is indeed ugly, but I'm afraid it doesn't always return
false for these platforms.
for instance plane_id = PLANE_SPRITE0
> +
> + if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv) && pipe ==
> PIPE_C)
> + return false;
> +
> + if (plane_id == PLANE_CURSOR || plane_id != PLANE_SPRITE0)
> + return false;
> +
> return true;
> }
>
> --
> 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
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Clean up skl_plane_has_planar()
== Series Details == Series: series starting with [1/2] drm/i915: Clean up skl_plane_has_planar() URL : https://patchwork.freedesktop.org/series/48687/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4704_full -> Patchwork_10013_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10013_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10013_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_10013_full: === IGT changes === Warnings igt@kms_vblank@pipe-a-wait-busy-hang: shard-snb: PASS -> SKIP +2 == Known issues == Here are the changes found in Patchwork_10013_full that come from known issues: === IGT changes === Issues hit igt@gem_busy@extended-parallel-bsd1: shard-hsw: SKIP -> INCOMPLETE (fdo#103540) +22 igt@gem_flink_basic@flink-lifetime: shard-hsw: PASS -> INCOMPLETE (fdo#103540) +46 igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) Possible fixes igt@kms_cursor_legacy@cursor-vs-flip-toggle: shard-hsw: FAIL (fdo#103355) -> PASS igt@kms_vblank@pipe-a-ts-continuation-suspend: shard-hsw: FAIL (fdo#104894) -> PASS fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894 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_4704 -> Patchwork_10013 CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10013: 24a2c0467627d06cbb32a43469a8b4dfbe9241a1 @ 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_10013/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-next-fixes
Hi Dave, This is the pull request that I tried to send few days ago, but apparently I had some issue with my mutt/smtp and it didn't go through. Well, I don't think that there is anything critical that cannot wait another week or -rc2 Anyway I'm sending just in case, for the record, and to try out my dim changes that allow to do pull requests with old generated tags. :) Here goes drm-intel-next-fixes-2018-08-22: One important fix for display affecting NUCs with LSPCON and other 3 small fixes for: - audio hook when display is disabled - icl's has_csr definition - vma stop holding ppgtt reference Thanks, Rodrigo. The following changes since commit 4795ac626a2fe5ce99ff788080ace343faf4c886: Merge tag 'gvt-next-fixes-2018-08-14' of https://github.com/intel/gvt-linux into drm-intel-next-fixes (2018-08-15 13:42:32 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2018-08-22 for you to fetch changes up to 2693efd99cad2509522a88722e04162b7e869333: drm/i915/audio: Hook up component bindings even if displays are disabled (2018-08-20 17:24:55 -0700) One important fix for display affecting NUCs with LSPCON and other 3 small fixes for: - audio hook when display is disabled - icl's has_csr definition - vma stop holding ppgtt reference Anusha Srivatsa (1): drm/i915: Do not redefine the has_csr parameter. Chris Wilson (2): drm/i915: Stop holding a ref to the ppgtt from each vma drm/i915/audio: Hook up component bindings even if displays are disabled Fredrik Schön (1): drm/i915: Increase LSPCON timeout drivers/gpu/drm/i915/i915_pci.c | 1 - drivers/gpu/drm/i915/i915_vma.c | 4 drivers/gpu/drm/i915/intel_audio.c | 3 --- drivers/gpu/drm/i915/intel_lspcon.c | 2 +- 4 files changed, 1 insertion(+), 9 deletions(-) ___ 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: Clean up skl_plane_has_planar()
== Series Details ==
Series: series starting with [1/2] drm/i915: Clean up skl_plane_has_planar()
URL : https://patchwork.freedesktop.org/series/48687/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4704 -> Patchwork_10013 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48687/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10013:
=== IGT changes ===
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-WARN -> DMESG-FAIL
== Known issues ==
Here are the changes found in Patchwork_10013 that come from known issues:
=== IGT changes ===
Issues hit
igt@kms_chamelium@hdmi-hpd-fast:
fi-kbl-7500u: SKIP -> FAIL (fdo#102672, fdo#103841)
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
Possible fixes
igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
fi-skl-guc: FAIL (fdo#103191) -> PASS +1
{fi-byt-clapper}: FAIL (fdo#107362, fdo#103191) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-skl-6260u: INCOMPLETE (fdo#104108) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
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_4704 -> Patchwork_10013
CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10013: 24a2c0467627d06cbb32a43469a8b4dfbe9241a1 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
24a2c0467627 drm/i915: Do not advertize support for NV12 on ICL yet.
b0c0b66f1e11 drm/i915: Clean up skl_plane_has_planar()
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10013/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/18] drm/i915: Bump gen4+ fb stride limit to 256KiB
On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> With gtt remapping plugged in we can simply raise the stride
> limit on gen4+. Let's just arbitraily pick 256 KiB as the limit.
>
> No remapping CCS because the virtual address of each page actually
> matters due to the new hash mode
> (WaCompressedResourceDisplayNewHashMode:skl,kbl etc.), and no
> remapping
> on gen2/3 due to lack of fence on the remapped vma.
With someone else review in patch 15 and 16:
Reviewed-by: José Roberto de Souza
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8fb78214b4be..fa199f469e81 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2505,6 +2505,19 @@ static
> u32 intel_fb_max_stride(struct drm_i915_private *dev_priv,
> u32 pixel_format, u64 modifier)
> {
> + /*
> + * Arbitrary limit for gen4+. We can deal with any page
> + * aligned stride via GTT remapping. Gen2/3 need a fence
> + * for tiled scanout which the remapped vma won't have,
> + * so we don't allow remapping on those platforms.
> + *
> + * Also the new hash mode we use for CCS isn't compatible
> + * with remapping as the virtual address of the pages
> + * affects the compressed data.
> + */
> + if (INTEL_GEN(dev_priv) >= 4 &&
> !intel_modifier_has_ccs(modifier))
> + return 256*1024;
> +
> return intel_plane_fb_max_stride(dev_priv, pixel_format,
> modifier);
> }
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/18] drm/i915: Bump gen4+ fb size limits to 32kx32k
On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> With gtt remapping in place we can use arbitraily large framebuffers.
> Let's bump the limits as high as we can (32k-1). Going beyond that
> would require switching out s16.16 src coordinate representation to
> something with more spare bits.
With someone else review in patch 15 and 16:
Reviewed-by: José Roberto de Souza
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fa199f469e81..8aa2a61d2943 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15429,8 +15429,8 @@ int intel_modeset_init(struct drm_device
> *dev)
> dev->mode_config.max_width = 4096;
> dev->mode_config.max_height = 4096;
> } else {
> - dev->mode_config.max_width = 8192;
> - dev->mode_config.max_height = 8192;
> + dev->mode_config.max_width = 32767;
> + dev->mode_config.max_height = 32767;
> }
>
> if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Do not advertize support for NV12 on ICL yet.
ICL requires two planes for scanning out a NV12 framebuffer. Do
not advertize support for creating NV12 framebuffers until required
plane programming is implemented.
v2: Do not allow adding buffers.
Check inside skl_plane_has_planar (Ville)
Bspec: Plane Planar YUV programming (18566)
Cc: Ville Syrjälä
Cc: Rodrigo Vivi
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/intel_display.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7e18bd8b21b8..8e4e88eb10e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13622,6 +13622,13 @@ static bool skl_plane_has_fbc(struct drm_i915_private
*dev_priv,
bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
enum pipe pipe, enum plane_id plane_id)
{
+ /*
+* FIXME: ICL requires two hardware planes for scanning out NV12
+* framebuffers. Do not advertize support until this is implemented.
+*/
+ if (INTEL_GEN(dev_priv) >= 11)
+ return false;
+
if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
return false;
@@ -14543,7 +14550,7 @@ static int intel_framebuffer_init(struct
intel_framebuffer *intel_fb,
break;
case DRM_FORMAT_NV12:
if (INTEL_GEN(dev_priv) < 9 || IS_SKYLAKE(dev_priv) ||
- IS_BROXTON(dev_priv)) {
+ IS_BROXTON(dev_priv) || INTEL_GEN(dev_priv) >= 11) {
DRM_DEBUG_KMS("unsupported pixel format: %s\n",
drm_get_format_name(mode_cmd->pixel_format,
_name));
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Clean up skl_plane_has_planar()
skl_plane_has_planar is hard to read, simplify the logic by checking for
support in the order of platform, pipe and plane.
No change in functionality intended.
Cc: Ville Syrjälä
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/intel_display.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 30fdfd1a3037..7e18bd8b21b8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct drm_i915_private
*dev_priv,
bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
enum pipe pipe, enum plane_id plane_id)
{
- if (plane_id == PLANE_PRIMARY) {
- if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
- return false;
- else if ((INTEL_GEN(dev_priv) == 9 && pipe == PIPE_C) &&
-!IS_GEMINILAKE(dev_priv))
- return false;
- } else if (plane_id >= PLANE_SPRITE0) {
- if (plane_id == PLANE_CURSOR)
- return false;
- if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) == 10) {
- if (plane_id != PLANE_SPRITE0)
- return false;
- } else {
- if (plane_id != PLANE_SPRITE0 || pipe == PIPE_C ||
- IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
- return false;
- }
- }
+ if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
+ return false;
+
+ if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv) && pipe ==
PIPE_C)
+ return false;
+
+ if (plane_id == PLANE_CURSOR || plane_id != PLANE_SPRITE0)
+ return false;
+
return true;
}
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/18] drm/i915: Store ggtt_view in plane_state
On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Stash the gtt_view structure into the plane state. This will become
> useful when we do GTT remapping as the gtt_view will not come
> directly
> from the fb anymore.
Reviewed-by: José Roberto de Souza
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 16 +---
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> drivers/gpu/drm/i915/intel_fbdev.c | 6 --
> 3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index eadb8b20d504..e6cb8238f257 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2079,14 +2079,13 @@ static bool intel_plane_uses_fence(const
> struct intel_plane_state *plane_state)
>
> struct i915_vma *
> intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> -unsigned int rotation,
> +const struct i915_ggtt_view *view,
> bool uses_fence,
> unsigned long *out_flags)
> {
> struct drm_device *dev = fb->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> - struct i915_ggtt_view view;
> struct i915_vma *vma;
> unsigned int pinctl;
> u32 alignment;
> @@ -2095,8 +2094,6 @@ intel_pin_and_fence_fb_obj(struct
> drm_framebuffer *fb,
>
> alignment = intel_surf_alignment(fb, 0);
>
> - intel_fill_fb_ggtt_view(, fb, rotation);
> -
> /* Note that the w/a also requires 64 PTE of padding following
> the
>* bo. We currently fill all unused PTE with the shadow page
> and so
>* we should always have valid PTE following the scanout
> preventing
> @@ -2129,7 +2126,7 @@ intel_pin_and_fence_fb_obj(struct
> drm_framebuffer *fb,
> pinctl |= PIN_MAPPABLE;
>
> vma = i915_gem_object_pin_to_display_plane(obj,
> -alignment, ,
> pinctl);
> +alignment, view,
> pinctl);
> if (IS_ERR(vma))
> goto err;
>
> @@ -2851,13 +2848,15 @@ intel_find_initial_plane_obj(struct
> intel_crtc *intel_crtc,
> return;
>
> valid_fb:
> + intel_fill_fb_ggtt_view(_state->view, fb,
> + intel_state->base.rotation);
> intel_state->color_plane[0].stride =
> intel_fb_pitch(fb, 0, intel_state->base.rotation);
>
> mutex_lock(>struct_mutex);
> intel_state->vma =
> intel_pin_and_fence_fb_obj(fb,
> -primary->state->rotation,
> +_state->view,
> intel_plane_uses_fence(intel
> _state),
> _state->flags);
> mutex_unlock(>struct_mutex);
> @@ -3171,6 +3170,7 @@ int skl_check_plane_surface(const struct
> intel_crtc_state *crtc_state,
> unsigned int rotation = plane_state->base.rotation;
> int ret;
>
> + intel_fill_fb_ggtt_view(_state->view, fb, rotation);
> plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation);
> plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1,
> rotation);
>
> @@ -3315,6 +3315,7 @@ int i9xx_check_plane_surface(struct
> intel_plane_state *plane_state)
> int src_y = plane_state->base.src.y1 >> 16;
> u32 offset;
>
> + intel_fill_fb_ggtt_view(_state->view, fb, rotation);
> plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation);
>
> intel_add_fb_offsets(_x, _y, plane_state, 0);
> @@ -9691,6 +9692,7 @@ static int intel_check_cursor(struct
> intel_crtc_state *crtc_state,
> return -EINVAL;
> }
>
> + intel_fill_fb_ggtt_view(_state->view, fb, rotation);
> plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation);
>
> src_x = plane_state->base.src_x >> 16;
> @@ -13029,7 +13031,7 @@ static int intel_plane_pin_fb(struct
> intel_plane_state *plane_state)
> }
>
> vma = intel_pin_and_fence_fb_obj(fb,
> - plane_state->base.rotation,
> + _state->view,
>intel_plane_uses_fence(plane_s
> tate),
>_state->flags);
> if (IS_ERR(vma))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f647f9e1f671..d70276ff3d0e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -494,6 +494,7 @@ struct intel_atomic_state {
>
> struct intel_plane_state {
> struct drm_plane_state base;
> + struct i915_ggtt_view view;
> struct i915_vma *vma;
> unsigned long
Re: [Intel-gfx] [PATCH 14/18] drm/i915: Extract intel_cursor_check_surface()
On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Extract intel_cursor_check_surface() to better match the code layout
> of the other plane types.
Reviewed-by: José Roberto de Souza
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 47 ++--
>
> 1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b0e39dcf2fb8..b39c941b4791 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9667,13 +9667,37 @@ static bool intel_cursor_size_ok(const struct
> intel_plane_state *plane_state)
> height > 0 && height <= config->cursor_height;
> }
>
> -static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> - struct intel_plane_state *plane_state)
> +static int intel_cursor_check_surface(struct intel_plane_state
> *plane_state)
> {
> const struct drm_framebuffer *fb = plane_state->base.fb;
> unsigned int rotation = plane_state->base.rotation;
> int src_x, src_y;
> u32 offset;
> +
> + intel_fill_fb_ggtt_view(_state->view, fb, rotation);
> + plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation);
> +
> + src_x = plane_state->base.src_x >> 16;
> + src_y = plane_state->base.src_y >> 16;
> +
> + intel_add_fb_offsets(_x, _y, plane_state, 0);
> + offset = intel_plane_compute_aligned_offset(_x, _y,
> + plane_state, 0);
> +
> + if (src_x != 0 || src_y != 0) {
> + DRM_DEBUG_KMS("Arbitrary cursor panning not
> supported\n");
> + return -EINVAL;
> + }
> +
> + plane_state->color_plane[0].offset = offset;
> +
> + return 0;
> +}
> +
> +static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> + struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> int ret;
>
> if (fb && fb->modifier != DRM_FORMAT_MOD_LINEAR) {
> @@ -9696,22 +9720,9 @@ static int intel_check_cursor(struct
> intel_crtc_state *crtc_state,
> if (ret)
> return ret;
>
> - intel_fill_fb_ggtt_view(_state->view, fb, rotation);
> - plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation);
> -
> - src_x = plane_state->base.src_x >> 16;
> - src_y = plane_state->base.src_y >> 16;
> -
> - intel_add_fb_offsets(_x, _y, plane_state, 0);
> - offset = intel_plane_compute_aligned_offset(_x, _y,
> - plane_state, 0);
> -
> - if (src_x != 0 || src_y != 0) {
> - DRM_DEBUG_KMS("Arbitrary cursor panning not
> supported\n");
> - return -EINVAL;
> - }
> -
> - plane_state->color_plane[0].offset = offset;
> + ret = intel_cursor_check_surface(plane_state);
> + if (ret)
> + return ret;
>
> return 0;
> }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/18] drm/i915: Move chv rotation checks to plane->check()
On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Move the chv rotation vs. reflections checks to the plane->check()
> hook,
> away from the (now) platform agnostic
> intel_plane_atomic_check_with_state().
Reviewed-by: José Roberto de Souza
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 9 -
> drivers/gpu/drm/i915/intel_display.c | 4
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_sprite.c | 21 +
> 4 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 7c0873060934..aabebe0d2e9b 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -113,7 +113,6 @@ int intel_plane_atomic_check_with_state(const
> struct intel_crtc_state *old_crtc_
> struct intel_plane_state
> *intel_state)
> {
> struct drm_plane *plane = intel_state->base.plane;
> - struct drm_i915_private *dev_priv = to_i915(plane->dev);
> struct drm_plane_state *state = _state->base;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> int ret;
> @@ -121,14 +120,6 @@ int intel_plane_atomic_check_with_state(const
> struct intel_crtc_state *old_crtc_
> if (!intel_state->base.crtc && !old_plane_state->base.crtc)
> return 0;
>
> - /* CHV ignores the mirror bit when the rotate bit is set :( */
> - if (IS_CHERRYVIEW(dev_priv) &&
> - state->rotation & DRM_MODE_ROTATE_180 &&
> - state->rotation & DRM_MODE_REFLECT_X) {
> - DRM_DEBUG_KMS("Cannot rotate and reflect at the same
> time\n");
> - return -EINVAL;
> - }
> -
> intel_state->base.visible = false;
> ret = intel_plane->check_plane(crtc_state, intel_state);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index dbcc9a23eefa..b0e39dcf2fb8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3318,6 +3318,10 @@ i9xx_plane_check(struct intel_crtc_state
> *crtc_state,
> {
> int ret;
>
> + ret = chv_plane_check_rotation(plane_state);
> + if (ret)
> + return ret;
> +
> ret = drm_atomic_helper_check_plane_state(_state->base,
> _state->base,
> DRM_PLANE_HELPER_NO_S
> CALING,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 55f3537ba8f8..91b7fb6a07e1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2113,6 +2113,7 @@ unsigned int skl_plane_max_stride(struct
> intel_plane *plane,
> int skl_plane_check(struct intel_crtc_state *crtc_state,
> struct intel_plane_state *plane_state);
> int intel_plane_check_src_coordinates(struct intel_plane_state
> *plane_state);
> +int chv_plane_check_rotation(const struct intel_plane_state
> *plane_state);
>
> /* intel_tv.c */
> void intel_tv_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index f43884f76212..93aa355f00e3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1126,12 +1126,33 @@ g4x_sprite_check(struct intel_crtc_state
> *crtc_state,
> return 0;
> }
>
> +int chv_plane_check_rotation(const struct intel_plane_state
> *plane_state)
> +{
> + struct intel_plane *plane = to_intel_plane(plane_state-
> >base.plane);
> + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> + unsigned int rotation = plane_state->base.rotation;
> +
> + /* CHV ignores the mirror bit when the rotate bit is set :( */
> + if (IS_CHERRYVIEW(dev_priv) &&
> + rotation & DRM_MODE_ROTATE_180 &&
> + rotation & DRM_MODE_REFLECT_X) {
> + DRM_DEBUG_KMS("Cannot rotate and reflect at the same
> time\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int
> vlv_sprite_check(struct intel_crtc_state *crtc_state,
>struct intel_plane_state *plane_state)
> {
> int ret;
>
> + ret = chv_plane_check_rotation(plane_state);
> + if (ret)
> + return ret;
> +
> ret = drm_atomic_helper_check_plane_state(_state->base,
> _state->base,
> DRM_PLANE_HELPER_NO_S
> CALING,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/18] drm/i915: Move skl plane fb related checks into a better place
On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Move the skl+ specific framebuffer related checks from
> intel_plane_atomic_check_with_state() into a new function
> (skl_plane_check_fb()) which we'll simply call from the skl
> plane->check() hook.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 42
> drivers/gpu/drm/i915/intel_display.c | 12 --
> drivers/gpu/drm/i915/intel_sprite.c | 66
> +++
> 3 files changed, 66 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index eddcdd6e4b3b..7c0873060934 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -116,40 +116,11 @@ int intel_plane_atomic_check_with_state(const
> struct intel_crtc_state *old_crtc_
> struct drm_i915_private *dev_priv = to_i915(plane->dev);
> struct drm_plane_state *state = _state->base;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> - const struct drm_display_mode *adjusted_mode =
> - _state->base.adjusted_mode;
> int ret;
>
> if (!intel_state->base.crtc && !old_plane_state->base.crtc)
> return 0;
>
> - if (state->fb && drm_rotation_90_or_270(state->rotation)) {
> - struct drm_format_name_buf format_name;
> -
> - if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> - state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> - DRM_DEBUG_KMS("Y/Yf tiling required for
> 90/270!\n");
> - return -EINVAL;
> - }
> -
> - /*
> - * 90/270 is not allowed with RGB64 16:16:16:16,
> - * RGB 16-bit 5:6:5, and Indexed 8-bit.
> - * TBD: Add RGB64 case once its added in supported
> format list.
> - */
> - switch (state->fb->format->format) {
> - case DRM_FORMAT_C8:
> - case DRM_FORMAT_RGB565:
> - DRM_DEBUG_KMS("Unsupported pixel format %s for
> 90/270!\n",
> - drm_get_format_name(state->fb-
> >format->format,
> - _name)
> );
> - return -EINVAL;
> -
> - default:
> - break;
> - }
> - }
> -
> /* CHV ignores the mirror bit when the rotate bit is set :( */
> if (IS_CHERRYVIEW(dev_priv) &&
> state->rotation & DRM_MODE_ROTATE_180 &&
> @@ -163,19 +134,6 @@ int intel_plane_atomic_check_with_state(const
> struct intel_crtc_state *old_crtc_
> if (ret)
> return ret;
>
> - /*
> - * Y-tiling is not supported in IF-ID Interlace mode in
> - * GEN9 and above.
> - */
> - if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state-
> >base.enable &&
> - adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> - if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> - state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
> - DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-
> ID mode\n");
> - return -EINVAL;
> - }
> - }
> -
> /* FIXME pre-g4x don't work like this */
> if (state->visible)
> crtc_state->active_planes |= BIT(intel_plane->id);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 9b9eaeda15dd..2381b762d109 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3151,12 +3151,6 @@ static int skl_check_ccs_aux_surface(struct
> intel_plane_state *plane_state)
> int y = src_y / vsub;
> u32 offset;
>
> - if (plane_state->base.rotation & ~(DRM_MODE_ROTATE_0 |
> DRM_MODE_ROTATE_180)) {
> - DRM_DEBUG_KMS("RC support only with 0/180 degree
> rotation %x\n",
> - plane_state->base.rotation);
> - return -EINVAL;
> - }
> -
> intel_add_fb_offsets(, , plane_state, 1);
> offset = intel_plane_compute_aligned_offset(, ,
> plane_state, 1);
>
> @@ -3178,12 +3172,6 @@ int skl_check_plane_surface(const struct
> intel_crtc_state *crtc_state,
> plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation);
> plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1,
> rotation);
>
> - if (rotation & DRM_MODE_REFLECT_X &&
> - fb->modifier == DRM_FORMAT_MOD_LINEAR) {
> - DRM_DEBUG_KMS("horizontal flip is not supported with
> linear surface formats\n");
> - return -EINVAL;
> - }
> -
> if (!plane_state->base.visible)
> return 0;
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Always enable mmio debugging for CI
== Series Details == Series: drm/i915: Always enable mmio debugging for CI URL : https://patchwork.freedesktop.org/series/48659/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4703_full -> Patchwork_10011_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10011_full that come from known issues: === IGT changes === Issues hit igt@gem_exec_await@wide-contexts: shard-apl: PASS -> FAIL (fdo#105900, fdo#106680) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) Possible fixes igt@prime_vgem@basic-sync-default: shard-snb: INCOMPLETE (fdo#105411) -> PASS fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900 fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680 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_4703 -> Patchwork_10011 CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10011: e30e72498959089ae4c92134f68b6f589d854981 @ 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_10011/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri, Aug 24, 2018 at 06:40:03PM +0200, Michal Hocko wrote:
> On Fri 24-08-18 11:12:40, Jerome Glisse wrote:
> [...]
> > I am fine with Michal patch, i already said so couple month ago first time
> > this discussion did pop up, Michal you can add:
> >
> > Reviewed-by: Jérôme Glisse
>
> So I guess the below is the patch you were talking about?
>
> From f7ac75277d526dccd011f343818dc6af627af2af Mon Sep 17 00:00:00 2001
> From: Michal Hocko
> Date: Fri, 24 Aug 2018 15:32:24 +0200
> Subject: [PATCH] mm, mmu_notifier: be explicit about range invalition
> non-blocking mode
>
> If invalidate_range_start is called for !blocking mode then all
> callbacks have to guarantee they will no block/sleep. The same obviously
> applies to invalidate_range_end because this operation pairs with the
> former and they are called from the same context. Make sure this is
> appropriately documented.
In my branch i already updated HMM to be like other existing user
ie all blocking operation in the start callback. But yes it would
be wise to added such comments.
>
> Signed-off-by: Michal Hocko
> ---
> include/linux/mmu_notifier.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> index 133ba78820ee..698e371aafe3 100644
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -153,7 +153,9 @@ struct mmu_notifier_ops {
>*
>* If blockable argument is set to false then the callback cannot
>* sleep and has to return with -EAGAIN. 0 should be returned
> - * otherwise.
> + * otherwise. Please note that if invalidate_range_start approves
> + * a non-blocking behavior then the same applies to
> + * invalidate_range_end.
>*
>*/
> int (*invalidate_range_start)(struct mmu_notifier *mn,
> --
> 2.18.0
>
> --
> Michal Hocko
> SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dsc: Fix PPS register definition macros for 2nd VDSC engine
>-Original Message-
>From: Navare, Manasi D
>Sent: Thursday, August 23, 2018 6:48 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Navare, Manasi D ; Srivatsa, Anusha
>
>Subject: [PATCH] drm/i915/dsc: Fix PPS register definition macros for 2nd VDSC
>engine
>
>This patch fixes the PPS4 and PPS register definition macros that were
>resulting
>into an incorect MMIO address.
>
>Fixes: 2efbb2f099fb ("i915/dp/dsc: Add DSC PPS register definitions")
>Cc: Anusha Srivatsa
>Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
>---
> drivers/gpu/drm/i915/i915_reg.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 41ab5b56ee52..64d7e675f7e8 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -10488,7 +10488,7 @@ enum skl_power_gate {
>
>_ICL_DSC0_PICTURE_PARAMETER_SET_4_PB, \
>
>_ICL_DSC0_PICTURE_PARAMETER_SET_4_PC)
> #define ICL_DSC1_PICTURE_PARAMETER_SET_4(pipe)_MMIO_PIPE((pipe) -
>PIPE_B, \
>-
>_ICL_DSC0_PICTURE_PARAMETER_SET_4_PB, \
>+
>_ICL_DSC1_PICTURE_PARAMETER_SET_4_PB, \
>
>_ICL_DSC1_PICTURE_PARAMETER_SET_4_PC)
> #define DSC_INITIAL_DEC_DELAY(dec_delay) ((dec_delay) << 16)
> #define DSC_INITIAL_XMIT_DELAY(xmit_delay) ((xmit_delay) << 0)
>@@ -10503,7 +10503,7 @@ enum skl_power_gate {
>
>_ICL_DSC0_PICTURE_PARAMETER_SET_5_PB, \
>
>_ICL_DSC0_PICTURE_PARAMETER_SET_5_PC)
> #define ICL_DSC1_PICTURE_PARAMETER_SET_5(pipe)_MMIO_PIPE((pipe) -
>PIPE_B, \
>-
>_ICL_DSC1_PICTURE_PARAMETER_SET_5_PC, \
>+
>_ICL_DSC1_PICTURE_PARAMETER_SET_5_PB, \
>
>_ICL_DSC1_PICTURE_PARAMETER_SET_5_PC)
> #define DSC_SCALE_DEC_INT(scale_dec) ((scale_dec) << 16)
> #define DSC_SCALE_INC_INT(scale_inc) ((scale_inc) << 0)
>--
>2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Always enable mmio debugging for CI
== Series Details ==
Series: drm/i915: Always enable mmio debugging for CI
URL : https://patchwork.freedesktop.org/series/48659/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4703 -> Patchwork_10011 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48659/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10011 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_coherency:
fi-gdg-551: PASS -> DMESG-FAIL (fdo#107164)
igt@drv_selftest@live_objects:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
fi-cnl-psr: PASS -> INCOMPLETE (fdo#105086)
fi-bxt-j4205: PASS -> INCOMPLETE (fdo#103927)
fi-glk-j4005: PASS -> INCOMPLETE (k.org#198133, fdo#103359)
igt@gem_mmap@basic-small-bo:
fi-glk-dsi: PASS -> INCOMPLETE (k.org#198133, fdo#103359)
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: PASS -> DMESG-FAIL (fdo#102614)
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@read-crc-pipe-a:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362)
{igt@kms_psr@primary_page_flip}:
{fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3
Possible fixes
igt@drv_selftest@live_hangcheck:
{fi-bdw-samus}: DMESG-FAIL (fdo#106560) -> PASS
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@gem_sync@basic-many-each:
{fi-byt-clapper}: FAIL -> PASS
{igt@pm_rpm@module-reload}:
{fi-bsw-kefka}: DMESG-WARN -> PASS
fi-bsw-n3050: DMESG-WARN -> PASS
fi-byt-j1900: DMESG-WARN -> PASS
fi-byt-n2820: DMESG-WARN -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (53 -> 49) ==
Additional (1): fi-icl-u
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4703 -> Patchwork_10011
CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10011: e30e72498959089ae4c92134f68b6f589d854981 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
e30e72498959 drm/i915: Always enable mmio debugging for CI
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10011/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for mm, oom: distinguish blockable mode for mmu notifiers (rev14)
== Series Details == Series: mm, oom: distinguish blockable mode for mmu notifiers (rev14) URL : https://patchwork.freedesktop.org/series/45263/ State : failure == Summary == Applying: mm, oom: distinguish blockable mode for mmu notifiers error: sha1 information is lacking or useless (include/linux/mmu_notifier.h). error: could not build fake ancestor Patch failed at 0001 mm, oom: distinguish blockable mode for mmu notifiers Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 11:12:40, Jerome Glisse wrote:
[...]
> I am fine with Michal patch, i already said so couple month ago first time
> this discussion did pop up, Michal you can add:
>
> Reviewed-by: Jérôme Glisse
So I guess the below is the patch you were talking about?
From f7ac75277d526dccd011f343818dc6af627af2af Mon Sep 17 00:00:00 2001
From: Michal Hocko
Date: Fri, 24 Aug 2018 15:32:24 +0200
Subject: [PATCH] mm, mmu_notifier: be explicit about range invalition
non-blocking mode
If invalidate_range_start is called for !blocking mode then all
callbacks have to guarantee they will no block/sleep. The same obviously
applies to invalidate_range_end because this operation pairs with the
former and they are called from the same context. Make sure this is
appropriately documented.
Signed-off-by: Michal Hocko
---
include/linux/mmu_notifier.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 133ba78820ee..698e371aafe3 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -153,7 +153,9 @@ struct mmu_notifier_ops {
*
* If blockable argument is set to false then the callback cannot
* sleep and has to return with -EAGAIN. 0 should be returned
-* otherwise.
+* otherwise. Please note that if invalidate_range_start approves
+* a non-blocking behavior then the same applies to
+* invalidate_range_end.
*
*/
int (*invalidate_range_start)(struct mmu_notifier *mn,
--
2.18.0
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 23:52:25, Tetsuo Handa wrote:
> On 2018/08/24 22:32, Michal Hocko wrote:
> > On Fri 24-08-18 22:02:23, Tetsuo Handa wrote:
> >> I worry that (currently
> >> out-of-tree) users of this API are involving work / recursion.
> >
> > I do not give a slightest about out-of-tree modules. They will have to
> > accomodate to the new API. I have no problems to extend the
> > documentation and be explicit about this expectation.
>
> You don't need to care about out-of-tree modules. But you need to hear from
> mm/hmm.c authors/maintainers when making changes for mmu-notifiers.
>
> > diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> > index 133ba78820ee..698e371aafe3 100644
> > --- a/include/linux/mmu_notifier.h
> > +++ b/include/linux/mmu_notifier.h
> > @@ -153,7 +153,9 @@ struct mmu_notifier_ops {
> > *
> > * If blockable argument is set to false then the callback cannot
> > * sleep and has to return with -EAGAIN. 0 should be returned
> > -* otherwise.
> > +* otherwise. Please note that if invalidate_range_start approves
> > +* a non-blocking behavior then the same applies to
> > +* invalidate_range_end.
>
> Prior to 93065ac753e44438 ("mm, oom: distinguish blockable mode for mmu
> notifiers"), whether to utilize MMU_INVALIDATE_DOES_NOT_BLOCK was up to
> mmu-notifiers users.
>
> -* If both of these callbacks cannot block, and invalidate_range
> -* cannot block, mmu_notifier_ops.flags should have
> -* MMU_INVALIDATE_DOES_NOT_BLOCK set.
> +* If blockable argument is set to false then the callback
> cannot
> +* sleep and has to return with -EAGAIN. 0 should be returned
> +* otherwise.
>
> Even out-of-tree mmu-notifiers users had rights not to accommodate (i.e.
> make changes) immediately by not setting MMU_INVALIDATE_DOES_NOT_BLOCK.
>
> Now we are in a merge window. And we noticed a possibility that out-of-tree
> mmu-notifiers users might have trouble with making changes immediately in
> order
> to follow 93065ac753e44438 if expectation for mm/hmm.c changes immediately.
> And you are trying to ignore such possibility by just updating expected
> behavior
> description instead of giving out-of-tree users a grace period to check and
> update
> their code.
This is just ridiculous. I have no idea what you are trying to achieve
here but please read through Documentation/process/stable-api-nonsense.rst
before you try to make strong statements again.
I have changed an in-kernel interface. I have gone through all users and
fixed them up. It is really appreciated to double check after me and I
am willing to fix up any fallouts. But that is just about it. I do not
get a whit about _any_ out of tree drivers when changing the interface.
I am willing to answer any questions regarding this change so developers
of those drivers know how to do their change properly but doing so is
completely their business.
> >> and keeps "all operations protected by hmm->mirrors_sem held for write are
> >> atomic". This suggests that "some operations protected by hmm->mirrors_sem
> >> held
> >> for read will sleep (and in the worst case involves memory allocation
> >> dependency)".
> >
> > Yes and so what? The clear expectation is that neither of the range
> > notifiers do not sleep in !blocking mode. I really fail to see what you
> > are trying to say.
>
> I'm saying "Get ACK from Jérôme about mm/hmm.c changes".
HMM is a library layer for other driver, until those get merged the same
applies for them as well.
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri, Aug 24, 2018 at 11:52:25PM +0900, Tetsuo Handa wrote:
> On 2018/08/24 22:32, Michal Hocko wrote:
> > On Fri 24-08-18 22:02:23, Tetsuo Handa wrote:
> >> I worry that (currently
> >> out-of-tree) users of this API are involving work / recursion.
> >
> > I do not give a slightest about out-of-tree modules. They will have to
> > accomodate to the new API. I have no problems to extend the
> > documentation and be explicit about this expectation.
>
> You don't need to care about out-of-tree modules. But you need to hear from
> mm/hmm.c authors/maintainers when making changes for mmu-notifiers.
>
> > diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> > index 133ba78820ee..698e371aafe3 100644
> > --- a/include/linux/mmu_notifier.h
> > +++ b/include/linux/mmu_notifier.h
> > @@ -153,7 +153,9 @@ struct mmu_notifier_ops {
> > *
> > * If blockable argument is set to false then the callback cannot
> > * sleep and has to return with -EAGAIN. 0 should be returned
> > -* otherwise.
> > +* otherwise. Please note that if invalidate_range_start approves
> > +* a non-blocking behavior then the same applies to
> > +* invalidate_range_end.
>
> Prior to 93065ac753e44438 ("mm, oom: distinguish blockable mode for mmu
> notifiers"), whether to utilize MMU_INVALIDATE_DOES_NOT_BLOCK was up to
> mmu-notifiers users.
>
> -* If both of these callbacks cannot block, and invalidate_range
> -* cannot block, mmu_notifier_ops.flags should have
> -* MMU_INVALIDATE_DOES_NOT_BLOCK set.
> +* If blockable argument is set to false then the callback
> cannot
> +* sleep and has to return with -EAGAIN. 0 should be returned
> +* otherwise.
>
> Even out-of-tree mmu-notifiers users had rights not to accommodate (i.e.
> make changes) immediately by not setting MMU_INVALIDATE_DOES_NOT_BLOCK.
>
> Now we are in a merge window. And we noticed a possibility that out-of-tree
> mmu-notifiers users might have trouble with making changes immediately in
> order
> to follow 93065ac753e44438 if expectation for mm/hmm.c changes immediately.
> And you are trying to ignore such possibility by just updating expected
> behavior
> description instead of giving out-of-tree users a grace period to check and
> update
> their code.
Intention is that 99% of HMM users will be upstream as long as they are
not people shouldn't worry. We have been working on nouveau to use it
for the last year or so. Many bits were added in 4.16, 4.17, 4.18 and i
hope it will all be there in 4.20/4.21 timeframe.
See my other mail for list of other users.
>
> >> and keeps "all operations protected by hmm->mirrors_sem held for write are
> >> atomic". This suggests that "some operations protected by hmm->mirrors_sem
> >> held
> >> for read will sleep (and in the worst case involves memory allocation
> >> dependency)".
> >
> > Yes and so what? The clear expectation is that neither of the range
> > notifiers do not sleep in !blocking mode. I really fail to see what you
> > are trying to say.
>
> I'm saying "Get ACK from Jérôme about mm/hmm.c changes".
I am fine with Michal patch, i already said so couple month ago first time
this discussion did pop up, Michal you can add:
Reviewed-by: Jérôme Glisse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri, Aug 24, 2018 at 02:33:41PM +0200, Michal Hocko wrote: > On Fri 24-08-18 14:18:44, Christian König wrote: > > Am 24.08.2018 um 14:03 schrieb Michal Hocko: > > > On Fri 24-08-18 13:57:52, Christian König wrote: > > > > Am 24.08.2018 um 13:52 schrieb Michal Hocko: > > > > > On Fri 24-08-18 13:43:16, Christian König wrote: > > > [...] > > > > > > That won't work like this there might be multiple > > > > > > invalidate_range_start()/invalidate_range_end() pairs open at the > > > > > > same time. > > > > > > E.g. the lock might be taken recursively and that is illegal for a > > > > > > rw_semaphore. > > > > > I am not sure I follow. Are you saying that one invalidate_range might > > > > > trigger another one from the same path? > > > > No, but what can happen is: > > > > > > > > invalidate_range_start(A,B); > > > > invalidate_range_start(C,D); > > > > ... > > > > invalidate_range_end(C,D); > > > > invalidate_range_end(A,B); > > > > > > > > Grabbing the read lock twice would be illegal in this case. > > > I am sorry but I still do not follow. What is the context the two are > > > called from? > > > > I don't have the slightest idea. > > > > > Can you give me an example. I simply do not see it in the > > > code, mostly because I am not familiar with it. > > > > I'm neither. > > > > We stumbled over that by pure observation and after discussing the problem > > with Jerome came up with this solution. > > > > No idea where exactly that case comes from, but I can confirm that it indeed > > happens. > > Thiking about it some more, I can imagine that a notifier callback which > performs an allocation might trigger a memory reclaim and that in turn > might trigger a notifier to be invoked and recurse. But notifier > shouldn't really allocate memory. They are called from deep MM code > paths and this would be extremely deadlock prone. Maybe Jerome can come > up some more realistic scenario. If not then I would propose to simplify > the locking here. We have lockdep to catch self deadlocks and it is > always better to handle a specific issue rather than having a code > without a clear indication how it can recurse. Multiple concurrent mmu notifier, for overlapping range or not, is common (each concurrent threads can trigger some). So you might have multiple invalidate_range_start() in flight for same mm and thus might complete in different order (invalidate_range_end()). IIRC this is what this lock was trying to protect against. I can't think of a reason for recursive mmu notifier call right now. I will ponder see if i remember something about it. Cheers, Jérôme ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG] i915 HDMI connector status is connected after disconnection
On Wed, 22 Aug 2018, Chris Chiu wrote: > On Fri, Jul 6, 2018 at 2:44 PM, Chris Chiu wrote: >> On Thu, Jul 5, 2018 at 10:40 PM, Ville Syrjälä >> wrote: >>> On Thu, Jul 05, 2018 at 03:58:36PM +0800, Chris Chiu wrote: Hi, We have few ASUS laptops X705FD (The new WiskyLake), X560UD (intel i5-8250U), X530UN (intel i7-8550U) share the same problem, which is the HDMI connector status stays connected even the HDMI cable has been unplugged. Look into the "/sys/class/drm/card0-HDMI-A-1/status" for checking the status while plug/unplug the HDMI, it shows "disconnected" before plug in HDMI cable, then switch to "connected" after plugin, and still stay "connected" after unplug. This would cause the audio output path cannot correctly switch from HDMI to internal speaker after unplugging the HDMI. I then try to verify with the latest kernel 4.18.0-rc3+, the bug still present. The full "dmesg" log is here. https://gist.github.com/mschiu77/d761d7c5cf191b7868d4d7788ae087f1 The HDMI cable is plugged in at ~26th second. "[ 26.214371] [drm:drm_detect_monitor_audio [drm]] Monitor has basic audio support" then unplug the HDMI at ~73th second. "[ 73.328361] [drm:drm_detect_monitor_audio [drm]] Monitor has basic audio support" Please advise what I can do to fix this. Thanks >>> >>> Pull the cable out faster? >>> >>> I presume this is the same old case of hpd disconnecting slightly >>> before ddc and we still manage to read the EDID when processing >>> the hpd irq. We kinda tried to fix that with the live status >>> check but that thing failed spectacularly. >>> >>> -- >>> Ville Syrjälä >>> Intel > > There's a patch https://bugs.freedesktop.org/show_bug.cgi?id=107125#c8. > And I verified on the X705FD/X560UD which were easy to reproduce, the patch > works as expected. Can anyone kindly give comments about this patch? > We can do anything to help fix this issue upstream. Thanks Seems like a hack. Should look into hw based debouncing or a slight delay in the hotplug work processing I think. BR, Jani. > > Chris > >> Thanks for the suggestion. I tried pulling the cable out faster, the status >> shows correctly. I also tried branch drm-tip of >> https://cgit.freedesktop.org/drm/drm-tip >> but the symptom persists. >> >> Anything I can help here? Or any old commit/patch I can try to do some >> experiments? >> >> Chris -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On 2018/08/24 22:32, Michal Hocko wrote:
> On Fri 24-08-18 22:02:23, Tetsuo Handa wrote:
>> I worry that (currently
>> out-of-tree) users of this API are involving work / recursion.
>
> I do not give a slightest about out-of-tree modules. They will have to
> accomodate to the new API. I have no problems to extend the
> documentation and be explicit about this expectation.
You don't need to care about out-of-tree modules. But you need to hear from
mm/hmm.c authors/maintainers when making changes for mmu-notifiers.
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> index 133ba78820ee..698e371aafe3 100644
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -153,7 +153,9 @@ struct mmu_notifier_ops {
>*
>* If blockable argument is set to false then the callback cannot
>* sleep and has to return with -EAGAIN. 0 should be returned
> - * otherwise.
> + * otherwise. Please note that if invalidate_range_start approves
> + * a non-blocking behavior then the same applies to
> + * invalidate_range_end.
Prior to 93065ac753e44438 ("mm, oom: distinguish blockable mode for mmu
notifiers"), whether to utilize MMU_INVALIDATE_DOES_NOT_BLOCK was up to
mmu-notifiers users.
-* If both of these callbacks cannot block, and invalidate_range
-* cannot block, mmu_notifier_ops.flags should have
-* MMU_INVALIDATE_DOES_NOT_BLOCK set.
+* If blockable argument is set to false then the callback
cannot
+* sleep and has to return with -EAGAIN. 0 should be returned
+* otherwise.
Even out-of-tree mmu-notifiers users had rights not to accommodate (i.e.
make changes) immediately by not setting MMU_INVALIDATE_DOES_NOT_BLOCK.
Now we are in a merge window. And we noticed a possibility that out-of-tree
mmu-notifiers users might have trouble with making changes immediately in order
to follow 93065ac753e44438 if expectation for mm/hmm.c changes immediately.
And you are trying to ignore such possibility by just updating expected behavior
description instead of giving out-of-tree users a grace period to check and
update
their code.
>> and keeps "all operations protected by hmm->mirrors_sem held for write are
>> atomic". This suggests that "some operations protected by hmm->mirrors_sem
>> held
>> for read will sleep (and in the worst case involves memory allocation
>> dependency)".
>
> Yes and so what? The clear expectation is that neither of the range
> notifiers do not sleep in !blocking mode. I really fail to see what you
> are trying to say.
I'm saying "Get ACK from Jérôme about mm/hmm.c changes".
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri, Aug 24, 2018 at 07:54:19PM +0900, Tetsuo Handa wrote:
> Two more worries for this patch.
[...]
>
> > --- a/mm/hmm.c
> > +++ b/mm/hmm.c
> > @@ -177,16 +177,19 @@ static void hmm_release(struct mmu_notifier *mn,
> > struct mm_struct *mm)
> > up_write(>mirrors_sem);
> > }
> >
> > -static void hmm_invalidate_range_start(struct mmu_notifier *mn,
> > +static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> >struct mm_struct *mm,
> >unsigned long start,
> > - unsigned long end)
> > + unsigned long end,
> > + bool blockable)
> > {
> > struct hmm *hmm = mm->hmm;
> >
> > VM_BUG_ON(!hmm);
> >
> > atomic_inc(>sequence);
> > +
> > + return 0;
> > }
> >
> > static void hmm_invalidate_range_end(struct mmu_notifier *mn,
>
> This assumes that hmm_invalidate_range_end() does not have memory
> allocation dependency. But hmm_invalidate_range() from
> hmm_invalidate_range_end() involves
>
> down_read(>mirrors_sem);
> list_for_each_entry(mirror, >mirrors, list)
> mirror->ops->sync_cpu_device_pagetables(mirror, action,
> start, end);
> up_read(>mirrors_sem);
>
> sequence. What is surprising is that there is no in-tree user who assigns
> sync_cpu_device_pagetables field.
>
> $ grep -Fr sync_cpu_device_pagetables *
> Documentation/vm/hmm.rst: /* sync_cpu_device_pagetables() - synchronize
> page tables
> include/linux/hmm.h: * will get callbacks through
> sync_cpu_device_pagetables() operation (see
> include/linux/hmm.h:/* sync_cpu_device_pagetables() - synchronize page
> tables
> include/linux/hmm.h:void (*sync_cpu_device_pagetables)(struct
> hmm_mirror *mirror,
> include/linux/hmm.h: * hmm_mirror_ops.sync_cpu_device_pagetables()
> callback, so that CPU page
> mm/hmm.c: mirror->ops->sync_cpu_device_pagetables(mirror,
> action,
>
> That is, this API seems to be currently used by only out-of-tree users. Since
> we can't check that nobody has memory allocation dependency, I think that
> hmm_invalidate_range_start() should return -EAGAIN if blockable == false for
> now.
So you can see update and user of this there:
https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-intel-v00
https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-nouveau-v01
https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-radeon-v00
I am still working on Mellanox and AMD GPU patchset.
I will post the HMM changes that adapt to Michal shortly as anyway
thus have been sufficiently tested by now.
https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-4.20=78785dcb5ba0924c2c5e7be027793f99ebbc39f3
https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-4.20=4fc25571dc893f2b278e90cda9e71e139e01de70
Cheers,
Jérôme
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for mm, oom: distinguish blockable mode for mmu notifiers (rev13)
== Series Details == Series: mm, oom: distinguish blockable mode for mmu notifiers (rev13) URL : https://patchwork.freedesktop.org/series/45263/ State : failure == Summary == Applying: mm, oom: distinguish blockable mode for mmu notifiers error: sha1 information is lacking or useless (drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c). error: could not build fake ancestor Patch failed at 0001 mm, oom: distinguish blockable mode for mmu notifiers Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 15:44:03, Christian König wrote:
> Am 24.08.2018 um 15:40 schrieb Michal Hocko:
> > On Fri 24-08-18 15:28:33, Christian König wrote:
> > > Am 24.08.2018 um 15:24 schrieb Michal Hocko:
> > > > On Fri 24-08-18 15:10:08, Christian König wrote:
> > > > > Am 24.08.2018 um 15:01 schrieb Michal Hocko:
> > > > > > On Fri 24-08-18 14:52:26, Christian König wrote:
> > > > > > > Am 24.08.2018 um 14:33 schrieb Michal Hocko:
> > > > > > [...]
> > > > > > > > Thiking about it some more, I can imagine that a notifier
> > > > > > > > callback which
> > > > > > > > performs an allocation might trigger a memory reclaim and that
> > > > > > > > in turn
> > > > > > > > might trigger a notifier to be invoked and recurse. But notifier
> > > > > > > > shouldn't really allocate memory. They are called from deep MM
> > > > > > > > code
> > > > > > > > paths and this would be extremely deadlock prone. Maybe Jerome
> > > > > > > > can come
> > > > > > > > up some more realistic scenario. If not then I would propose to
> > > > > > > > simplify
> > > > > > > > the locking here. We have lockdep to catch self deadlocks and
> > > > > > > > it is
> > > > > > > > always better to handle a specific issue rather than having a
> > > > > > > > code
> > > > > > > > without a clear indication how it can recurse.
> > > > > > > Well I agree that we should probably fix that, but I have some
> > > > > > > concerns to
> > > > > > > remove the existing workaround.
> > > > > > >
> > > > > > > See we added that to get rid of a real problem in a customer
> > > > > > > environment and
> > > > > > > I don't want to that to show up again.
> > > > > > It would really help to know more about that case and fix it
> > > > > > properly
> > > > > > rather than workaround it like this. Anyway, let me think how to
> > > > > > handle
> > > > > > the non-blocking notifier invocation then. I was not able to come up
> > > > > > with anything remotely sane yet.
> > > > > With avoiding allocating memory in the write lock path I don't see an
> > > > > issue
> > > > > any more with that.
> > > > >
> > > > > All what the write lock path does now is adding items to a linked
> > > > > lists,
> > > > > arrays etc
> > > > Can we change it to non-sleepable lock then?
> > > No, the write side doesn't sleep any more, but the read side does.
> > >
> > > See amdgpu_mn_invalidate_node() and that is where you actually need to
> > > handle the non-blocking flag correctly.
> > Ohh, right you are. We already handle that by bailing out before calling
> > amdgpu_mn_invalidate_node in !blockable mode.
>
> Yeah, that is sufficient.
>
> It could be improved because we have something like 90% chance that
> amdgpu_mn_invalidate_node() actually doesn't need to do anything.
>
> But I can take care of that when the patch set has landed.
>
> > So does this looks good to you?
>
> Yeah, that looks perfect to me. Reviewed-by: Christian König
>
Cool! Thanks for your guidance and patience with me. Here is the full
patch. Feel free to take it and route per your preference.
From 4e297bf5a55906ee369d70bee9f7beeb3cba74bb Mon Sep 17 00:00:00 2001
From: Michal Hocko
Date: Fri, 24 Aug 2018 15:45:52 +0200
Subject: [PATCH] drm/amd: clarify amdgpu_mn_read_lock !blocking mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Tetsuo has noticed that 93065ac753e4 ("mm, oom: distinguish blockable
mode for mmu notifiers") !blocking case for amdgpu_mn_read_lock is
incomplete because it might sleep on the notifier lock. This is true
but as it turned out from the discussion with Christian this doesn't
really matter.
The amd notifier lock doesn't block in the exclusive mode. It only ever
sleeps with the read lock inside amdgpu_mn_invalidate_node. That one
is not called in !blockable state so while we might sleep on notifier
read_lock this will only be for a short while. The same applies on the
notifier lock.
Therefore remove blockable handling from amdgpu_mn_read_lock and
document it properly.
Noticed-by: Tetsuo Handa
Reviewed-by: Christian König
Signed-off-by: Michal Hocko
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..48fa152231be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -180,11 +180,15 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
- return -EAGAIN;
-
+ /*
+* We can take sleepable lock even on !blockable mode because
+* read_lock is only ever take from this path and the notifier
+* lock never really sleeps. In fact the only reason why the
+* later is
Re: [Intel-gfx] [PATCH] drm/i915: Always enable mmio debugging for CI
Chris Wilson writes: > The default behaviour is to periodically check for a mmio access error, > and once detected enable mmio access checking. However this is useless > if the error only occurs once. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala This could help onto zeroing on those handful of unclaimed mmio bugs we have atm. Also this will have a nice effect on preventing any simple ones getting past under our radar. Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/Kconfig.debug | 12 > drivers/gpu/drm/i915/i915_params.h | 8 +++- > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug > b/drivers/gpu/drm/i915/Kconfig.debug > index 9e36ffb5eb7c..ca947ed64d4e 100644 > --- a/drivers/gpu/drm/i915/Kconfig.debug > +++ b/drivers/gpu/drm/i915/Kconfig.debug > @@ -31,6 +31,7 @@ config DRM_I915_DEBUG > select DRM_I915_SW_FENCE_DEBUG_OBJECTS > select DRM_I915_SELFTEST > select DRM_I915_DEBUG_RUNTIME_PM > + select DRM_I915_DEBUG_MMIO > default n > help >Choose this option to turn on extra driver debugging that may > affect > @@ -40,6 +41,17 @@ config DRM_I915_DEBUG > >If in doubt, say "N". > > +config DRM_I915_DEBUG_MMIO > +bool "Always insert extra checks around mmio access" > +default n > +help > + Always enables the extra sanity checks (extra register reads) > + around every mmio (register) access that will slow the system down. > + > + Recommended for driver developers only. > + > + If in doubt, say "N". > + > config DRM_I915_DEBUG_GEM > bool "Insert extra checks into the GEM internals" > default n > diff --git a/drivers/gpu/drm/i915/i915_params.h > b/drivers/gpu/drm/i915/i915_params.h > index 6c4d4a21474b..6a3308a5dfe1 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -33,6 +33,12 @@ struct drm_printer; > #define ENABLE_GUC_SUBMISSIONBIT(0) > #define ENABLE_GUC_LOAD_HUC BIT(1) > > +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO) > +#define MMIO_DEBUG_DFL -1 > +#else > +#define MMIO_DEBUG_DFL 0 > +#endif > + > #define I915_PARAMS_FOR_EACH(param) \ > param(char *, vbt_firmware, NULL) \ > param(int, modeset, -1) \ > @@ -51,7 +57,7 @@ struct drm_printer; > param(char *, guc_firmware_path, NULL) \ > param(char *, huc_firmware_path, NULL) \ > param(char *, dmc_firmware_path, NULL) \ > - param(int, mmio_debug, 0) \ > + param(int, mmio_debug, MMIO_DEBUG_DFL) \ > param(int, edp_vswing, 0) \ > param(int, reset, 2) \ > param(unsigned int, inject_load_failure, 0) \ > -- > 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] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 15:40 schrieb Michal Hocko:
On Fri 24-08-18 15:28:33, Christian König wrote:
Am 24.08.2018 um 15:24 schrieb Michal Hocko:
On Fri 24-08-18 15:10:08, Christian König wrote:
Am 24.08.2018 um 15:01 schrieb Michal Hocko:
On Fri 24-08-18 14:52:26, Christian König wrote:
Am 24.08.2018 um 14:33 schrieb Michal Hocko:
[...]
Thiking about it some more, I can imagine that a notifier callback which
performs an allocation might trigger a memory reclaim and that in turn
might trigger a notifier to be invoked and recurse. But notifier
shouldn't really allocate memory. They are called from deep MM code
paths and this would be extremely deadlock prone. Maybe Jerome can come
up some more realistic scenario. If not then I would propose to simplify
the locking here. We have lockdep to catch self deadlocks and it is
always better to handle a specific issue rather than having a code
without a clear indication how it can recurse.
Well I agree that we should probably fix that, but I have some concerns to
remove the existing workaround.
See we added that to get rid of a real problem in a customer environment and
I don't want to that to show up again.
It would really help to know more about that case and fix it properly
rather than workaround it like this. Anyway, let me think how to handle
the non-blocking notifier invocation then. I was not able to come up
with anything remotely sane yet.
With avoiding allocating memory in the write lock path I don't see an issue
any more with that.
All what the write lock path does now is adding items to a linked lists,
arrays etc
Can we change it to non-sleepable lock then?
No, the write side doesn't sleep any more, but the read side does.
See amdgpu_mn_invalidate_node() and that is where you actually need to
handle the non-blocking flag correctly.
Ohh, right you are. We already handle that by bailing out before calling
amdgpu_mn_invalidate_node in !blockable mode.
Yeah, that is sufficient.
It could be improved because we have something like 90% chance that
amdgpu_mn_invalidate_node() actually doesn't need to do anything.
But I can take care of that when the patch set has landed.
So does this looks good to
you?
Yeah, that looks perfect to me. Reviewed-by: Christian König
Thanks,
Christian.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..48fa152231be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -180,11 +180,15 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
- return -EAGAIN;
-
+ /*
+* We can take sleepable lock even on !blockable mode because
+* read_lock is only ever take from this path and the notifier
+* lock never really sleeps. In fact the only reason why the
+* later is sleepable is because the notifier itself might sleep
+* in amdgpu_mn_invalidate_node but blockable mode is handled
+* before calling into that path.
+*/
+ mutex_lock(>read_lock);
if (atomic_inc_return(>recursion) == 1)
down_read_non_owner(>lock);
mutex_unlock(>read_lock);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for mm, oom: distinguish blockable mode for mmu notifiers (rev12)
== Series Details == Series: mm, oom: distinguish blockable mode for mmu notifiers (rev12) URL : https://patchwork.freedesktop.org/series/45263/ State : failure == Summary == Applying: mm, oom: distinguish blockable mode for mmu notifiers error: sha1 information is lacking or useless (drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c). error: could not build fake ancestor Patch failed at 0001 mm, oom: distinguish blockable mode for mmu notifiers Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 15:28:33, Christian König wrote:
> Am 24.08.2018 um 15:24 schrieb Michal Hocko:
> > On Fri 24-08-18 15:10:08, Christian König wrote:
> > > Am 24.08.2018 um 15:01 schrieb Michal Hocko:
> > > > On Fri 24-08-18 14:52:26, Christian König wrote:
> > > > > Am 24.08.2018 um 14:33 schrieb Michal Hocko:
> > > > [...]
> > > > > > Thiking about it some more, I can imagine that a notifier callback
> > > > > > which
> > > > > > performs an allocation might trigger a memory reclaim and that in
> > > > > > turn
> > > > > > might trigger a notifier to be invoked and recurse. But notifier
> > > > > > shouldn't really allocate memory. They are called from deep MM code
> > > > > > paths and this would be extremely deadlock prone. Maybe Jerome can
> > > > > > come
> > > > > > up some more realistic scenario. If not then I would propose to
> > > > > > simplify
> > > > > > the locking here. We have lockdep to catch self deadlocks and it is
> > > > > > always better to handle a specific issue rather than having a code
> > > > > > without a clear indication how it can recurse.
> > > > > Well I agree that we should probably fix that, but I have some
> > > > > concerns to
> > > > > remove the existing workaround.
> > > > >
> > > > > See we added that to get rid of a real problem in a customer
> > > > > environment and
> > > > > I don't want to that to show up again.
> > > > It would really help to know more about that case and fix it properly
> > > > rather than workaround it like this. Anyway, let me think how to handle
> > > > the non-blocking notifier invocation then. I was not able to come up
> > > > with anything remotely sane yet.
> > > With avoiding allocating memory in the write lock path I don't see an
> > > issue
> > > any more with that.
> > >
> > > All what the write lock path does now is adding items to a linked lists,
> > > arrays etc
> > Can we change it to non-sleepable lock then?
>
> No, the write side doesn't sleep any more, but the read side does.
>
> See amdgpu_mn_invalidate_node() and that is where you actually need to
> handle the non-blocking flag correctly.
Ohh, right you are. We already handle that by bailing out before calling
amdgpu_mn_invalidate_node in !blockable mode. So does this looks good to
you?
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..48fa152231be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -180,11 +180,15 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
- return -EAGAIN;
-
+ /*
+* We can take sleepable lock even on !blockable mode because
+* read_lock is only ever take from this path and the notifier
+* lock never really sleeps. In fact the only reason why the
+* later is sleepable is because the notifier itself might sleep
+* in amdgpu_mn_invalidate_node but blockable mode is handled
+* before calling into that path.
+*/
+ mutex_lock(>read_lock);
if (atomic_inc_return(>recursion) == 1)
down_read_non_owner(>lock);
mutex_unlock(>read_lock);
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for mm, oom: distinguish blockable mode for mmu notifiers (rev11)
== Series Details == Series: mm, oom: distinguish blockable mode for mmu notifiers (rev11) URL : https://patchwork.freedesktop.org/series/45263/ State : failure == Summary == Applying: mm, oom: distinguish blockable mode for mmu notifiers error: sha1 information is lacking or useless (include/linux/mmu_notifier.h). error: could not build fake ancestor Patch failed at 0001 mm, oom: distinguish blockable mode for mmu notifiers Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 22:02:23, Tetsuo Handa wrote:
> On 2018/08/24 20:36, Michal Hocko wrote:
> >> That is, this API seems to be currently used by only out-of-tree users.
> >> Since
> >> we can't check that nobody has memory allocation dependency, I think that
> >> hmm_invalidate_range_start() should return -EAGAIN if blockable == false
> >> for now.
> >
> > The code expects that the invalidate_range_end doesn't block if
> > invalidate_range_start hasn't blocked. That is the reason why the end
> > callback doesn't have blockable parameter. If this doesn't hold then the
> > whole scheme is just fragile because those two calls should pair.
> >
> That is
>
> More worrisome part in that patch is that I don't know whether using
> trylock if blockable == false at entry is really sufficient.
>
> . Since those two calls should pair, I think that we need to determine whether
> we need to return -EAGAIN at start call by evaluating both calls.
Yes, and I believe I have done that audit. Module my misunderstanding of
the code.
> Like mn_invl_range_start() involves schedule_delayed_work() which could be
> blocked on memory allocation under OOM situation,
It doesn't because that code path is not invoked for the !blockable
case.
> I worry that (currently
> out-of-tree) users of this API are involving work / recursion.
I do not give a slightest about out-of-tree modules. They will have to
accomodate to the new API. I have no problems to extend the
documentation and be explicit about this expectation.
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 133ba78820ee..698e371aafe3 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -153,7 +153,9 @@ struct mmu_notifier_ops {
*
* If blockable argument is set to false then the callback cannot
* sleep and has to return with -EAGAIN. 0 should be returned
-* otherwise.
+* otherwise. Please note that if invalidate_range_start approves
+* a non-blocking behavior then the same applies to
+* invalidate_range_end.
*
*/
int (*invalidate_range_start)(struct mmu_notifier *mn,
> And hmm_release() says that
>
> /*
>* Drop mirrors_sem so callback can wait on any pending
>* work that might itself trigger mmu_notifier callback
>* and thus would deadlock with us.
>*/
>
> and keeps "all operations protected by hmm->mirrors_sem held for write are
> atomic". This suggests that "some operations protected by hmm->mirrors_sem
> held
> for read will sleep (and in the worst case involves memory allocation
> dependency)".
Yes and so what? The clear expectation is that neither of the range
notifiers do not sleep in !blocking mode. I really fail to see what you
are trying to say.
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 15:24 schrieb Michal Hocko: On Fri 24-08-18 15:10:08, Christian König wrote: Am 24.08.2018 um 15:01 schrieb Michal Hocko: On Fri 24-08-18 14:52:26, Christian König wrote: Am 24.08.2018 um 14:33 schrieb Michal Hocko: [...] Thiking about it some more, I can imagine that a notifier callback which performs an allocation might trigger a memory reclaim and that in turn might trigger a notifier to be invoked and recurse. But notifier shouldn't really allocate memory. They are called from deep MM code paths and this would be extremely deadlock prone. Maybe Jerome can come up some more realistic scenario. If not then I would propose to simplify the locking here. We have lockdep to catch self deadlocks and it is always better to handle a specific issue rather than having a code without a clear indication how it can recurse. Well I agree that we should probably fix that, but I have some concerns to remove the existing workaround. See we added that to get rid of a real problem in a customer environment and I don't want to that to show up again. It would really help to know more about that case and fix it properly rather than workaround it like this. Anyway, let me think how to handle the non-blocking notifier invocation then. I was not able to come up with anything remotely sane yet. With avoiding allocating memory in the write lock path I don't see an issue any more with that. All what the write lock path does now is adding items to a linked lists, arrays etc Can we change it to non-sleepable lock then? No, the write side doesn't sleep any more, but the read side does. See amdgpu_mn_invalidate_node() and that is where you actually need to handle the non-blocking flag correctly. Christian. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 15:10:08, Christian König wrote: > Am 24.08.2018 um 15:01 schrieb Michal Hocko: > > On Fri 24-08-18 14:52:26, Christian König wrote: > > > Am 24.08.2018 um 14:33 schrieb Michal Hocko: > > [...] > > > > Thiking about it some more, I can imagine that a notifier callback which > > > > performs an allocation might trigger a memory reclaim and that in turn > > > > might trigger a notifier to be invoked and recurse. But notifier > > > > shouldn't really allocate memory. They are called from deep MM code > > > > paths and this would be extremely deadlock prone. Maybe Jerome can come > > > > up some more realistic scenario. If not then I would propose to simplify > > > > the locking here. We have lockdep to catch self deadlocks and it is > > > > always better to handle a specific issue rather than having a code > > > > without a clear indication how it can recurse. > > > Well I agree that we should probably fix that, but I have some concerns to > > > remove the existing workaround. > > > > > > See we added that to get rid of a real problem in a customer environment > > > and > > > I don't want to that to show up again. > > It would really help to know more about that case and fix it properly > > rather than workaround it like this. Anyway, let me think how to handle > > the non-blocking notifier invocation then. I was not able to come up > > with anything remotely sane yet. > > With avoiding allocating memory in the write lock path I don't see an issue > any more with that. > > All what the write lock path does now is adding items to a linked lists, > arrays etc Can we change it to non-sleepable lock then? -- Michal Hocko SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 15:01 schrieb Michal Hocko: On Fri 24-08-18 14:52:26, Christian König wrote: Am 24.08.2018 um 14:33 schrieb Michal Hocko: [...] Thiking about it some more, I can imagine that a notifier callback which performs an allocation might trigger a memory reclaim and that in turn might trigger a notifier to be invoked and recurse. But notifier shouldn't really allocate memory. They are called from deep MM code paths and this would be extremely deadlock prone. Maybe Jerome can come up some more realistic scenario. If not then I would propose to simplify the locking here. We have lockdep to catch self deadlocks and it is always better to handle a specific issue rather than having a code without a clear indication how it can recurse. Well I agree that we should probably fix that, but I have some concerns to remove the existing workaround. See we added that to get rid of a real problem in a customer environment and I don't want to that to show up again. It would really help to know more about that case and fix it properly rather than workaround it like this. Anyway, let me think how to handle the non-blocking notifier invocation then. I was not able to come up with anything remotely sane yet. With avoiding allocating memory in the write lock path I don't see an issue any more with that. All what the write lock path does now is adding items to a linked lists, arrays etc So there is no more blocking involved here and the read lock side should be able to grab the lock immediately. Christian. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On 2018/08/24 20:36, Michal Hocko wrote: >> That is, this API seems to be currently used by only out-of-tree users. Since >> we can't check that nobody has memory allocation dependency, I think that >> hmm_invalidate_range_start() should return -EAGAIN if blockable == false for >> now. > > The code expects that the invalidate_range_end doesn't block if > invalidate_range_start hasn't blocked. That is the reason why the end > callback doesn't have blockable parameter. If this doesn't hold then the > whole scheme is just fragile because those two calls should pair. > That is More worrisome part in that patch is that I don't know whether using trylock if blockable == false at entry is really sufficient. . Since those two calls should pair, I think that we need to determine whether we need to return -EAGAIN at start call by evaluating both calls. Like mn_invl_range_start() involves schedule_delayed_work() which could be blocked on memory allocation under OOM situation, I worry that (currently out-of-tree) users of this API are involving work / recursion. And hmm_release() says that /* * Drop mirrors_sem so callback can wait on any pending * work that might itself trigger mmu_notifier callback * and thus would deadlock with us. */ and keeps "all operations protected by hmm->mirrors_sem held for write are atomic". This suggests that "some operations protected by hmm->mirrors_sem held for read will sleep (and in the worst case involves memory allocation dependency)". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 14:52:26, Christian König wrote: > Am 24.08.2018 um 14:33 schrieb Michal Hocko: [...] > > Thiking about it some more, I can imagine that a notifier callback which > > performs an allocation might trigger a memory reclaim and that in turn > > might trigger a notifier to be invoked and recurse. But notifier > > shouldn't really allocate memory. They are called from deep MM code > > paths and this would be extremely deadlock prone. Maybe Jerome can come > > up some more realistic scenario. If not then I would propose to simplify > > the locking here. We have lockdep to catch self deadlocks and it is > > always better to handle a specific issue rather than having a code > > without a clear indication how it can recurse. > > Well I agree that we should probably fix that, but I have some concerns to > remove the existing workaround. > > See we added that to get rid of a real problem in a customer environment and > I don't want to that to show up again. It would really help to know more about that case and fix it properly rather than workaround it like this. Anyway, let me think how to handle the non-blocking notifier invocation then. I was not able to come up with anything remotely sane yet. -- Michal Hocko SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 14:33 schrieb Michal Hocko: On Fri 24-08-18 14:18:44, Christian König wrote: Am 24.08.2018 um 14:03 schrieb Michal Hocko: On Fri 24-08-18 13:57:52, Christian König wrote: Am 24.08.2018 um 13:52 schrieb Michal Hocko: On Fri 24-08-18 13:43:16, Christian König wrote: [...] That won't work like this there might be multiple invalidate_range_start()/invalidate_range_end() pairs open at the same time. E.g. the lock might be taken recursively and that is illegal for a rw_semaphore. I am not sure I follow. Are you saying that one invalidate_range might trigger another one from the same path? No, but what can happen is: invalidate_range_start(A,B); invalidate_range_start(C,D); ... invalidate_range_end(C,D); invalidate_range_end(A,B); Grabbing the read lock twice would be illegal in this case. I am sorry but I still do not follow. What is the context the two are called from? I don't have the slightest idea. Can you give me an example. I simply do not see it in the code, mostly because I am not familiar with it. I'm neither. We stumbled over that by pure observation and after discussing the problem with Jerome came up with this solution. No idea where exactly that case comes from, but I can confirm that it indeed happens. Thiking about it some more, I can imagine that a notifier callback which performs an allocation might trigger a memory reclaim and that in turn might trigger a notifier to be invoked and recurse. But notifier shouldn't really allocate memory. They are called from deep MM code paths and this would be extremely deadlock prone. Maybe Jerome can come up some more realistic scenario. If not then I would propose to simplify the locking here. We have lockdep to catch self deadlocks and it is always better to handle a specific issue rather than having a code without a clear indication how it can recurse. Well I agree that we should probably fix that, but I have some concerns to remove the existing workaround. See we added that to get rid of a real problem in a customer environment and I don't want to that to show up again. In the meantime I've send out a fix to avoid allocating memory while holding the mn_lock. Thanks for pointing that out, Christian. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for Decode memdev info and bandwidth and implemnt latency WA (rev3)
== Series Details == Series: Decode memdev info and bandwidth and implemnt latency WA (rev3) URL : https://patchwork.freedesktop.org/series/46481/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4703_full -> Patchwork_10006_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10006_full that come from known issues: === IGT changes === Issues hit igt@kms_cursor_legacy@cursor-vs-flip-toggle: shard-hsw: PASS -> FAIL (fdo#103355) Possible fixes igt@gem_ctx_isolation@bcs0-s3: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@gem_exec_big: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 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_4703 -> Patchwork_10006 CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10006: 6e6423a93963b98c9198b87d2c1aee78a0c98c73 @ 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_10006/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 14:18:44, Christian König wrote: > Am 24.08.2018 um 14:03 schrieb Michal Hocko: > > On Fri 24-08-18 13:57:52, Christian König wrote: > > > Am 24.08.2018 um 13:52 schrieb Michal Hocko: > > > > On Fri 24-08-18 13:43:16, Christian König wrote: > > [...] > > > > > That won't work like this there might be multiple > > > > > invalidate_range_start()/invalidate_range_end() pairs open at the > > > > > same time. > > > > > E.g. the lock might be taken recursively and that is illegal for a > > > > > rw_semaphore. > > > > I am not sure I follow. Are you saying that one invalidate_range might > > > > trigger another one from the same path? > > > No, but what can happen is: > > > > > > invalidate_range_start(A,B); > > > invalidate_range_start(C,D); > > > ... > > > invalidate_range_end(C,D); > > > invalidate_range_end(A,B); > > > > > > Grabbing the read lock twice would be illegal in this case. > > I am sorry but I still do not follow. What is the context the two are > > called from? > > I don't have the slightest idea. > > > Can you give me an example. I simply do not see it in the > > code, mostly because I am not familiar with it. > > I'm neither. > > We stumbled over that by pure observation and after discussing the problem > with Jerome came up with this solution. > > No idea where exactly that case comes from, but I can confirm that it indeed > happens. Thiking about it some more, I can imagine that a notifier callback which performs an allocation might trigger a memory reclaim and that in turn might trigger a notifier to be invoked and recurse. But notifier shouldn't really allocate memory. They are called from deep MM code paths and this would be extremely deadlock prone. Maybe Jerome can come up some more realistic scenario. If not then I would propose to simplify the locking here. We have lockdep to catch self deadlocks and it is always better to handle a specific issue rather than having a code without a clear indication how it can recurse. -- Michal Hocko SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 14:03 schrieb Michal Hocko: On Fri 24-08-18 13:57:52, Christian König wrote: Am 24.08.2018 um 13:52 schrieb Michal Hocko: On Fri 24-08-18 13:43:16, Christian König wrote: [...] That won't work like this there might be multiple invalidate_range_start()/invalidate_range_end() pairs open at the same time. E.g. the lock might be taken recursively and that is illegal for a rw_semaphore. I am not sure I follow. Are you saying that one invalidate_range might trigger another one from the same path? No, but what can happen is: invalidate_range_start(A,B); invalidate_range_start(C,D); ... invalidate_range_end(C,D); invalidate_range_end(A,B); Grabbing the read lock twice would be illegal in this case. I am sorry but I still do not follow. What is the context the two are called from? I don't have the slightest idea. Can you give me an example. I simply do not see it in the code, mostly because I am not familiar with it. I'm neither. We stumbled over that by pure observation and after discussing the problem with Jerome came up with this solution. No idea where exactly that case comes from, but I can confirm that it indeed happens. Regards, Christian. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Two more worries for this patch.
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> @@ -178,12 +178,18 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
> *
> * @amn: our notifier
> */
> -static void amdgpu_mn_read_lock(struct amdgpu_mn *amn)
> +static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
> {
> - mutex_lock(>read_lock);
> + if (blockable)
> + mutex_lock(>read_lock);
> + else if (!mutex_trylock(>read_lock))
> + return -EAGAIN;
> +
> if (atomic_inc_return(>recursion) == 1)
> down_read_non_owner(>lock);
Why don't we need to use trylock here if blockable == false ?
Want comment why it is safe to use blocking lock here.
> mutex_unlock(>read_lock);
> +
> + return 0;
> }
>
> /**
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -177,16 +177,19 @@ static void hmm_release(struct mmu_notifier *mn, struct
> mm_struct *mm)
> up_write(>mirrors_sem);
> }
>
> -static void hmm_invalidate_range_start(struct mmu_notifier *mn,
> +static int hmm_invalidate_range_start(struct mmu_notifier *mn,
>struct mm_struct *mm,
>unsigned long start,
> - unsigned long end)
> + unsigned long end,
> + bool blockable)
> {
> struct hmm *hmm = mm->hmm;
>
> VM_BUG_ON(!hmm);
>
> atomic_inc(>sequence);
> +
> + return 0;
> }
>
> static void hmm_invalidate_range_end(struct mmu_notifier *mn,
This assumes that hmm_invalidate_range_end() does not have memory
allocation dependency. But hmm_invalidate_range() from
hmm_invalidate_range_end() involves
down_read(>mirrors_sem);
list_for_each_entry(mirror, >mirrors, list)
mirror->ops->sync_cpu_device_pagetables(mirror, action,
start, end);
up_read(>mirrors_sem);
sequence. What is surprising is that there is no in-tree user who assigns
sync_cpu_device_pagetables field.
$ grep -Fr sync_cpu_device_pagetables *
Documentation/vm/hmm.rst: /* sync_cpu_device_pagetables() - synchronize
page tables
include/linux/hmm.h: * will get callbacks through
sync_cpu_device_pagetables() operation (see
include/linux/hmm.h:/* sync_cpu_device_pagetables() - synchronize page
tables
include/linux/hmm.h:void (*sync_cpu_device_pagetables)(struct hmm_mirror
*mirror,
include/linux/hmm.h: * hmm_mirror_ops.sync_cpu_device_pagetables() callback,
so that CPU page
mm/hmm.c: mirror->ops->sync_cpu_device_pagetables(mirror,
action,
That is, this API seems to be currently used by only out-of-tree users. Since
we can't check that nobody has memory allocation dependency, I think that
hmm_invalidate_range_start() should return -EAGAIN if blockable == false for
now.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/18] drm/i915: Extract per-platform plane->check() functions
On Fri, Aug 24, 2018 at 01:01:36AM +, Souza, Jose wrote:
> On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Split up intel_check_primary_plane() and intel_check_sprite_plane()
> > into per-platform variants. This way we can get a unified behaviour
> > between the SKL universal planes, and we stop checking for non-SKL
> > specific scaling limits for the "sprite" planes. And we now get
> > a natural place where to add more plarform specific checks.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +-
> > drivers/gpu/drm/i915/intel_display.c | 123 +---
> > drivers/gpu/drm/i915/intel_drv.h | 13 +-
> > drivers/gpu/drm/i915/intel_sprite.c | 303 +++-
> > --
> > 4 files changed, 249 insertions(+), 192 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index dcba645cabb8..eddcdd6e4b3b 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -159,7 +159,7 @@ int intel_plane_atomic_check_with_state(const
> > struct intel_crtc_state *old_crtc_
> > }
> >
> > intel_state->base.visible = false;
> > - ret = intel_plane->check_plane(intel_plane, crtc_state,
> > intel_state);
> > + ret = intel_plane->check_plane(crtc_state, intel_state);
> > if (ret)
> > return ret;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index fdc5bedc5135..9b9eaeda15dd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3350,6 +3350,36 @@ int i9xx_check_plane_surface(struct
> > intel_plane_state *plane_state)
> > return 0;
> > }
> >
> > +static int
> > +i9xx_plane_check(struct intel_crtc_state *crtc_state,
> > +struct intel_plane_state *plane_state)
> > +{
> > + int ret;
> > +
> > + ret = drm_atomic_helper_check_plane_state(_state->base,
> > + _state->base,
> > + DRM_PLANE_HELPER_NO_S
> > CALING,
> > + DRM_PLANE_HELPER_NO_S
> > CALING,
> > + false, true);
> > + if (ret)
> > + return ret;
> > +
> > + if (!plane_state->base.visible)
> > + return 0;
> > +
> > + ret = intel_plane_check_src_coordinates(plane_state);
> > + if (ret)
> > + return ret;
> > +
> > + ret = i9xx_check_plane_surface(plane_state);
> > + if (ret)
> > + return ret;
> > +
> > + plane_state->ctl = i9xx_plane_ctl(crtc_state, plane_state);
> > +
> > + return 0;
> > +}
> > +
> > static void i9xx_update_plane(struct intel_plane *plane,
> > const struct intel_crtc_state
> > *crtc_state,
> > const struct intel_plane_state
> > *plane_state)
> > @@ -9680,6 +9710,11 @@ static int intel_check_cursor(struct
> > intel_crtc_state *crtc_state,
> > u32 offset;
> > int ret;
> >
> > + if (fb && fb->modifier != DRM_FORMAT_MOD_LINEAR) {
> > + DRM_DEBUG_KMS("cursor cannot be tiled\n");
> > + return -EINVAL;
> > + }
> > +
> > ret = drm_atomic_helper_check_plane_state(_state->base,
> > _state->base,
> > DRM_PLANE_HELPER_NO_S
> > CALING,
> > @@ -9688,13 +9723,12 @@ static int intel_check_cursor(struct
> > intel_crtc_state *crtc_state,
> > if (ret)
> > return ret;
> >
> > - if (!fb)
> > + if (!plane_state->base.visible)
> > return 0;
> >
> > - if (fb->modifier != DRM_FORMAT_MOD_LINEAR) {
> > - DRM_DEBUG_KMS("cursor cannot be tiled\n");
> > - return -EINVAL;
> > - }
> > + ret = intel_plane_check_src_coordinates(plane_state);
> > + if (ret)
> > + return ret;
> >
> > intel_fill_fb_ggtt_view(_state->view, fb, rotation);
> > plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> > rotation);
> > @@ -9744,8 +9778,7 @@ static bool i845_cursor_size_ok(const struct
> > intel_plane_state *plane_state)
> > return intel_cursor_size_ok(plane_state) && IS_ALIGNED(width,
> > 64);
> > }
> >
> > -static int i845_check_cursor(struct intel_plane *plane,
> > -struct intel_crtc_state *crtc_state,
> > +static int i845_check_cursor(struct intel_crtc_state *crtc_state,
> > struct intel_plane_state *plane_state)
>
> You are dropping struct intel_plane *plane from check_plane and in
> skl_max_scale(), I think it is a good thing to do but would be better
> do it in a previous patch and leave this one just extracting
> check_plane().
Yeah, I can yank that out easily enough.
>
> > {
> > const struct drm_framebuffer *fb =
Re: [Intel-gfx] [igt-dev] RFC: Migration to Gitlab
On Fri, Aug 24, 2018 at 8:52 AM, Jani Nikula wrote: > On Wed, 22 Aug 2018, Rodrigo Vivi wrote: >> On Wed, Aug 22, 2018 at 10:19:19AM -0400, Adam Jackson wrote: >>> On Wed, 2018-08-22 at 16:13 +0300, Jani Nikula wrote: >>> >>> > - Sticking to fdo bugzilla and disabling gitlab issues for at least >>> > drm-intel for the time being. Doing that migration in the same go is a >>> > bit much I think. Reassignment across bugzilla and gitlab will be an >>> > issue. >>> >>> Can you elaborate a bit on the issues here? The actual move-the-bugs >>> process has been pretty painless for the parts of xorg we've done so >>> far. >> >> I guess there is nothing against moving the bugs there. The concern is only >> on >> doing everything at once. > > No, it's not just that. > > We have some automation using the bugzilla APIs directly, and > someone(tm) needs to figure out how this should work with gitlab. Maybe > we have a better chance of doing things with gitlab's APIs, maybe we can > reduce our dependence on external logic altogether. > > We have special "i915 platform" and "i915 features" fields in > bugzilla. We use a mailing list default assignee. Some of us use the > "whiteboard" and "keywords" fields. Etc. > > I don't think figuring all this out is rocket science, but someone needs > to actually do it, and get our workflows straightened out *before* we > flip the switch. I'm just trying to figure out if that is a blocker to > migrating the repos. I think the mesa approach of flat-out disabling gitlab issues and merge request is solid. That way there's no confusions with contributions and bug reports stranded in the wrong place. This should allow us to handle all the different feature sets gitlab provides independently and as we see fit: Repo hosting, documentation/artifact hosting, CI integration, patch submissions, issue tracking. We can choose what we think is good, and ignore everything else. And of course if 1-2 years down the road things change, we can reevaluate. I think for a rough timeline if things go well we'll have igt and maintainer-tools migrated by end of year (just the repos, nothing else), and a solid plan for migrating all the drm* repos next year. Everything else is too far away to have solid plans for, or even just a timeline. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 13:57:52, Christian König wrote: > Am 24.08.2018 um 13:52 schrieb Michal Hocko: > > On Fri 24-08-18 13:43:16, Christian König wrote: [...] > > > That won't work like this there might be multiple > > > invalidate_range_start()/invalidate_range_end() pairs open at the same > > > time. > > > E.g. the lock might be taken recursively and that is illegal for a > > > rw_semaphore. > > I am not sure I follow. Are you saying that one invalidate_range might > > trigger another one from the same path? > > No, but what can happen is: > > invalidate_range_start(A,B); > invalidate_range_start(C,D); > ... > invalidate_range_end(C,D); > invalidate_range_end(A,B); > > Grabbing the read lock twice would be illegal in this case. I am sorry but I still do not follow. What is the context the two are called from? Can you give me an example. I simply do not see it in the code, mostly because I am not familiar with it. -- Michal Hocko SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 13:52 schrieb Michal Hocko:
On Fri 24-08-18 13:43:16, Christian König wrote:
Am 24.08.2018 um 13:32 schrieb Michal Hocko:
On Fri 24-08-18 19:54:19, Tetsuo Handa wrote:
Two more worries for this patch.
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -178,12 +178,18 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*
* @amn: our notifier
*/
-static void amdgpu_mn_read_lock(struct amdgpu_mn *amn)
+static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- mutex_lock(>read_lock);
+ if (blockable)
+ mutex_lock(>read_lock);
+ else if (!mutex_trylock(>read_lock))
+ return -EAGAIN;
+
if (atomic_inc_return(>recursion) == 1)
down_read_non_owner(>lock);
Why don't we need to use trylock here if blockable == false ?
Want comment why it is safe to use blocking lock here.
Hmm, I am pretty sure I have checked the code but it was quite confusing
so I might have missed something. Double checking now, it seems that
this read_lock is not used anywhere else and it is not _the_ lock we are
interested about. It is the amn->lock (amdgpu_mn_lock) which matters as
it is taken in exclusive mode for expensive operations.
The write side of the lock is only taken in the command submission IOCTL.
So you actually don't need to change anything here (even the proposed
changes are overkill) since we can't tear down the struct_mm while an IOCTL
is still using.
I am not so sure. We are not in the mm destruction phase yet. This is
mostly about the oom context which might fire right during the IOCTL. If
any of the path which is holding the write lock blocks for unbound
amount of time or even worse allocates a memory then we are screwed. So
we need to back of when blockable = false.
Oh, yeah good point. Haven't thought about that possibility.
Is that correct Christian? If this is correct then we need to update the
locking here. I am struggling to grasp the ref counting part. Why cannot
all readers simply take the lock rather than rely on somebody else to
take it? 1ed3d2567c800 didn't really help me to understand the locking
scheme here so any help would be appreciated.
That won't work like this there might be multiple
invalidate_range_start()/invalidate_range_end() pairs open at the same time.
E.g. the lock might be taken recursively and that is illegal for a
rw_semaphore.
I am not sure I follow. Are you saying that one invalidate_range might
trigger another one from the same path?
No, but what can happen is:
invalidate_range_start(A,B);
invalidate_range_start(C,D);
...
invalidate_range_end(C,D);
invalidate_range_end(A,B);
Grabbing the read lock twice would be illegal in this case.
Regards,
Christian.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for Decode memdev info and bandwidth and implemnt latency WA (rev3)
== Series Details == Series: Decode memdev info and bandwidth and implemnt latency WA (rev3) URL : https://patchwork.freedesktop.org/series/46481/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4703_full -> Patchwork_10005_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10005_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10005_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_10005_full: === IGT changes === Possible regressions igt@gem_eio@reset-stress: shard-kbl: PASS -> FAIL Warnings igt@kms_chv_cursor_fail@pipe-a-256x256-right-edge: shard-snb: PASS -> SKIP +1 == Known issues == Here are the changes found in Patchwork_10005_full that come from known issues: === IGT changes === Issues hit igt@kms_draw_crc@draw-method-rgb565-mmap-wc-xtiled: shard-glk: PASS -> FAIL (fdo#103184) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) igt@kms_vblank@pipe-b-ts-continuation-suspend: shard-hsw: PASS -> FAIL (fdo#104894) Possible fixes igt@gem_exec_big: shard-hsw: INCOMPLETE (fdo#103540) -> PASS fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894 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_4703 -> Patchwork_10005 CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10005: a2a83254992b4983e9c32c3e622049a0db88f50a @ 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_10005/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 13:43:16, Christian König wrote:
> Am 24.08.2018 um 13:32 schrieb Michal Hocko:
> > On Fri 24-08-18 19:54:19, Tetsuo Handa wrote:
> > > Two more worries for this patch.
> > >
> > >
> > >
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > > > @@ -178,12 +178,18 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
> > > >*
> > > >* @amn: our notifier
> > > >*/
> > > > -static void amdgpu_mn_read_lock(struct amdgpu_mn *amn)
> > > > +static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
> > > > {
> > > > - mutex_lock(>read_lock);
> > > > + if (blockable)
> > > > + mutex_lock(>read_lock);
> > > > + else if (!mutex_trylock(>read_lock))
> > > > + return -EAGAIN;
> > > > +
> > > > if (atomic_inc_return(>recursion) == 1)
> > > > down_read_non_owner(>lock);
> > > Why don't we need to use trylock here if blockable == false ?
> > > Want comment why it is safe to use blocking lock here.
> > Hmm, I am pretty sure I have checked the code but it was quite confusing
> > so I might have missed something. Double checking now, it seems that
> > this read_lock is not used anywhere else and it is not _the_ lock we are
> > interested about. It is the amn->lock (amdgpu_mn_lock) which matters as
> > it is taken in exclusive mode for expensive operations.
>
> The write side of the lock is only taken in the command submission IOCTL.
>
> So you actually don't need to change anything here (even the proposed
> changes are overkill) since we can't tear down the struct_mm while an IOCTL
> is still using.
I am not so sure. We are not in the mm destruction phase yet. This is
mostly about the oom context which might fire right during the IOCTL. If
any of the path which is holding the write lock blocks for unbound
amount of time or even worse allocates a memory then we are screwed. So
we need to back of when blockable = false.
> > Is that correct Christian? If this is correct then we need to update the
> > locking here. I am struggling to grasp the ref counting part. Why cannot
> > all readers simply take the lock rather than rely on somebody else to
> > take it? 1ed3d2567c800 didn't really help me to understand the locking
> > scheme here so any help would be appreciated.
>
> That won't work like this there might be multiple
> invalidate_range_start()/invalidate_range_end() pairs open at the same time.
> E.g. the lock might be taken recursively and that is illegal for a
> rw_semaphore.
I am not sure I follow. Are you saying that one invalidate_range might
trigger another one from the same path?
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for mm, oom: distinguish blockable mode for mmu notifiers (rev10)
== Series Details == Series: mm, oom: distinguish blockable mode for mmu notifiers (rev10) URL : https://patchwork.freedesktop.org/series/45263/ State : failure == Summary == Applying: mm, oom: distinguish blockable mode for mmu notifiers error: sha1 information is lacking or useless (drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c). error: could not build fake ancestor Patch failed at 0001 mm, oom: distinguish blockable mode for mmu notifiers Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 24.08.2018 um 13:32 schrieb Michal Hocko:
On Fri 24-08-18 19:54:19, Tetsuo Handa wrote:
Two more worries for this patch.
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -178,12 +178,18 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*
* @amn: our notifier
*/
-static void amdgpu_mn_read_lock(struct amdgpu_mn *amn)
+static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- mutex_lock(>read_lock);
+ if (blockable)
+ mutex_lock(>read_lock);
+ else if (!mutex_trylock(>read_lock))
+ return -EAGAIN;
+
if (atomic_inc_return(>recursion) == 1)
down_read_non_owner(>lock);
Why don't we need to use trylock here if blockable == false ?
Want comment why it is safe to use blocking lock here.
Hmm, I am pretty sure I have checked the code but it was quite confusing
so I might have missed something. Double checking now, it seems that
this read_lock is not used anywhere else and it is not _the_ lock we are
interested about. It is the amn->lock (amdgpu_mn_lock) which matters as
it is taken in exclusive mode for expensive operations.
The write side of the lock is only taken in the command submission IOCTL.
So you actually don't need to change anything here (even the proposed
changes are overkill) since we can't tear down the struct_mm while an
IOCTL is still using.
Is that correct Christian? If this is correct then we need to update the
locking here. I am struggling to grasp the ref counting part. Why cannot
all readers simply take the lock rather than rely on somebody else to
take it? 1ed3d2567c800 didn't really help me to understand the locking
scheme here so any help would be appreciated.
That won't work like this there might be multiple
invalidate_range_start()/invalidate_range_end() pairs open at the same
time. E.g. the lock might be taken recursively and that is illegal for a
rw_semaphore.
Regards,
Christian.
I am wondering why we cannot do
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..93034178673d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -180,14 +180,11 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
- return -EAGAIN;
-
- if (atomic_inc_return(>recursion) == 1)
- down_read_non_owner(>lock);
- mutex_unlock(>read_lock);
+ if (!down_read_trylock(>lock)) {
+ if (!blockable)
+ return -EAGAIN;
+ down_read(amn->lock);
+ }
return 0;
}
@@ -199,8 +196,7 @@ static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool
blockable)
*/
static void amdgpu_mn_read_unlock(struct amdgpu_mn *amn)
{
- if (atomic_dec_return(>recursion) == 0)
- up_read_non_owner(>lock);
+ up_read(>lock);
}
/**
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 19:54:19, Tetsuo Handa wrote:
[...]
> > --- a/mm/hmm.c
> > +++ b/mm/hmm.c
> > @@ -177,16 +177,19 @@ static void hmm_release(struct mmu_notifier *mn,
> > struct mm_struct *mm)
> > up_write(>mirrors_sem);
> > }
> >
> > -static void hmm_invalidate_range_start(struct mmu_notifier *mn,
> > +static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> >struct mm_struct *mm,
> >unsigned long start,
> > - unsigned long end)
> > + unsigned long end,
> > + bool blockable)
> > {
> > struct hmm *hmm = mm->hmm;
> >
> > VM_BUG_ON(!hmm);
> >
> > atomic_inc(>sequence);
> > +
> > + return 0;
> > }
> >
> > static void hmm_invalidate_range_end(struct mmu_notifier *mn,
>
> This assumes that hmm_invalidate_range_end() does not have memory
> allocation dependency. But hmm_invalidate_range() from
> hmm_invalidate_range_end() involves
>
> down_read(>mirrors_sem);
> list_for_each_entry(mirror, >mirrors, list)
> mirror->ops->sync_cpu_device_pagetables(mirror, action,
> start, end);
> up_read(>mirrors_sem);
>
> sequence. What is surprising is that there is no in-tree user who assigns
> sync_cpu_device_pagetables field.
Yes HMM doesn't have any in tree user yet.
> $ grep -Fr sync_cpu_device_pagetables *
> Documentation/vm/hmm.rst: /* sync_cpu_device_pagetables() - synchronize
> page tables
> include/linux/hmm.h: * will get callbacks through
> sync_cpu_device_pagetables() operation (see
> include/linux/hmm.h:/* sync_cpu_device_pagetables() - synchronize page
> tables
> include/linux/hmm.h:void (*sync_cpu_device_pagetables)(struct
> hmm_mirror *mirror,
> include/linux/hmm.h: * hmm_mirror_ops.sync_cpu_device_pagetables()
> callback, so that CPU page
> mm/hmm.c: mirror->ops->sync_cpu_device_pagetables(mirror,
> action,
>
> That is, this API seems to be currently used by only out-of-tree users. Since
> we can't check that nobody has memory allocation dependency, I think that
> hmm_invalidate_range_start() should return -EAGAIN if blockable == false for
> now.
The code expects that the invalidate_range_end doesn't block if
invalidate_range_start hasn't blocked. That is the reason why the end
callback doesn't have blockable parameter. If this doesn't hold then the
whole scheme is just fragile because those two calls should pair.
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 24-08-18 19:54:19, Tetsuo Handa wrote:
> Two more worries for this patch.
>
>
>
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > @@ -178,12 +178,18 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
> > *
> > * @amn: our notifier
> > */
> > -static void amdgpu_mn_read_lock(struct amdgpu_mn *amn)
> > +static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
> > {
> > - mutex_lock(>read_lock);
> > + if (blockable)
> > + mutex_lock(>read_lock);
> > + else if (!mutex_trylock(>read_lock))
> > + return -EAGAIN;
> > +
> > if (atomic_inc_return(>recursion) == 1)
> > down_read_non_owner(>lock);
>
> Why don't we need to use trylock here if blockable == false ?
> Want comment why it is safe to use blocking lock here.
Hmm, I am pretty sure I have checked the code but it was quite confusing
so I might have missed something. Double checking now, it seems that
this read_lock is not used anywhere else and it is not _the_ lock we are
interested about. It is the amn->lock (amdgpu_mn_lock) which matters as
it is taken in exclusive mode for expensive operations.
Is that correct Christian? If this is correct then we need to update the
locking here. I am struggling to grasp the ref counting part. Why cannot
all readers simply take the lock rather than rely on somebody else to
take it? 1ed3d2567c800 didn't really help me to understand the locking
scheme here so any help would be appreciated.
I am wondering why we cannot do
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..93034178673d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -180,14 +180,11 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
- return -EAGAIN;
-
- if (atomic_inc_return(>recursion) == 1)
- down_read_non_owner(>lock);
- mutex_unlock(>read_lock);
+ if (!down_read_trylock(>lock)) {
+ if (!blockable)
+ return -EAGAIN;
+ down_read(amn->lock);
+ }
return 0;
}
@@ -199,8 +196,7 @@ static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool
blockable)
*/
static void amdgpu_mn_read_unlock(struct amdgpu_mn *amn)
{
- if (atomic_dec_return(>recursion) == 0)
- up_read_non_owner(>lock);
+ up_read(>lock);
}
/**
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Decode memdev info and bandwidth and implemnt latency WA (rev3)
== Series Details ==
Series: Decode memdev info and bandwidth and implemnt latency WA (rev3)
URL : https://patchwork.freedesktop.org/series/46481/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4703 -> Patchwork_10006 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46481/revisions/3/mbox/
== Known issues ==
Here are the changes found in Patchwork_10006 that come from known issues:
=== IGT changes ===
Issues hit
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362, fdo#103191)
{igt@kms_psr@primary_page_flip}:
{fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3
{igt@pm_rpm@module-reload}:
fi-cnl-psr: PASS -> WARN (fdo#107602)
Possible fixes
igt@drv_selftest@live_hangcheck:
{fi-bdw-samus}: DMESG-FAIL (fdo#106560) -> PASS
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@gem_sync@basic-many-each:
{fi-byt-clapper}: FAIL -> 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#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
== Participating hosts (53 -> 49) ==
Additional (1): fi-icl-u
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4703 -> Patchwork_10006
CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10006: 6e6423a93963b98c9198b87d2c1aee78a0c98c73 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
6e6423a93963 drm/i915/kbl+: Enable IPC only for symmetric memory configurations
ad084286084b drm/i915/skl+: don't trust IPC value set by BIOS
00825915e644 drm/i915: Implement 16GB dimm wa for latency level-0
85f1a226cd8a drm/i915/skl+: Decode memory bandwidth and parameters
6890cf014d0e drm/i915/bxt: Decode memory bandwidth and parameters
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10006/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Decode memdev info and bandwidth and implemnt latency WA (rev3)
== Series Details ==
Series: Decode memdev info and bandwidth and implemnt latency WA (rev3)
URL : https://patchwork.freedesktop.org/series/46481/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4703 -> Patchwork_10005 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46481/revisions/3/mbox/
== Known issues ==
Here are the changes found in Patchwork_10005 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
fi-cfl-s3: PASS -> DMESG-FAIL (fdo#106560)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
{igt@pm_rpm@module-reload}:
fi-cnl-psr: PASS -> WARN (fdo#107602)
Possible fixes
igt@drv_selftest@live_hangcheck:
{fi-bdw-samus}: DMESG-FAIL (fdo#106560) -> PASS
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
igt@gem_sync@basic-many-each:
{fi-byt-clapper}: FAIL -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN -> 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#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
== Participating hosts (53 -> 48) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4703 -> Patchwork_10005
CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10005: a2a83254992b4983e9c32c3e622049a0db88f50a @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a2a83254992b drm/i915/kbl+: Enable IPC only for symmetric memory configurations
9776ee899bc1 drm/i915/skl+: don't trust IPC value set by BIOS
75013c1382d3 drm/i915: Implement 16GB dimm wa for latency level-0
3d1dd3f4ab6d drm/i915/skl+: Decode memory bandwidth and parameters
70f3f002256d drm/i915/bxt: Decode memory bandwidth and parameters
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10005/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Decode memdev info and bandwidth and implemnt latency WA (rev3)
== Series Details == Series: Decode memdev info and bandwidth and implemnt latency WA (rev3) URL : https://patchwork.freedesktop.org/series/46481/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/bxt: Decode memory bandwidth and parameters -drivers/gpu/drm/i915/selftests/../i915_drv.h:3685:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3696:16: warning: expression using sizeof(void) Commit: drm/i915/skl+: Decode memory bandwidth and parameters +drivers/gpu/drm/i915/i915_drv.c:1169:35: warning: expression using sizeof(void) +drivers/gpu/drm/i915/i915_drv.c:1169:35: warning: expression using sizeof(void) -drivers/gpu/drm/i915/selftests/../i915_drv.h:3696:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3704:16: warning: expression using sizeof(void) Commit: drm/i915: Implement 16GB dimm wa for latency level-0 -drivers/gpu/drm/i915/selftests/../i915_drv.h:3704:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3707:16: warning: expression using sizeof(void) Commit: drm/i915/skl+: don't trust IPC value set by BIOS Okay! Commit: drm/i915/kbl+: Enable IPC only for symmetric memory configurations -drivers/gpu/drm/i915/selftests/../i915_drv.h:3707:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3708:16: warning: expression using sizeof(void) ___ 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: Always enable mmio debugging for CI
== Series Details == Series: drm/i915: Always enable mmio debugging for CI URL : https://patchwork.freedesktop.org/series/48659/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4703_full -> Patchwork_10004_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10004_full that come from known issues: === IGT changes === Issues hit igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) 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_4703 -> Patchwork_10004 CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10004: a161c4b25d38447755ed624c0ab0fc4e4421f336 @ 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_10004/shards.html ___ 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: Always enable mmio debugging for CI
== Series Details ==
Series: drm/i915: Always enable mmio debugging for CI
URL : https://patchwork.freedesktop.org/series/48659/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4703 -> Patchwork_10004 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48659/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10004 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
fi-kbl-guc: PASS -> DMESG-FAIL (fdo#106947)
igt@drv_selftest@live_objects:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
fi-glk-dsi: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
fi-cnl-psr: PASS -> INCOMPLETE (fdo#105086)
fi-bxt-j4205: PASS -> INCOMPLETE (fdo#103927)
fi-glk-j4005: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362, fdo#103191)
{igt@kms_psr@primary_page_flip}:
{fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3
{igt@pm_rpm@module-reload}:
fi-cnl-psr: PASS -> WARN (fdo#107602)
Possible fixes
igt@drv_selftest@live_hangcheck:
{fi-bdw-samus}: DMESG-FAIL (fdo#106560) -> PASS
igt@gem_sync@basic-many-each:
{fi-byt-clapper}: FAIL -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN -> PASS
{igt@pm_rpm@module-reload}:
{fi-bsw-kefka}: DMESG-WARN -> PASS
fi-bsw-n3050: DMESG-WARN -> PASS
fi-byt-j1900: DMESG-WARN -> PASS
fi-byt-n2820: DMESG-WARN -> 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#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (53 -> 48) ==
Additional (1): fi-icl-u
Missing(6): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_4703 -> Patchwork_10004
CI_DRM_4703: 5dedf9d9259854872430c04a29737924731ba6f1 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10004: a161c4b25d38447755ed624c0ab0fc4e4421f336 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a161c4b25d38 drm/i915: Always enable mmio debugging for CI
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10004/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V2 5/5] drm/i915/kbl+: Enable IPC only for symmetric memory configurations
IPC may cause underflows if not used with dual channel symmetric
memory configuration. Disable IPC for non symmetric configurations in
affected platforms.
Display WA #1141
Changes Since V1:
- Re-arrange the code.
- update wrapper to return if memory is symmetric (Rodrigo)
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/i915/i915_drv.c | 27 ++-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 5 +
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2bc74c01a0e5..61d756ae7bf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1154,21 +1154,32 @@ skl_dram_get_channel_info(struct dram_channel_info *ch,
u32 val)
return 0;
}
+static bool
+intel_is_dram_symmetric(u32 val_ch0, u32 val_ch1,
+ struct dram_channel_info *ch0)
+{
+ return (val_ch0 == val_ch1 &&
+ (ch0->s_info.size == 0 ||
+(ch0->l_info.size == ch0->s_info.size &&
+ ch0->l_info.width == ch0->s_info.width &&
+ ch0->l_info.rank == ch0->s_info.rank)));
+}
+
static int
skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
{
struct dram_info *dram_info = _priv->dram_info;
struct dram_channel_info ch0, ch1;
- u32 val;
+ u32 val_ch0, val_ch1;
int ret;
- val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
- ret = skl_dram_get_channel_info(, val);
+ val_ch0 = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(, val_ch0);
if (ret == 0)
dram_info->num_channels++;
- val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
- ret = skl_dram_get_channel_info(, val);
+ val_ch1 = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(, val_ch1);
if (ret == 0)
dram_info->num_channels++;
@@ -1198,6 +1209,12 @@ skl_dram_get_channels_info(struct drm_i915_private
*dev_priv)
if (ch0.is_16gb_dimm || ch1.is_16gb_dimm)
dram_info->is_16gb_dimm = true;
+ dev_priv->dram_info.symmetric_memory = intel_is_dram_symmetric(val_ch0,
+ val_ch1,
+ );
+
+ DRM_DEBUG_KMS("memory configuration is %sSymmetric memory\n",
+ dev_priv->dram_info.symmetric_memory ? "" : "not ");
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6c432684c721..e7faa046f78a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1955,6 +1955,7 @@ struct drm_i915_private {
I915_DRAM_RANK_DUAL
} rank;
u32 bandwidth_kbps;
+ bool symmetric_memory;
} dram_info;
struct i915_runtime_pm runtime_pm;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 77970e38d939..c27646fb4cec 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6124,6 +6124,11 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
if (IS_SKYLAKE(dev_priv))
dev_priv->ipc_enabled = false;
+ /* Display WA #1141: SKL:all KBL:all CFL */
+ if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
+ !dev_priv->dram_info.symmetric_memory)
+ dev_priv->ipc_enabled = false;
+
val = I915_READ(DISP_ARB_CTL2);
if (dev_priv->ipc_enabled)
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 1/5] drm/i915/bxt: Decode memory bandwidth and parameters
This patch adds support to decode system memory bandwidth and other
parameters for broxton platform, which will be used for arbitrated
display memory bandwidth calculation in GEN9 based platforms and
WM latency level-0 Work-around calculation on GEN9+ platforms.
Changes since V1:
- s/memdev_info/dram_info
Changes since V2:
- Adhere to i915 coding style (Rodrigo)
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/i915/i915_drv.c | 117
drivers/gpu/drm/i915/i915_drv.h | 11
drivers/gpu/drm/i915/i915_reg.h | 30 +++
3 files changed, 158 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 77a4a01ddc08..37fb609e6f80 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1076,6 +1076,117 @@ static void intel_sanitize_options(struct
drm_i915_private *dev_priv)
intel_gvt_sanitize_options(dev_priv);
}
+static int
+bxt_get_dram_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = _priv->dram_info;
+ u32 dram_channels;
+ u32 mem_freq_khz, val;
+ u8 num_active_channels;
+ int i;
+
+ val = I915_READ(BXT_P_CR_MC_BIOS_REQ_0_0_0);
+ mem_freq_khz = DIV_ROUND_UP((val & BXT_REQ_DATA_MASK) *
+ BXT_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
+
+ dram_channels = val & BXT_DRAM_CHANNEL_ACTIVE_MASK;
+ num_active_channels = hweight32(dram_channels);
+
+ /* Each active bit represents 4-byte channel */
+ dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
+
+ if (dram_info->bandwidth_kbps == 0) {
+ DRM_INFO("Couldn't get system memory bandwidth\n");
+ return -EINVAL;
+ }
+
+ /*
+* Now read each DUNIT8/9/10/11 to check the rank of each dimms.
+*/
+ for (i = BXT_D_CR_DRP0_DUNIT_START; i <= BXT_D_CR_DRP0_DUNIT_END; i++) {
+ u8 size, width;
+ enum dram_rank rank;
+ u32 tmp;
+
+ val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
+ if (val == 0x)
+ continue;
+
+ dram_info->num_channels++;
+ tmp = val & BXT_DRAM_RANK_MASK;
+
+ if (tmp == BXT_DRAM_RANK_SINGLE)
+ rank = I915_DRAM_RANK_SINGLE;
+ else if (tmp == BXT_DRAM_RANK_DUAL)
+ rank = I915_DRAM_RANK_DUAL;
+ else
+ rank = I915_DRAM_RANK_INVALID;
+
+ tmp = val & BXT_DRAM_SIZE_MASK;
+ if (tmp == BXT_DRAM_SIZE_4GB)
+ size = 4;
+ else if (tmp == BXT_DRAM_SIZE_6GB)
+ size = 6;
+ else if (tmp == BXT_DRAM_SIZE_8GB)
+ size = 8;
+ else if (tmp == BXT_DRAM_SIZE_12GB)
+ size = 12;
+ else if (tmp == BXT_DRAM_SIZE_16GB)
+ size = 16;
+ else
+ size = 0;
+
+ tmp = (val & BXT_DRAM_WIDTH_MASK) >> BXT_DRAM_WIDTH_SHIFT;
+ width = (1 << tmp) * 8;
+ DRM_DEBUG_KMS("dram size:%dGB width:X%d rank:%s\n", size,
+ width, rank == I915_DRAM_RANK_SINGLE ? "single" :
+ rank == I915_DRAM_RANK_DUAL ? "dual" : "unknown");
+
+ /*
+* If any of the channel is single rank channel,
+* worst case output will be same as if single rank
+* memory, so consider single rank memory.
+*/
+ if (dram_info->rank == I915_DRAM_RANK_INVALID)
+ dram_info->rank = rank;
+ else if (rank == I915_DRAM_RANK_SINGLE)
+ dram_info->rank = I915_DRAM_RANK_SINGLE;
+ }
+
+ if (dram_info->rank == I915_DRAM_RANK_INVALID) {
+ DRM_INFO("couldn't get memory rank information\n");
+ return -EINVAL;
+ }
+
+ dram_info->valid = true;
+ return 0;
+}
+
+static void
+intel_get_dram_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = _priv->dram_info;
+ int ret;
+
+ dram_info->valid = false;
+ dram_info->rank = I915_DRAM_RANK_INVALID;
+ dram_info->bandwidth_kbps = 0;
+ dram_info->num_channels = 0;
+
+ if (!IS_BROXTON(dev_priv))
+ return;
+
+ ret = bxt_get_dram_info(dev_priv);
+ if (ret)
+ return;
+
+ DRM_DEBUG_KMS("DRAM bandwidth:%d KBps, total-channels: %u\n",
+ dram_info->bandwidth_kbps, dram_info->num_channels);
+ DRM_DEBUG_KMS("DRAM rank: %s rank\n",
+ (dram_info->rank == I915_DRAM_RANK_DUAL) ?
+ "dual" : "single");
+}
+
/**
* i915_driver_init_hw - setup state requiring device access
* @dev_priv:
[Intel-gfx] [PATCH V1 4/5] drm/i915/skl+: don't trust IPC value set by BIOS
If KMS decide to disable IPC make sure we override IPC configuration set
by BIOS.
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/i915/intel_pm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9550e24ffc2f..77970e38d939 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6121,10 +6121,8 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
u32 val;
/* Display WA #0477 WaDisableIPC: skl */
- if (IS_SKYLAKE(dev_priv)) {
+ if (IS_SKYLAKE(dev_priv))
dev_priv->ipc_enabled = false;
- return;
- }
val = I915_READ(DISP_ARB_CTL2);
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 3/5] drm/i915: Implement 16GB dimm wa for latency level-0
Memory with 16GB dimms require an increase of 1us in level-0 latency.
This patch implements the same.
Bspec: 4381
changes since V1:
- s/memdev_info/dram_info
- make skl_is_16gb_dimm pure function
Changes since V2:
- make is_16gb_dimm more generic
- rebase
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/i915/i915_drv.c | 33 +++--
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 13 +
3 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e09e9ce8fadf..2bc74c01a0e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1088,6 +1088,21 @@ static enum dram_rank skl_get_dimm_rank(u8 size, u32
rank)
return I915_DRAM_RANK_INVALID;
}
+static bool
+skl_is_16gb_dimm(enum dram_rank rank, u8 size, u8 width)
+{
+ if (rank == I915_DRAM_RANK_SINGLE && width == 8 && size == 16)
+ return true;
+ else if (rank == I915_DRAM_RANK_DUAL && width == 8 && size == 32)
+ return true;
+ else if (rank == SKL_DRAM_RANK_SINGLE && width == 16 && size == 8)
+ return true;
+ else if (rank == SKL_DRAM_RANK_DUAL && width == 16 && size == 16)
+ return true;
+
+ return false;
+}
+
static int
skl_dram_get_channel_info(struct dram_channel_info *ch, u32 val)
{
@@ -1125,6 +1140,11 @@ skl_dram_get_channel_info(struct dram_channel_info *ch,
u32 val)
else
ch->rank = I915_DRAM_RANK_SINGLE;
+ ch->is_16gb_dimm = skl_is_16gb_dimm(ch->l_info.rank, ch->l_info.size,
+ ch->l_info.width) ||
+ skl_is_16gb_dimm(ch->s_info.rank, ch->s_info.size,
+ ch->s_info.width);
+
DRM_DEBUG_KMS("(size:width:rank) L(%dGB:X%d:%s) S(%dGB:X%d:%s)\n",
ch->l_info.size, ch->l_info.width,
ch->l_info.rank ? "dual" : "single",
@@ -1157,6 +1177,8 @@ skl_dram_get_channels_info(struct drm_i915_private
*dev_priv)
return -EINVAL;
}
+ dram_info->valid_dimm = true;
+
/*
* If any of the channel is single rank channel, worst case output
* will be same as if single rank memory, so consider single rank
@@ -1172,6 +1194,10 @@ skl_dram_get_channels_info(struct drm_i915_private
*dev_priv)
DRM_INFO("couldn't get memory rank information\n");
return -EINVAL;
}
+
+ if (ch0.is_16gb_dimm || ch1.is_16gb_dimm)
+ dram_info->is_16gb_dimm = true;
+
return 0;
}
@@ -1284,6 +1310,7 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
return -EINVAL;
}
+ dram_info->valid_dimm = true;
dram_info->valid = true;
return 0;
}
@@ -1296,6 +1323,8 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
int ret;
dram_info->valid = false;
+ dram_info->valid_dimm = false;
+ dram_info->is_16gb_dimm = false;
dram_info->rank = I915_DRAM_RANK_INVALID;
dram_info->bandwidth_kbps = 0;
dram_info->num_channels = 0;
@@ -1319,9 +1348,9 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
sprintf(bandwidth_str, "unknown");
DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
bandwidth_str, dram_info->num_channels);
- DRM_DEBUG_KMS("DRAM rank: %s rank\n",
+ DRM_DEBUG_KMS("DRAM rank: %s rank 16GB-dimm:%s\n",
(dram_info->rank == I915_DRAM_RANK_DUAL) ?
- "dual" : "single");
+ "dual" : "single", yesno(dram_info->is_16gb_dimm));
}
/**
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0dfa0fdbbae2..6c432684c721 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1946,6 +1946,8 @@ struct drm_i915_private {
struct dram_info {
bool valid;
+ bool valid_dimm;
+ bool is_16gb_dimm;
u8 num_channels;
enum dram_rank {
I915_DRAM_RANK_INVALID = 0,
@@ -2174,6 +2176,7 @@ struct dram_channel_info {
enum dram_rank rank;
} l_info, s_info;
enum dram_rank rank;
+ bool is_16gb_dimm;
};
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d99e5fabe93c..9550e24ffc2f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2875,6 +2875,19 @@ static void intel_read_wm_latency(struct
drm_i915_private *dev_priv,
}
}
+ /*
+* WA Level-0 adjustment for 16GB DIMMs: SKL+
+* If we could not
[Intel-gfx] [PATCH V3 2/5] drm/i915/skl+: Decode memory bandwidth and parameters
This patch adds support to decode system memory bandwidth and other
parameters for skylake and Gen9+ platforms, which will be used for
arbitrated display memory bandwidth calculation in GEN9 based
platforms and WM latency level-0 Work-around calculation on GEN9+.
Changes Since V1:
- s/memdev_info/dram_info
- create a struct to hold channel info
Changes Since V2:
- rewrite code to adhere i915 coding style
- not valid for GLK
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/i915/i915_drv.c | 145 ++--
drivers/gpu/drm/i915/i915_drv.h | 8 +++
drivers/gpu/drm/i915/i915_reg.h | 18 +
3 files changed, 167 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 37fb609e6f80..e09e9ce8fadf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1076,6 +1076,132 @@ static void intel_sanitize_options(struct
drm_i915_private *dev_priv)
intel_gvt_sanitize_options(dev_priv);
}
+static enum dram_rank skl_get_dimm_rank(u8 size, u32 rank)
+{
+ if (size == 0)
+ return I915_DRAM_RANK_INVALID;
+ if (rank == SKL_DRAM_RANK_SINGLE)
+ return I915_DRAM_RANK_SINGLE;
+ else if (rank == SKL_DRAM_RANK_DUAL)
+ return I915_DRAM_RANK_DUAL;
+
+ return I915_DRAM_RANK_INVALID;
+}
+
+static int
+skl_dram_get_channel_info(struct dram_channel_info *ch, u32 val)
+{
+ u32 tmp_l, tmp_s;
+ u32 s_val = val >> SKL_DRAM_S_SHIFT;
+
+ if (!val)
+ return -EINVAL;
+
+ tmp_l = val & SKL_DRAM_SIZE_MASK;
+ tmp_s = s_val & SKL_DRAM_SIZE_MASK;
+
+ if (tmp_l == 0 && tmp_s == 0)
+ return -EINVAL;
+
+ ch->l_info.size = tmp_l;
+ ch->s_info.size = tmp_s;
+
+ tmp_l = (val & SKL_DRAM_WIDTH_MASK) >> SKL_DRAM_WIDTH_SHIFT;
+ tmp_s = (s_val & SKL_DRAM_WIDTH_MASK) >> SKL_DRAM_WIDTH_SHIFT;
+ ch->l_info.width = (1 << tmp_l) * 8;
+ ch->s_info.width = (1 << tmp_s) * 8;
+
+ tmp_l = val & SKL_DRAM_RANK_MASK;
+ tmp_s = s_val & SKL_DRAM_RANK_MASK;
+ ch->l_info.rank = skl_get_dimm_rank(ch->l_info.size, tmp_l);
+ ch->s_info.rank = skl_get_dimm_rank(ch->s_info.size, tmp_s);
+
+ if (ch->l_info.rank == I915_DRAM_RANK_DUAL ||
+ ch->s_info.rank == I915_DRAM_RANK_DUAL)
+ ch->rank = I915_DRAM_RANK_DUAL;
+ else if (ch->l_info.rank == I915_DRAM_RANK_SINGLE &&
+ch->s_info.rank == I915_DRAM_RANK_SINGLE)
+ ch->rank = I915_DRAM_RANK_DUAL;
+ else
+ ch->rank = I915_DRAM_RANK_SINGLE;
+
+ DRM_DEBUG_KMS("(size:width:rank) L(%dGB:X%d:%s) S(%dGB:X%d:%s)\n",
+ ch->l_info.size, ch->l_info.width,
+ ch->l_info.rank ? "dual" : "single",
+ ch->s_info.size, ch->s_info.width,
+ ch->s_info.rank ? "dual" : "single");
+
+ return 0;
+}
+
+static int
+skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = _priv->dram_info;
+ struct dram_channel_info ch0, ch1;
+ u32 val;
+ int ret;
+
+ val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(, val);
+ if (ret == 0)
+ dram_info->num_channels++;
+
+ val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(, val);
+ if (ret == 0)
+ dram_info->num_channels++;
+
+ if (dram_info->num_channels == 0) {
+ DRM_INFO("Number of memory channels is zero\n");
+ return -EINVAL;
+ }
+
+ /*
+* If any of the channel is single rank channel, worst case output
+* will be same as if single rank memory, so consider single rank
+* memory.
+*/
+ if (ch0.rank == I915_DRAM_RANK_SINGLE ||
+ ch1.rank == I915_DRAM_RANK_SINGLE)
+ dram_info->rank = I915_DRAM_RANK_SINGLE;
+ else
+ dram_info->rank = max(ch0.rank, ch1.rank);
+
+ if (dram_info->rank == I915_DRAM_RANK_INVALID) {
+ DRM_INFO("couldn't get memory rank information\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int
+skl_get_dram_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = _priv->dram_info;
+ u32 mem_freq_khz, val;
+ int ret;
+
+ ret = skl_dram_get_channels_info(dev_priv);
+ if (ret)
+ return ret;
+
+ val = I915_READ(SKL_MC_BIOS_DATA_0_0_0_MCHBAR_PCU);
+ mem_freq_khz = DIV_ROUND_UP((val & SKL_REQ_DATA_MASK) *
+ SKL_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
+
+ dram_info->bandwidth_kbps = dram_info->num_channels *
+ mem_freq_khz * 8;
+
+ if (dram_info->bandwidth_kbps == 0) {
+
[Intel-gfx] [PATCH 0/5] Decode memdev info and bandwidth and implemnt latency WA
This series adds support to calculate system memdev parameters and calculate total system memory bandwidth. This parameters and BW will be used to enable WM level-0 latency workaround and display memory bandwidth related WA for gen9. while we are here to enable IPC based on memory configuration lets also make sure that we override IPC value set of BIOS incase we decide to disable IPC in KMS. Mahesh Kumar (5): drm/i915/bxt: Decode memory bandwidth and parameters drm/i915/skl+: Decode memory bandwidth and parameters drm/i915: Implement 16GB dimm wa for latency level-0 drm/i915/skl+: don't trust IPC value set by BIOS drm/i915/kbl+: Enable IPC only for symmetric memory configurations drivers/gpu/drm/i915/i915_drv.c | 300 drivers/gpu/drm/i915/i915_drv.h | 23 +++ drivers/gpu/drm/i915/i915_reg.h | 48 +++ drivers/gpu/drm/i915/intel_pm.c | 22 ++- 4 files changed, 390 insertions(+), 3 deletions(-) -- 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Always enable mmio debugging for CI
The default behaviour is to periodically check for a mmio access error, and once detected enable mmio access checking. However this is useless if the error only occurs once. Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/Kconfig.debug | 12 drivers/gpu/drm/i915/i915_params.h | 8 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 9e36ffb5eb7c..ca947ed64d4e 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -31,6 +31,7 @@ config DRM_I915_DEBUG select DRM_I915_SW_FENCE_DEBUG_OBJECTS select DRM_I915_SELFTEST select DRM_I915_DEBUG_RUNTIME_PM + select DRM_I915_DEBUG_MMIO default n help Choose this option to turn on extra driver debugging that may affect @@ -40,6 +41,17 @@ config DRM_I915_DEBUG If in doubt, say "N". +config DRM_I915_DEBUG_MMIO +bool "Always insert extra checks around mmio access" +default n +help + Always enables the extra sanity checks (extra register reads) + around every mmio (register) access that will slow the system down. + + Recommended for driver developers only. + + If in doubt, say "N". + config DRM_I915_DEBUG_GEM bool "Insert extra checks into the GEM internals" default n diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 6c4d4a21474b..6a3308a5dfe1 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -33,6 +33,12 @@ struct drm_printer; #define ENABLE_GUC_SUBMISSION BIT(0) #define ENABLE_GUC_LOAD_HUCBIT(1) +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO) +#define MMIO_DEBUG_DFL -1 +#else +#define MMIO_DEBUG_DFL 0 +#endif + #define I915_PARAMS_FOR_EACH(param) \ param(char *, vbt_firmware, NULL) \ param(int, modeset, -1) \ @@ -51,7 +57,7 @@ struct drm_printer; param(char *, guc_firmware_path, NULL) \ param(char *, huc_firmware_path, NULL) \ param(char *, dmc_firmware_path, NULL) \ - param(int, mmio_debug, 0) \ + param(int, mmio_debug, MMIO_DEBUG_DFL) \ param(int, edp_vswing, 0) \ param(int, reset, 2) \ param(unsigned int, inject_load_failure, 0) \ -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: enable P010, P012, P016 formats for primary and sprite planes
On 16-Aug-18 9:21 PM, Maarten Lankhorst wrote:
Op 16-08-18 om 14:55 schreef Juha-Pekka Heikkila:
Enabling of P010, P012 and P016 formats. These formats will
extend NV12 for larger bit depths.
Signed-off-by: Juha-Pekka Heikkila
Reviewed-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 24 +-
drivers/gpu/drm/i915/intel_sprite.c | 39 +++-
2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 80ce742..5c7dc96 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -104,6 +104,25 @@ static const uint32_t skl_pri_planar_formats[] = {
DRM_FORMAT_NV12,
};
Hi Juha,
Shouldn't we add planar prefix with glk_primary_formats i.e.
glk_pri_planar_formats to make it more clear?
+static const uint32_t glk_primary_formats[] = {
+ DRM_FORMAT_C8,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_P010,
+ DRM_FORMAT_P012,
+ DRM_FORMAT_P016,
+};
+
static const uint64_t skl_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -13721,7 +13740,10 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
primary->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
PLANE_PRIMARY);
- if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
+ if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+ intel_primary_formats = glk_primary_formats;
+ num_formats = ARRAY_SIZE(glk_primary_formats);
+ } else if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY))
{
intel_primary_formats = skl_pri_planar_formats;
num_formats = ARRAY_SIZE(skl_pri_planar_formats);
} else {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 68db026..5cc97ba 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1292,6 +1292,22 @@ static uint32_t skl_planar_formats[] = {
DRM_FORMAT_NV12,
};
+static uint32_t glk_planar_formats[] = {
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_P010,
+ DRM_FORMAT_P012,
+ DRM_FORMAT_P016,
+};
+
static const uint64_t skl_plane_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -1537,7 +1553,28 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
}
intel_plane->base.state = >base;
- if (INTEL_GEN(dev_priv) >= 9) {
+ if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+ intel_plane->can_scale = true;
+ state->scaler_id = -1;
+
+ intel_plane->update_plane = skl_update_plane;
+ intel_plane->disable_plane = skl_disable_plane;
+ intel_plane->get_hw_state = skl_plane_get_hw_state;
+
+ if (skl_plane_has_planar(dev_priv, pipe,
+PLANE_SPRITE0 + plane)) {
+ plane_formats = glk_planar_formats;
+ num_plane_formats = ARRAY_SIZE(glk_planar_formats);
+ } else {
+ plane_formats = skl_plane_formats;
+ num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+ }
+
+ if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
+ modifiers = skl_plane_format_modifiers_ccs;
+ else
+ modifiers = skl_plane_format_modifiers_noccs;
+ } else if (INTEL_GEN(dev_priv) >= 9) {
intel_plane->can_scale = true;
state->scaler_id = -1;
Tested, still works ok against IGT. :)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Thanks and Regards,
Swati
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
Hi John, On Friday, 24 August 2018 00:12:46 EEST John Stultz wrote: > On Thu, Aug 23, 2018 at 1:49 PM, Laurent Pinchart wrote: > > On Thursday, 23 August 2018 20:48:40 EEST John Stultz wrote: > >> On Thu, Aug 23, 2018 at 1:09 AM, Daniel Vetter wrote: > >>> On Thu, Aug 23, 2018 at 10:46:15AM +0300, Laurent Pinchart wrote: > Possibly slightly out of topic, but we're in 2018, is there any plan > to make SurfaceFlinger move away from FBDEV ? > >>> > >>> Is surfaceflinger really using direct fbdev still (maybe for boot-up)? > >>> Or is this just an artifact of the mali blob hwcomposer backend? > >> > >> Mostly its due to the simple fbdev being a legacy solution on android > >> that works out of the box. > >> I do suspect the android devs hope to retire it, which is why I'm > >> working on getting things going w/ the drm_hwcomposer right now so we > >> can get away from the fbdev. > > > > That would be good news. Are there many Android components other than > > vendor- specific hwcomposer implementations that still use fbdev ? > > So yea, I can't really speak about what the various vendors are doing, > as I don't really know, but I'm aware there are still a few (in some > cases major) vendors who still use fbdev on their shipping devices > with their custom hwcomposer code. > > Other then that, to my knowledge AOSP only has a default fallback > hwcomposer that uses fbdev, which is what we've used here as we didn't > want to take the vendor's proprietary hwcomposer blob. But again, > moving to the drm_hwcomposer is the shiny bright future, as soon as a > few remaining issues are sorted upstream. Last time I looked (and that was years ago) the init process also used fbdev to render the boot splash screen. Is it still the case ? If so is there any chance you could add a fix for that to your todo list ? :-) -- Regards, Laurent Pinchart ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] RFC: Migration to Gitlab
On Wed, 22 Aug 2018, Rodrigo Vivi wrote: > On Wed, Aug 22, 2018 at 10:19:19AM -0400, Adam Jackson wrote: >> On Wed, 2018-08-22 at 16:13 +0300, Jani Nikula wrote: >> >> > - Sticking to fdo bugzilla and disabling gitlab issues for at least >> > drm-intel for the time being. Doing that migration in the same go is a >> > bit much I think. Reassignment across bugzilla and gitlab will be an >> > issue. >> >> Can you elaborate a bit on the issues here? The actual move-the-bugs >> process has been pretty painless for the parts of xorg we've done so >> far. > > I guess there is nothing against moving the bugs there. The concern is only on > doing everything at once. No, it's not just that. We have some automation using the bugzilla APIs directly, and someone(tm) needs to figure out how this should work with gitlab. Maybe we have a better chance of doing things with gitlab's APIs, maybe we can reduce our dependence on external logic altogether. We have special "i915 platform" and "i915 features" fields in bugzilla. We use a mailing list default assignee. Some of us use the "whiteboard" and "keywords" fields. Etc. I don't think figuring all this out is rocket science, but someone needs to actually do it, and get our workflows straightened out *before* we flip the switch. I'm just trying to figure out if that is a blocker to migrating the repos. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dsc: Fix PPS register definition macros for 2nd VDSC engine
On Thu, Aug 23, 2018 at 06:48:07PM -0700, Manasi Navare wrote:
> This patch fixes the PPS4 and PPS register definition macros that were
> resulting into an incorect MMIO address.
>
> Fixes: 2efbb2f099fb ("i915/dp/dsc: Add DSC PPS register definitions")
> Cc: Anusha Srivatsa
> Signed-off-by: Manasi Navare
Reviewed-by: Rodrigo Vivi
(also checked others around to see if there was similar issues,
but the rest seems right)
> ---
> drivers/gpu/drm/i915/i915_reg.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 41ab5b56ee52..64d7e675f7e8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10488,7 +10488,7 @@ enum skl_power_gate {
>
> _ICL_DSC0_PICTURE_PARAMETER_SET_4_PB, \
>
> _ICL_DSC0_PICTURE_PARAMETER_SET_4_PC)
> #define ICL_DSC1_PICTURE_PARAMETER_SET_4(pipe) _MMIO_PIPE((pipe) -
> PIPE_B, \
> -
> _ICL_DSC0_PICTURE_PARAMETER_SET_4_PB, \
> +
> _ICL_DSC1_PICTURE_PARAMETER_SET_4_PB, \
>
> _ICL_DSC1_PICTURE_PARAMETER_SET_4_PC)
> #define DSC_INITIAL_DEC_DELAY(dec_delay) ((dec_delay) << 16)
> #define DSC_INITIAL_XMIT_DELAY(xmit_delay) ((xmit_delay) << 0)
> @@ -10503,7 +10503,7 @@ enum skl_power_gate {
>
> _ICL_DSC0_PICTURE_PARAMETER_SET_5_PB, \
>
> _ICL_DSC0_PICTURE_PARAMETER_SET_5_PC)
> #define ICL_DSC1_PICTURE_PARAMETER_SET_5(pipe) _MMIO_PIPE((pipe) -
> PIPE_B, \
> -
> _ICL_DSC1_PICTURE_PARAMETER_SET_5_PC, \
> +
> _ICL_DSC1_PICTURE_PARAMETER_SET_5_PB, \
>
> _ICL_DSC1_PICTURE_PARAMETER_SET_5_PC)
> #define DSC_SCALE_DEC_INT(scale_dec)((scale_dec) << 16)
> #define DSC_SCALE_INC_INT(scale_inc)((scale_inc) << 0)
> --
> 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
