Re: [Intel-gfx] [igt-dev] [PATH i-g-t 2/2] tests: add slice power programming test
Quoting Tvrtko Ursulin (2018-09-05 15:25:44) > From: Lionel Landwerlin > > Verifies that the kernel programs slices correctly based by reading > the value of PWR_CLK_STATE register or MI_SET_PREDICATE on platforms > before Cannonlake. > > v2: Add subslice tests (Lionel) > Use MI_SET_PREDICATE for further verification when available (Lionel) > > v3: Rename to gem_ctx_rpcs (Lionel) > > v4: Update kernel API (Lionel) > Add 0 value test (Lionel) > Exercise invalid values (Lionel) > > v5: Add perf tests (Lionel) > > v6: Add new sysfs entry tests (Lionel) > > v7: Test rsvd fields > Update for kernel series changes > > v8: Drop test_no_sseu_support() test (Kelvin) > Drop drm_intel_*() apis (Chris) > > v9: by Chris: > Drop all do_ioctl/do_ioctl_err() > Use gem_context_[gs]et_param() > Use gem_read() instead of mapping memory > by Lionel: > Test dynamic sseu on/off more > > Tvrtko Ursulin: > > v10: > * Various style tweaks and refactorings. > * New test coverage. I didn't notice any testing of: - param->size - feeding garbage into param->value user pointer (always cleared before use, perhaps just poison instead), along with abusive pointers. E.g., how does the code fare if we pass in an unfaulted GGTT mmap? We so need a cook book (even automated) for pointer abuse. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: add LG eDP panel to quirk database
== Series Details ==
Series: drm/i915: add LG eDP panel to quirk database
URL : https://patchwork.freedesktop.org/series/49251/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
e5c2ff993f03 drm/i915: add LG eDP panel to quirk database
-:36: WARNING:LONG_LINE: line over 100 characters
#36: FILE: drivers/gpu/drm/drm_dp_helper.c:1271:
+ { OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N),
DEVICE_ID(0, 0, 0, 0, 0, 0) },
-:38: WARNING:LONG_LINE: line over 100 characters
#38: FILE: drivers/gpu/drm/drm_dp_helper.c:1273:
+ { OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N),
DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
total: 0 errors, 2 warnings, 0 checks, 127 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: add LG eDP panel to quirk database
== Series Details == Series: drm/i915: add LG eDP panel to quirk database URL : https://patchwork.freedesktop.org/series/49251/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915: add LG eDP panel to quirk database -O:drivers/gpu/drm/i915/intel_display.c:6702:18: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_display.c:6705:26: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm: Reject unknown legacy bpp and depth for drm_mode_addfb ioctl
Quoting Chris Wilson (2018-09-05 16:31:16)
> Since this is handling user provided bpp and depth, we need to sanity
> check and propagate the EINVAL back rather than assume what the insane
> client intended and fill the logs with DRM_ERROR.
>
> v2: Check both bpp and depth match the builtin pixel format, and
> introduce a canonical DRM_FORMAT_INVALID to reserve 0 against any future
> fourcc.
>
> v3: Mark up DRM_FORMAT_C8 as being {bpp:8, depth:8}
>
> Testcase: igt/kms_addfb_basic/legacy-format
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: Ville Syrjälä
> Cc: Michel Dänzer
> Reviewed-by: Daniel Vetter
Pushed the sanity check. Hopefully we are all happy with
DRM_FORMAT_C8: {bpp:8, depth:8}
DRM_FORMAT_XRGB1555:{bpp:16, depth:15}
DRM_FORMAT_RGB565: {bpp:16, depth:16}
DRM_FORMAT_RGB888: {bpp:24, depth:24}
DRM_FORMAT_XRGB:{bpp:32, depth:24}
DRM_FORMAT_XRGB2101010: {bpp:32, depth:30}
DRM_FORMAT_ARGB:{bpp:32, depth:32}
Thanks,
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: add LG eDP panel to quirk database
== Series Details == Series: drm/i915: add LG eDP panel to quirk database URL : https://patchwork.freedesktop.org/series/49251/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4776 -> Patchwork_10105 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49251/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10105 that come from known issues: === IGT changes === Issues hit igt@kms_pipe_crc_basic@read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#107362) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) Possible fixes igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-byt-clapper: INCOMPLETE (fdo#102657) -> PASS fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 == Participating hosts (53 -> 47) == Missing(6): fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4776 -> Patchwork_10105 CI_DRM_4776: 18e1a6da2ae33504c83d1e5dfc7a08a3940e7b58 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10105: e5c2ff993f03a1fc46874810f8c730f222d9e7ca @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == e5c2ff993f03 drm/i915: add LG eDP panel to quirk database == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10105/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add LG eDP panel to quirk database
On Thu, 06 Sep 2018, "Lee, Shawn C" wrote:
> The N value was computed by kernel driver that based on synchronous clock
> mode. But only specific N value (0x8000) would be acceptable for
> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
As I wrote before, it's the DP source that determines between
synchronous and asynchronous clock mode, not the sink. The sink in
question appears to expect a constant N related to asynchronous clock
mode even when we're using and advertising synchronous clock mode in DP
Main Stream Attributes.
(It's possible a certain other OS uses the same constant N regardless of
clock mode, leading to the panel working by coincidence.)
> With the other N value, Tcon will enter BITS mode and display black screen.
> Add this panel into quirk database and give particular N value when
> calculate M/N divider.
>
> Cc: Jani Nikula
> Cc: Cooper Chiou
> Cc: Matt Atwood
> Signed-off-by: Lee, Shawn C
> ---
> drivers/gpu/drm/drm_dp_helper.c | 10 +-
> drivers/gpu/drm/i915/intel_display.c | 16 ++--
> drivers/gpu/drm/i915/intel_display.h | 3 ++-
> drivers/gpu/drm/i915/intel_dp.c | 8 ++--
> drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
> include/drm/drm_dp_helper.h | 1 +
> 6 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..a0259bbbe9df 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
> u8 oui[3];
> bool is_branch;
> u32 quirks;
> + u8 dev_id[6];
Your commit message says nothing about the need to extend the quirk
mechanism to use the device ID. Do we really need it?
If we do need it, please name it device_id like it is in struct
drm_dp_dpcd_ident, and place it after oui.
> };
>
> #define OUI(first, second, third) { (first), (second), (third) }
> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
> + { (first), (second), (third), (fourth), (fifth), (sixth) }
>
> static const struct dpcd_quirk dpcd_quirk_list[] = {
> /* Analogix 7737 needs reduced M and N at HBR2 link rates */
> - { OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> + { OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N),
> DEVICE_ID(0, 0, 0, 0, 0, 0) },
Maybe add #define DEVICE_ID_ANY to initialize all to zero.
> + /* LG LP140WF6-SPM1 eDP panel */
> + { OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N),
> DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
> };
>
> #undef OUI
#undef DEVICE_ID
#undef DEVICE_ID_ANY
> @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident
> *ident, bool is_branch)
> if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
> continue;
>
> + if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) !=
> 0)
> + continue;
> +
Why not branch devices? This is really a trick to avoid matching on the
Analogix device which you set to zeros, and now you require device id
match for all non-branch quirks. Not good. Please check if the quirk
table has all zeros, and compare if not.
> quirks |= quirk->quirks;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ec3e24f07486..ff01a742b054 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct
> intel_crtc *intel_crtc,
> pipe_config->fdi_lanes = lane;
>
> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> -link_bw, &pipe_config->fdi_m_n, false);
> +link_bw, &pipe_config->fdi_m_n, false, false);
This is getting pretty ugly, to be honest. Which is why I was wondering
if we could actually turn the Analogix quirk into a constant N...
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
> @@ -6680,7 +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>
> static void compute_m_n(unsigned int m, unsigned int n,
> uint32_t *ret_m, uint32_t *ret_n,
> - bool reduce_m_n)
> + bool reduce_m_n, bool constant_n)
> {
> /*
>* Reduce M/N as much as possible without loss in precision. Several DP
> @@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
> }
> }
>
> - *ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
> + if (constant_n)
> + *ret_n = 0x8000;
> + else
> + *ret_n = min_t(unsigned int, roundup_pow_of_two(n),
> DATA_LINK_N_MAX);
> +
> *ret_m = div_u64((uint64_t
Re: [Intel-gfx] [PATCH] drm/i915: add LG eDP panel to quirk database
On Thu, 06 Sep 2018, "Lee, Shawn C" wrote:
> The N value was computed by kernel driver that based on synchronous clock
> mode. But only specific N value (0x8000) would be acceptable for
> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
> With the other N value, Tcon will enter BITS mode and display black screen.
> Add this panel into quirk database and give particular N value when
> calculate M/N divider.
>
> Cc: Jani Nikula
> Cc: Cooper Chiou
> Cc: Matt Atwood
> Signed-off-by: Lee, Shawn C
> ---
> drivers/gpu/drm/drm_dp_helper.c | 10 +-
PS. This is not drm/i915. This is DRM.
> drivers/gpu/drm/i915/intel_display.c | 16 ++--
> drivers/gpu/drm/i915/intel_display.h | 3 ++-
> drivers/gpu/drm/i915/intel_dp.c | 8 ++--
> drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
> include/drm/drm_dp_helper.h | 1 +
> 6 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..a0259bbbe9df 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
> u8 oui[3];
> bool is_branch;
> u32 quirks;
> + u8 dev_id[6];
> };
>
> #define OUI(first, second, third) { (first), (second), (third) }
> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
> + { (first), (second), (third), (fourth), (fifth), (sixth) }
>
> static const struct dpcd_quirk dpcd_quirk_list[] = {
> /* Analogix 7737 needs reduced M and N at HBR2 link rates */
> - { OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> + { OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N),
> DEVICE_ID(0, 0, 0, 0, 0, 0) },
> + /* LG LP140WF6-SPM1 eDP panel */
> + { OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N),
> DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
> };
>
> #undef OUI
> @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident
> *ident, bool is_branch)
> if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
> continue;
>
> + if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) !=
> 0)
> + continue;
> +
> quirks |= quirk->quirks;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ec3e24f07486..ff01a742b054 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct
> intel_crtc *intel_crtc,
> pipe_config->fdi_lanes = lane;
>
> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> -link_bw, &pipe_config->fdi_m_n, false);
> +link_bw, &pipe_config->fdi_m_n, false, false);
>
> ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
> @@ -6680,7 +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>
> static void compute_m_n(unsigned int m, unsigned int n,
> uint32_t *ret_m, uint32_t *ret_n,
> - bool reduce_m_n)
> + bool reduce_m_n, bool constant_n)
> {
> /*
>* Reduce M/N as much as possible without loss in precision. Several DP
> @@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
> }
> }
>
> - *ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
> + if (constant_n)
> + *ret_n = 0x8000;
> + else
> + *ret_n = min_t(unsigned int, roundup_pow_of_two(n),
> DATA_LINK_N_MAX);
> +
> *ret_m = div_u64((uint64_t) m * *ret_n, n);
> intel_reduce_m_n_ratio(ret_m, ret_n);
> }
> @@ -6704,18 +6708,18 @@ void
> intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> -bool reduce_m_n)
> +bool reduce_m_n, bool constant_n)
> {
> m_n->tu = 64;
>
> compute_m_n(bits_per_pixel * pixel_clock,
> link_clock * nlanes * 8,
> &m_n->gmch_m, &m_n->gmch_n,
> - reduce_m_n);
> + reduce_m_n, constant_n);
>
> compute_m_n(pixel_clock, link_clock,
> &m_n->link_m, &m_n->link_n,
> - reduce_m_n);
> + reduce_m_n, constant_n);
> }
>
> static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_display.h
> b/drivers/gpu/drm/i915/intel_display.h
> index 43f080c6538d..dde7d73bd36d 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -379,7 +3
[Intel-gfx] [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world
Using the guc, we cannot disable the user interrupt generation as we use
it for driving submission. And from Icelake, we no longer have the
ability to individually mask interrupt generation from each engine,
disabling our ability to fake missed interrupts.
In both cases, report back to userspace that the missed interrupt
generator is no longer available.
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 1f7051e97afb..b4744a68cd88 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4117,6 +4117,17 @@ i915_ring_test_irq_set(void *data, u64 val)
{
struct drm_i915_private *i915 = data;
+ /* GuC keeps the user interrupt permanently enabled for submission */
+ if (USES_GUC_SUBMISSION(i915))
+ return -ENODEV;
+
+ /*
+* From icl, we can no longer individually mask interrupt generation
+* from each engine.
+*/
+ if (INTEL_GEN(i915) >= 11)
+ return -ENODEV;
+
val &= INTEL_INFO(i915)->ring_mask;
DRM_DEBUG_DRIVER("Masking interrupts on rings 0x%08llx\n", val);
--
2.19.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915/execlists: Reset CSB pointers on canceling requests (wedging)
The prior assumption was that we did not need to reset the CSB on
wedging when cancelling the outstanding requests as it would be cleaned
up in the subsequent reset prior to restarting the GPU. However, what
was not accounted for was that in performing the reset, we would try to
process the outstanding CSB entries. If the GPU happened to complete a
CS event just as we were performing the cancellation of requests, that
event would be kept in the CSB until the reset -- but our bookkeeping
was cleared, causing confusion when trying to complete the CS event.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/intel_lrc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9b1f0e5211a0..066ab178a8b2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -850,6 +850,7 @@ static void execlists_cancel_requests(struct
intel_engine_cs *engine)
/* Cancel the requests on the HW and clear the ELSP tracker. */
execlists_cancel_port_requests(execlists);
execlists_user_end(execlists);
+ reset_csb_pointers(execlists);
/* Mark all executing requests as skipped. */
list_for_each_entry(rq, &engine->timeline.requests, link) {
--
2.19.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/7] drm/i915/execlists: Use coherent writes into the context image
That we use a WB mapping for updating the RING_TAIL register inside the
context image even on !llc machines has been a source of consternation
for every reader. It appears to work on bsw+, but it may just have been
that we have been incredibly bad at detecting the errors.
v2: With extra enthusiasm.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 6 ++
drivers/gpu/drm/i915/i915_gem.c | 2 ++
drivers/gpu/drm/i915/i915_perf.c| 3 ++-
drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c| 8 +---
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
6 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 767615ecdea5..db006ff73827 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3073,6 +3073,12 @@ enum i915_map_type {
I915_MAP_FORCE_WC = I915_MAP_WC | I915_MAP_OVERRIDE,
};
+static inline enum i915_map_type
+i915_coherent_map_type(struct drm_i915_private *i915)
+{
+ return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
+}
+
/**
* i915_gem_object_pin_map - return a contiguous mapping of the entire object
* @obj: the object to map into kernel address space
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 89834ce19acd..d6f2bbd6a0dc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5417,6 +5417,8 @@ static int __intel_engines_record_defaults(struct
drm_i915_private *i915)
for_each_engine(engine, i915, id) {
struct i915_vma *state;
+ GEM_BUG_ON(to_intel_context(ctx, engine)->pin_count);
+
state = to_intel_context(ctx, engine)->state;
if (!state)
continue;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index ccb20230df2c..0dabfeb2297a 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1847,7 +1847,8 @@ static int gen8_configure_all_contexts(struct
drm_i915_private *dev_priv,
if (!ce->state)
continue;
- regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
+ regs = i915_gem_object_pin_map(ce->state->obj,
+ i915_coherent_map_type(dev_priv));
if (IS_ERR(regs))
return PTR_ERR(regs);
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 10cd051ba29e..c99f2cb9b0e1 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1150,7 +1150,7 @@ void intel_engines_unpark(struct drm_i915_private *i915)
map = NULL;
if (engine->default_state)
map = i915_gem_object_pin_map(engine->default_state,
- I915_MAP_WB);
+ I915_MAP_FORCE_WB);
if (!IS_ERR_OR_NULL(map))
engine->pinned_default_state = map;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 595ff42e7662..b8648449817c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1314,7 +1314,7 @@ static int __context_pin(struct i915_gem_context *ctx,
struct i915_vma *vma)
* on an active context (which by nature is already on the GPU).
*/
if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
- err = i915_gem_object_set_to_gtt_domain(vma->obj, true);
+ err = i915_gem_object_set_to_wc_domain(vma->obj, true);
if (err)
return err;
}
@@ -1342,7 +1342,9 @@ __execlists_context_pin(struct intel_engine_cs *engine,
if (ret)
goto err;
- vaddr = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
+ vaddr = i915_gem_object_pin_map(ce->state->obj,
+ i915_coherent_map_type(ctx->i915) |
+ I915_MAP_OVERRIDE);
if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);
goto unpin_vma;
@@ -2771,7 +2773,7 @@ populate_lr_context(struct i915_gem_context *ctx,
void *defaults;
defaults = i915_gem_object_pin_map(engine->default_state,
- I915_MAP_WB);
+ I915_MAP_FORCE_WB);
if (IS_ERR(defaults)) {
ret = PTR_ERR(defaults);
goto err_unpin_ctx;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 472939f5c18f..266c6d047d10 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/dri
[Intel-gfx] [PATCH 3/7] drm/i915/execlists: Avoid kicking priority on the current context
If the request is currently on the HW (in port 0), then we do not need
to kick the submission tasklet to evaluate whether we should be
preempting itself in order to execute it again.
In the case that was annoying me:
execlists_schedule: rq(18:211173).prio=0 -> 2
need_preempt: last(18:211174).prio=0, queue.prio=2
We are bumping the priority of the first of a pair of requests running
in the current context. Then when evaluating preempt, we would see that
that our priority request is higher than the last executing request in
ELSP0 and so trigger preemption, not realising that our intended request
was already executing.
v2: As we assume state of the execlists->port[] that is only valid while
we hold the timeline lock we have to repeat some earlier tests that on
the validity of the node.
v3: Wrap guc submission under the timeline.lock as is now the way of all
things.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_guc_submission.c | 18 +++--
drivers/gpu/drm/i915/intel_lrc.c| 41 +++--
2 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c
b/drivers/gpu/drm/i915/intel_guc_submission.c
index 07b9d313b019..7b878790228a 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -771,19 +771,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
static void guc_dequeue(struct intel_engine_cs *engine)
{
- unsigned long flags;
- bool submit;
-
- local_irq_save(flags);
-
- spin_lock(&engine->timeline.lock);
- submit = __guc_dequeue(engine);
- spin_unlock(&engine->timeline.lock);
-
- if (submit)
+ if ( __guc_dequeue(engine))
guc_submit(engine);
-
- local_irq_restore(flags);
}
static void guc_submission_tasklet(unsigned long data)
@@ -792,6 +781,9 @@ static void guc_submission_tasklet(unsigned long data)
struct intel_engine_execlists * const execlists = &engine->execlists;
struct execlist_port *port = execlists->port;
struct i915_request *rq;
+ unsigned long flags;
+
+ spin_lock_irqsave(&engine->timeline.lock, flags);
rq = port_request(port);
while (rq && i915_request_completed(rq)) {
@@ -815,6 +807,8 @@ static void guc_submission_tasklet(unsigned long data)
if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
guc_dequeue(engine);
+
+ spin_unlock_irqrestore(&engine->timeline.lock, flags);
}
static struct i915_request *
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 066ab178a8b2..881ab979f02f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -355,13 +355,8 @@ execlists_unwind_incomplete_requests(struct
intel_engine_execlists *execlists)
{
struct intel_engine_cs *engine =
container_of(execlists, typeof(*engine), execlists);
- unsigned long flags;
-
- spin_lock_irqsave(&engine->timeline.lock, flags);
__unwind_incomplete_requests(engine);
-
- spin_unlock_irqrestore(&engine->timeline.lock, flags);
}
static inline void
@@ -1234,9 +1229,13 @@ static void execlists_schedule(struct i915_request
*request,
engine = sched_lock_engine(node, engine);
+ /* Recheck after acquiring the engine->timeline.lock */
if (prio <= node->attr.priority)
continue;
+ if (i915_sched_node_signaled(node))
+ continue;
+
node->attr.priority = prio;
if (!list_empty(&node->link)) {
if (last != engine) {
@@ -1245,14 +1244,34 @@ static void execlists_schedule(struct i915_request
*request,
}
GEM_BUG_ON(pl->priority != prio);
list_move_tail(&node->link, &pl->requests);
+ } else {
+ /*
+* If the request is not in the priolist queue because
+* it is not yet runnable, then it doesn't contribute
+* to our preemption decisions. On the other hand,
+* if the request is on the HW, it too is not in the
+* queue; but in that case we may still need to reorder
+* the inflight requests.
+*/
+ if
(!i915_sw_fence_done(&sched_to_request(node)->submit))
+ continue;
}
- if (prio > engine->execlists.queue_priority &&
- i915_sw_fence_done(&sched_to_request(node)->submit)) {
- /* defer submission until after all of our updates */
- __update_queue(engine, prio);
- tasklet_hi_schedule(
[Intel-gfx] [PATCH 4/7] drm/i915/selftests: Basic stress test for rapid context switching
We need to exercise the HW and submission paths for switching contexts
rapidly to check that features such as execlists' wa_tail are adequate.
Plus it's an interesting baseline latency metric.
v2: Check the initial request for allocation errors
Signed-off-by: Chris Wilson
---
.../gpu/drm/i915/selftests/i915_gem_context.c | 188 ++
1 file changed, 188 insertions(+)
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 1c92560d35da..24734dd91309 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -22,6 +22,8 @@
*
*/
+#include
+
#include "../i915_selftest.h"
#include "i915_random.h"
#include "igt_flush_test.h"
@@ -32,6 +34,191 @@
#define DW_PER_PAGE (PAGE_SIZE / sizeof(u32))
+struct live_test {
+ struct drm_i915_private *i915;
+ const char *func;
+ const char *name;
+
+ unsigned int reset_count;
+};
+
+static int begin_live_test(struct live_test *t,
+ struct drm_i915_private *i915,
+ const char *func,
+ const char *name)
+{
+ int err;
+
+ t->i915 = i915;
+ t->func = func;
+ t->name = name;
+
+ err = i915_gem_wait_for_idle(i915,
+I915_WAIT_LOCKED,
+MAX_SCHEDULE_TIMEOUT);
+ if (err) {
+ pr_err("%s(%s): failed to idle before, with err=%d!",
+ func, name, err);
+ return err;
+ }
+
+ i915->gpu_error.missed_irq_rings = 0;
+ t->reset_count = i915_reset_count(&i915->gpu_error);
+
+ return 0;
+}
+
+static int end_live_test(struct live_test *t)
+{
+ struct drm_i915_private *i915 = t->i915;
+
+ i915_retire_requests(i915);
+
+ if (wait_for(intel_engines_are_idle(i915), 10)) {
+ pr_err("%s(%s): GPU not idle\n", t->func, t->name);
+ return -EIO;
+ }
+
+ if (t->reset_count != i915_reset_count(&i915->gpu_error)) {
+ pr_err("%s(%s): GPU was reset %d times!\n",
+ t->func, t->name,
+ i915_reset_count(&i915->gpu_error) - t->reset_count);
+ return -EIO;
+ }
+
+ if (i915->gpu_error.missed_irq_rings) {
+ pr_err("%s(%s): Missed interrupts on engines %lx\n",
+ t->func, t->name, i915->gpu_error.missed_irq_rings);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int live_nop_switch(void *arg)
+{
+ const unsigned int nctx = 1024;
+ struct drm_i915_private *i915 = arg;
+ struct intel_engine_cs *engine;
+ struct i915_gem_context **ctx;
+ enum intel_engine_id id;
+ struct drm_file *file;
+ struct live_test t;
+ unsigned long n;
+ int err = -ENODEV;
+
+ /*
+* Create as many contexts as we can feasibly get away with
+* and check we can switch between them rapidly.
+*
+* Serves as very simple stress test for submission and HW switching
+* between contexts.
+*/
+
+ if (!DRIVER_CAPS(i915)->has_logical_contexts)
+ return 0;
+
+ file = mock_file(i915);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ mutex_lock(&i915->drm.struct_mutex);
+
+ ctx = kcalloc(nctx, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ for (n = 0; n < nctx; n++) {
+ ctx[n] = i915_gem_create_context(i915, file->driver_priv);
+ if (IS_ERR(ctx[n])) {
+ err = PTR_ERR(ctx[n]);
+ goto out_unlock;
+ }
+ }
+
+ for_each_engine(engine, i915, id) {
+ struct i915_request *rq;
+ unsigned long end_time, prime;
+ ktime_t times[2] = {};
+
+ times[0] = ktime_get_raw();
+ for (n = 0; n < nctx; n++) {
+ rq = i915_request_alloc(engine, ctx[n]);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto out_unlock;
+ }
+ i915_request_add(rq);
+ }
+ i915_request_wait(rq,
+ I915_WAIT_LOCKED,
+ MAX_SCHEDULE_TIMEOUT);
+ times[1] = ktime_get_raw();
+
+ pr_info("Populated %d contexts on %s in %lluns\n",
+ nctx, engine->name, ktime_to_ns(times[1] - times[0]));
+
+ err = begin_live_test(&t, i915, __func__, engine->name);
+ if (err)
+ goto out_unlock;
+
+ end_time = jiffies + i915_selftest.timeout_jiffies;
+
[Intel-gfx] [PATCH 7/7] drm/i915/execlists: Onion unwind for logical_ring_init() failure
Fix up the error unwind for logical_ring_init() failing by moving the
cleanup into the callers who own the various bits of state during
initialisation.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b8648449817c..ebf05be35f4d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2414,7 +2414,7 @@ static int logical_ring_init(struct intel_engine_cs
*engine)
ret = intel_engine_init_common(engine);
if (ret)
- goto error;
+ return ret;
if (HAS_LOGICAL_RING_ELSQ(i915)) {
execlists->submit_reg = i915->regs +
@@ -2456,10 +2456,6 @@ static int logical_ring_init(struct intel_engine_cs
*engine)
reset_csb_pointers(execlists);
return 0;
-
-error:
- intel_logical_ring_cleanup(engine);
- return ret;
}
int logical_render_ring_init(struct intel_engine_cs *engine)
@@ -2482,10 +2478,14 @@ int logical_render_ring_init(struct intel_engine_cs
*engine)
engine->emit_breadcrumb = gen8_emit_breadcrumb_rcs;
engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_rcs_sz;
- ret = intel_engine_create_scratch(engine, PAGE_SIZE);
+ ret = logical_ring_init(engine);
if (ret)
return ret;
+ ret = intel_engine_create_scratch(engine, PAGE_SIZE);
+ if (ret)
+ goto err_cleanup_common;
+
ret = intel_init_workaround_bb(engine);
if (ret) {
/*
@@ -2497,7 +2497,11 @@ int logical_render_ring_init(struct intel_engine_cs
*engine)
ret);
}
- return logical_ring_init(engine);
+ return 0;
+
+err_cleanup_common:
+ intel_engine_cleanup_common(engine);
+ return ret;
}
int logical_xcs_ring_init(struct intel_engine_cs *engine)
--
2.19.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/i915/execlists: Delay updating ring register state after resume
Now that we reload both RING_HEAD and RING_TAIL when rebinding the
context, we do not need to scrub those registers immediately on resume.
v2: Handle the perma-pinned contexts.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_lrc.c | 29 +++--
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 881ab979f02f..595ff42e7662 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2861,13 +2861,14 @@ static int execlists_context_deferred_alloc(struct
i915_gem_context *ctx,
return ret;
}
-void intel_lr_context_resume(struct drm_i915_private *dev_priv)
+void intel_lr_context_resume(struct drm_i915_private *i915)
{
struct intel_engine_cs *engine;
struct i915_gem_context *ctx;
enum intel_engine_id id;
- /* Because we emit WA_TAIL_DWORDS there may be a disparity
+ /*
+* Because we emit WA_TAIL_DWORDS there may be a disparity
* between our bookkeeping in ce->ring->head and ce->ring->tail and
* that stored in context. As we only write new commands from
* ce->ring->tail onwards, everything before that is junk. If the GPU
@@ -2877,28 +2878,20 @@ void intel_lr_context_resume(struct drm_i915_private
*dev_priv)
* So to avoid that we reset the context images upon resume. For
* simplicity, we just zero everything out.
*/
- list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
- for_each_engine(engine, dev_priv, id) {
+ list_for_each_entry(ctx, &i915->contexts.list, link) {
+ for_each_engine(engine, i915, id) {
struct intel_context *ce =
to_intel_context(ctx, engine);
- u32 *reg;
-
- if (!ce->state)
- continue;
- reg = i915_gem_object_pin_map(ce->state->obj,
- I915_MAP_WB);
- if (WARN_ON(IS_ERR(reg)))
+ if (!ce->ring)
continue;
- reg += LRC_STATE_PN * PAGE_SIZE / sizeof(*reg);
- reg[CTX_RING_HEAD+1] = 0;
- reg[CTX_RING_TAIL+1] = 0;
-
- ce->state->obj->mm.dirty = true;
- i915_gem_object_unpin_map(ce->state->obj);
-
intel_ring_reset(ce->ring, 0);
+
+ if (ce->pin_count) { /* otherwise done in context_pin */
+ ce->lrc_reg_state[CTX_RING_HEAD+1] = 0;
+ ce->lrc_reg_state[CTX_RING_TAIL+1] = 0;
+ }
}
}
}
--
2.19.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/csr: keep max firmware size together with firmare name and version
Move max firmware size to the same if ladder with firmware name and
required version. This allows us to detect the missing max size for a
platform without actually loading the firmware, and makes the whole
thing easier to maintain.
We need to move the power get earlier to allow for early return in the
missing platform case. We also need to move the module parameter
override later to reuse the max firmware size, which is independent of
the override. Note how this works with gen 11+ which don't have a
specified firmware blob yet, but do have a maximum size.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_csr.c | 41 +---
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 368066010f94..62444f4c3c8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -465,8 +465,9 @@ struct intel_csr {
struct work_struct work;
const char *fw_path;
uint32_t required_version;
+ uint32_t max_fw_size; /* bytes */
uint32_t *dmc_payload;
- uint32_t dmc_fw_size;
+ uint32_t dmc_fw_size; /* dwords */
uint32_t version;
uint32_t mmio_count;
i915_reg_t mmioaddr[8];
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9a60bb9cc443..956ac8bbf5e4 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -281,7 +281,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
struct intel_csr *csr = &dev_priv->csr;
const struct stepping_info *si = intel_get_stepping_info(dev_priv);
uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
- uint32_t max_fw_size = 0;
uint32_t i;
uint32_t *dmc_payload;
@@ -378,15 +377,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
nbytes = dmc_header->fw_size * 4;
- if (INTEL_GEN(dev_priv) >= 11)
- max_fw_size = ICL_CSR_MAX_FW_SIZE;
- else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
- max_fw_size = GLK_CSR_MAX_FW_SIZE;
- else if (IS_GEN9(dev_priv))
- max_fw_size = BXT_CSR_MAX_FW_SIZE;
- else
- MISSING_CASE(INTEL_REVID(dev_priv));
- if (nbytes > max_fw_size) {
+ if (nbytes > csr->max_fw_size) {
DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
return NULL;
}
@@ -451,32 +442,44 @@ void intel_csr_ucode_init(struct drm_i915_private
*dev_priv)
if (!HAS_CSR(dev_priv))
return;
- if (i915_modparams.dmc_firmware_path) {
- csr->fw_path = i915_modparams.dmc_firmware_path;
- /* Bypass version check for firmware override. */
- csr->required_version = 0;
+ /*
+* Obtain a runtime pm reference, until CSR is loaded,
+* to avoid entering runtime-suspend.
+*/
+ intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+
+ if (INTEL_GEN(dev_priv) >= 11) {
+ csr->max_fw_size = ICL_CSR_MAX_FW_SIZE;
} else if (IS_CANNONLAKE(dev_priv)) {
csr->fw_path = I915_CSR_CNL;
csr->required_version = CNL_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
} else if (IS_GEMINILAKE(dev_priv)) {
csr->fw_path = I915_CSR_GLK;
csr->required_version = GLK_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
csr->fw_path = I915_CSR_KBL;
csr->required_version = KBL_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
} else if (IS_SKYLAKE(dev_priv)) {
csr->fw_path = I915_CSR_SKL;
csr->required_version = SKL_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
} else if (IS_BROXTON(dev_priv)) {
csr->fw_path = I915_CSR_BXT;
csr->required_version = BXT_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
+ } else {
+ MISSING_CASE(INTEL_REVID(dev_priv));
+ return;
}
- /*
-* Obtain a runtime pm reference, until CSR is loaded,
-* to avoid entering runtime-suspend.
-*/
- intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+ if (i915_modparams.dmc_firmware_path) {
+ csr->fw_path = i915_modparams.dmc_firmware_path;
+ /* Bypass version check for firmware override. */
+ csr->required_version = 0;
+ }
if (csr->fw_path == NULL) {
DRM_DEBUG_
[Intel-gfx] [PATCH 3/3] drm/i915/csr: bypass firmware request on i915.dmc_firmware_path=""
With i915.dmc_firmware_path="" it's obvious the intention is to disable
CSR firmware loading. Bypass the firmware request altogether in this
case, with more obvious debug logging.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_csr.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 956ac8bbf5e4..dbaeffddb5e7 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -476,6 +476,12 @@ void intel_csr_ucode_init(struct drm_i915_private
*dev_priv)
}
if (i915_modparams.dmc_firmware_path) {
+ if (strlen(i915_modparams.dmc_firmware_path) == 0) {
+ csr->fw_path = NULL;
+ DRM_DEBUG_KMS("Disabling CSR firmare and runtime PM\n");
+ return;
+ }
+
csr->fw_path = i915_modparams.dmc_firmware_path;
/* Bypass version check for firmware override. */
csr->required_version = 0;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/csr: keep firmware name and required version together
Having two separate if ladders gets increasingly hard to maintain. Put
them together.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_csr.c | 54
2 files changed, 23 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 767615ecdea5..368066010f94 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -464,6 +464,7 @@ struct drm_i915_display_funcs {
struct intel_csr {
struct work_struct work;
const char *fw_path;
+ uint32_t required_version;
uint32_t *dmc_payload;
uint32_t dmc_fw_size;
uint32_t version;
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 14cf4c367e36..9a60bb9cc443 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -284,7 +284,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
uint32_t max_fw_size = 0;
uint32_t i;
uint32_t *dmc_payload;
- uint32_t required_version;
if (!fw)
return NULL;
@@ -299,36 +298,19 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
return NULL;
}
- csr->version = css_header->version;
-
- if (csr->fw_path == i915_modparams.dmc_firmware_path) {
- /* Bypass version check for firmware override. */
- required_version = csr->version;
- } else if (IS_CANNONLAKE(dev_priv)) {
- required_version = CNL_CSR_VERSION_REQUIRED;
- } else if (IS_GEMINILAKE(dev_priv)) {
- required_version = GLK_CSR_VERSION_REQUIRED;
- } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
- required_version = KBL_CSR_VERSION_REQUIRED;
- } else if (IS_SKYLAKE(dev_priv)) {
- required_version = SKL_CSR_VERSION_REQUIRED;
- } else if (IS_BROXTON(dev_priv)) {
- required_version = BXT_CSR_VERSION_REQUIRED;
- } else {
- MISSING_CASE(INTEL_REVID(dev_priv));
- required_version = 0;
- }
-
- if (csr->version != required_version) {
+ if (csr->required_version &&
+ css_header->version != csr->required_version) {
DRM_INFO("Refusing to load DMC firmware v%u.%u,"
" please use v%u.%u\n",
-CSR_VERSION_MAJOR(csr->version),
-CSR_VERSION_MINOR(csr->version),
-CSR_VERSION_MAJOR(required_version),
-CSR_VERSION_MINOR(required_version));
+CSR_VERSION_MAJOR(css_header->version),
+CSR_VERSION_MINOR(css_header->version),
+CSR_VERSION_MAJOR(csr->required_version),
+CSR_VERSION_MINOR(csr->required_version));
return NULL;
}
+ csr->version = css_header->version;
+
readcount += sizeof(struct intel_css_header);
/* Extract Package Header information*/
@@ -469,18 +451,26 @@ void intel_csr_ucode_init(struct drm_i915_private
*dev_priv)
if (!HAS_CSR(dev_priv))
return;
- if (i915_modparams.dmc_firmware_path)
+ if (i915_modparams.dmc_firmware_path) {
csr->fw_path = i915_modparams.dmc_firmware_path;
- else if (IS_CANNONLAKE(dev_priv))
+ /* Bypass version check for firmware override. */
+ csr->required_version = 0;
+ } else if (IS_CANNONLAKE(dev_priv)) {
csr->fw_path = I915_CSR_CNL;
- else if (IS_GEMINILAKE(dev_priv))
+ csr->required_version = CNL_CSR_VERSION_REQUIRED;
+ } else if (IS_GEMINILAKE(dev_priv)) {
csr->fw_path = I915_CSR_GLK;
- else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
+ csr->required_version = GLK_CSR_VERSION_REQUIRED;
+ } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
csr->fw_path = I915_CSR_KBL;
- else if (IS_SKYLAKE(dev_priv))
+ csr->required_version = KBL_CSR_VERSION_REQUIRED;
+ } else if (IS_SKYLAKE(dev_priv)) {
csr->fw_path = I915_CSR_SKL;
- else if (IS_BROXTON(dev_priv))
+ csr->required_version = SKL_CSR_VERSION_REQUIRED;
+ } else if (IS_BROXTON(dev_priv)) {
csr->fw_path = I915_CSR_BXT;
+ csr->required_version = BXT_CSR_VERSION_REQUIRED;
+ }
/*
* Obtain a runtime pm reference, until CSR is loaded,
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/21] drm/i915/guc: Don't allow GuC submission on pre-Gen11
Quoting Michal Wajdeczko (2018-08-29 22:10:36) > Upcoming Gen11 GuC firmware requires new interface that is incompatible > with existing pre-Gen11 firmwares. Updated firmwares for pre-Gen11 will > arrive later. In the meantime sanitize the enable_guc option so that we > can enable HuC authentication but nothing else on pre-Gen11. Do you have a plan to source a GuC ICL machine to CI to regain the lost coverage? That'd be the minimum requirement. This is because I'm not really sold for replacing the existing interface in upstream (for which we have some machines in CI) with something we don't currently have any coverage for. Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/21] drm/i915/guc: Don't allow GuC submission on pre-Gen11
Quoting Michal Wajdeczko (2018-08-29 22:10:36)
> Upcoming Gen11 GuC firmware requires new interface that is incompatible
> with existing pre-Gen11 firmwares. Updated firmwares for pre-Gen11 will
> arrive later. In the meantime sanitize the enable_guc option so that we
> can enable HuC authentication but nothing else on pre-Gen11.
>
> Signed-off-by: Michal Wajdeczko
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Daniele Ceraolo Spurio
> Cc: Michel Thierry
> Cc: John Spotswood
> Cc: Vinay Belgaumkar
> Cc: Tony Ye
> Cc: Anusha Srivatsa
> Cc: Jeff Mcgee
> Cc: Antonio Argenziano
> Cc: Sujaritha Sundaresan
> @@ -292,6 +301,12 @@ int intel_uc_init(struct drm_i915_private *i915)
> return ret;
>
> if (USES_GUC_SUBMISSION(i915)) {
> +
Extra newline.
Regards, Joonas
> + if (INTEL_GEN(i915) < 11) {
> + intel_guc_fini(guc);
> + return -EIO;
> + }
> +
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
== Series Details ==
Series: series starting with [1/7] drm/i915: Missed interrupt simulation is no
more, tell the world
URL : https://patchwork.freedesktop.org/series/49255/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ac19c8f1d890 drm/i915: Missed interrupt simulation is no more, tell the world
7e6243ec098e drm/i915/execlists: Reset CSB pointers on canceling requests
(wedging)
08bf3205457a drm/i915/execlists: Avoid kicking priority on the current context
-:49: ERROR:SPACING: space prohibited after that open parenthesis '('
#49: FILE: drivers/gpu/drm/i915/intel_guc_submission.c:774:
+ if ( __guc_dequeue(engine))
total: 1 errors, 0 warnings, 0 checks, 103 lines checked
55badd0ca312 drm/i915/selftests: Basic stress test for rapid context switching
53a11c83e442 drm/i915/execlists: Delay updating ring register state after resume
-:68: CHECK:SPACING: spaces preferred around that '+' (ctx:VxV)
#68: FILE: drivers/gpu/drm/i915/intel_lrc.c:2892:
+ ce->lrc_reg_state[CTX_RING_HEAD+1] = 0;
^
-:69: CHECK:SPACING: spaces preferred around that '+' (ctx:VxV)
#69: FILE: drivers/gpu/drm/i915/intel_lrc.c:2893:
+ ce->lrc_reg_state[CTX_RING_TAIL+1] = 0;
^
total: 0 errors, 0 warnings, 2 checks, 52 lines checked
b6970b210332 drm/i915/execlists: Use coherent writes into the context image
-:56: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#56: FILE: drivers/gpu/drm/i915/i915_perf.c:1851:
+ regs = i915_gem_object_pin_map(ce->state->obj,
+ i915_coherent_map_type(dev_priv));
total: 0 errors, 0 warnings, 1 checks, 71 lines checked
b818668614c0 drm/i915/execlists: Onion unwind for logical_ring_init() failure
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/21] drm/i915/guc: Simplify preparation of GuC parameter block
Quoting Michal Wajdeczko (2018-08-29 22:10:37) > Definition of the parameters block passed to GuC is about to change. > Slightly refactor code now to make upcoming patch smaller. > > Signed-off-by: Michal Wajdeczko > Cc: Joonas Lahtinen > Cc: John Spotswood Reviewed-by: Joonas Lahtinen Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
== Series Details == Series: series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world URL : https://patchwork.freedesktop.org/series/49255/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915: Missed interrupt simulation is no more, tell the world Okay! Commit: drm/i915/execlists: Reset CSB pointers on canceling requests (wedging) Okay! Commit: drm/i915/execlists: Avoid kicking priority on the current context Okay! Commit: drm/i915/selftests: Basic stress test for rapid context switching +./include/linux/slab.h:631:13: error: undefined identifier '__builtin_mul_overflow' +./include/linux/slab.h:631:13: warning: call with no type! Commit: drm/i915/execlists: Delay updating ring register state after resume Okay! Commit: drm/i915/execlists: Use coherent writes into the context image -drivers/gpu/drm/i915/selftests/../i915_drv.h:3688:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3694:16: warning: expression using sizeof(void) Commit: drm/i915/execlists: Onion unwind for logical_ring_init() failure Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/21] drm/i915/guc: Support dual Gen9/Gen11 parameters block
Quoting Michal Wajdeczko (2018-08-29 22:10:38) > Gen11 GuC boot parameter definitions are different than previously > used for Gen9. Try to support both definitions until new firmwares > for pre-Gen11 will be available. This is exactly the kind of branching we want to avoid. Purpose of the GuC is to hide per-Gen differences, not to cause them :) The new interface code should just be GuC interface code (not refer to ICL or any generation specifically), then pre-Gen11 platforms just won't have a drm-tip compatible firmware until -- well -- they do have it. So for the series, just axe the old interface and replace it in-place. Makes the review of the interface differences much more effective, too. Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/5] lib/igt_vmwgfx: Add vmwgfx device
On Wed, Sep 05, 2018 at 05:03:46PM -0700, Deepak Rawat wrote:
> Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
>
> Signed-off-by: Deepak Rawat
> ---
> lib/drmtest.c | 9 -
> lib/drmtest.h | 3 +++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index bfa2e0f0..563d5b8b 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> return __is_device(fd, "i915");
> }
>
> +bool is_vmwgfx_device(int fd)
> +{
> + return __is_device(fd, "vmwg");
> +}
> +
> static bool has_known_intel_chipset(int fd)
> {
> struct drm_i915_getparam gp;
> @@ -205,7 +210,7 @@ static const struct module {
> { DRIVER_VC4, "vc4" },
> { DRIVER_VGEM, "vgem" },
> { DRIVER_VIRTIO, "virtio-gpu" },
> - { DRIVER_VIRTIO, "virtio_gpu" },
> + { DRIVER_VMWGFX, "vmwgfx" },
> {}
Don't remove the second virtio-gpu line, there's a reason for it.
we should probably add a comment there.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/5] lib/igt_fb: Call dumb_destroy ioctl in case of dumb buffers
Quoting Deepak Rawat (2018-09-06 01:03:47)
> vmwgfx does not support GEM interface so calling gem_close on vmwgfx
> results in error. Call dumb destroy IOCTL in case have dumb buffer.
>
> Signed-off-by: Deepak Rawat
> ---
> lib/igt_fb.c | 5 -
> lib/igt_kms.c | 15 +++
> lib/igt_kms.h | 1 +
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ae71d967..ba995a1a 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1920,7 +1920,10 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
>
> cairo_surface_destroy(fb->cairo_surface);
> do_or_die(drmModeRmFB(fd, fb->fb_id));
> - gem_close(fd, fb->gem_handle);
> + if (fb->is_dumb)
> + kmstest_dumb_destroy(fd, fb->gem_handle);
> + else
> + gem_close(fd, fb->gem_handle);
> fb->fb_id = 0;
> }
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 62d84684..9e9414cf 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -639,6 +639,21 @@ void *kmstest_dumb_map_buffer(int fd, uint32_t handle,
> uint64_t size,
> return ptr;
> }
>
> +/**
> + * kmstest_dumb_destroy:
> + * @fd: Opened drm file descriptor
> + * @handle: Offset in the file referred to by fd
> + */
> +void kmstest_dumb_destroy(int fd, uint32_t handle)
> +{
> + struct drm_mode_destroy_dumb arg = {};
> +
> + igt_assert_neq(handle, 0);
Don't bother doing the kernel's job for it. Abusing the ioctl iface is
the name of the game.
> + arg.handle = handle;
> + do_ioctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &arg);
struct drm_mode_destroy_dumb arg = { handle };
Try never to use do_ioctl() if you ever want to debug an error.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
== Series Details == Series: series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world URL : https://patchwork.freedesktop.org/series/49255/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4778 -> Patchwork_10106 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10106 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10106, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49255/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10106: === IGT changes === Possible regressions igt@drv_selftest@live_hangcheck: fi-cfl-s3: PASS -> DMESG-FAIL == Known issues == Here are the changes found in Patchwork_10106 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: PASS -> DMESG-WARN (fdo#107139, fdo#105128) igt@kms_chamelium@dp-edid-read: fi-kbl-7500u: PASS -> FAIL (fdo#103841) igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: 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) igt@kms_psr@primary_page_flip: fi-icl-u: NOTRUN -> FAIL (fdo#107383) +3 igt@pm_backlight@basic-brightness: fi-glk-dsi: PASS -> INCOMPLETE (k.org#198133, fdo#103359) igt@prime_vgem@basic-fence-flip: fi-ilk-650: PASS -> FAIL (fdo#104008) Possible fixes igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-byt-clapper: FAIL (fdo#107362, fdo#103191) -> PASS +1 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#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 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_4778 -> Patchwork_10106 CI_DRM_4778: 456cfc52e9f12423c6e597f677d8acb05851c3e3 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10106: b818668614c06148e9f7d8bbf2d894679200589b @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == b818668614c0 drm/i915/execlists: Onion unwind for logical_ring_init() failure b6970b210332 drm/i915/execlists: Use coherent writes into the context image 53a11c83e442 drm/i915/execlists: Delay updating ring register state after resume 55badd0ca312 drm/i915/selftests: Basic stress test for rapid context switching 08bf3205457a drm/i915/execlists: Avoid kicking priority on the current context 7e6243ec098e drm/i915/execlists: Reset CSB pointers on canceling requests (wedging) ac19c8f1d890 drm/i915: Missed interrupt simulation is no more, tell the world == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10106/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/5] tests/kms: Don't check crtc state for vmwgfx legacy set_crtc
Quoting Deepak Rawat (2018-09-06 01:03:48) > For a Xorg bug vmwgfx has a kernel workaround which reset the value of > mode::type. This will cause crtc state not to match what is expected. Seems suspect, I think we need a stronger reason to accept iface irregularities. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface
Quoting Michal Wajdeczko (2018-08-29 22:10:40) > Until now the GuC and HW engine class has been the same, which allowed > us to use them interchangeable. But it is better to start doing the > right thing and use the GuC definitions for the firmware interface. > > We also keep the same class id in the ctx descriptor to be able to have > the same values in the driver and firmware logs. > > Signed-off-by: Michel Thierry > Signed-off-by: Rodrigo Vivi > Signed-off-by: Michal Wajdeczko > Cc: Daniele Ceraolo Spurio > Cc: Michel Thierry > Cc: Lucas De Marchi > Cc: Tomasz Lis > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -249,7 +249,15 @@ static inline bool need_preempt(const struct > intel_engine_cs *engine, > > /* TODO: decide what to do with SW counter (bits 55-60) */ > > - desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT; > + /* > +* Although GuC will never see this upper part as it fills > +* its own descriptor, using the guc_class here will help keep > +* the i915 and firmware logs in sync. > +*/ > + if (HAS_GUC_SCHED(ctx->i915)) > + desc |= (u64)engine->guc_class << > GEN11_ENGINE_CLASS_SHIFT; > + else > + desc |= (u64)engine->class << > GEN11_ENGINE_CLASS_SHIFT; > /* bits 61-63 > */ I'm fairly confident I've given this review comment long ago, but here goes again. The new member name should just be hw_class or something else agnostic, and the branching of using ELSP vs. GuC identifier should happen at the engine init time (or at another sweet spot). And then the actual write should be unconditionally with the hw_class value. We should not be adding GuC specifics to intel_lrc.c, I think. Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/csr: keep firmware name and required version together
== Series Details == Series: series starting with [1/3] drm/i915/csr: keep firmware name and required version together URL : https://patchwork.freedesktop.org/series/49256/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/csr: keep firmware name and required version together -drivers/gpu/drm/i915/selftests/../i915_drv.h:3688:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3689:16: warning: expression using sizeof(void) Commit: drm/i915/csr: keep max firmware size together with firmare name and version -drivers/gpu/drm/i915/selftests/../i915_drv.h:3689:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3690:16: warning: expression using sizeof(void) Commit: drm/i915/csr: bypass firmware request on i915.dmc_firmware_path="" Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/5] lib/igt_fb: Check for stride before creating cairo surface
Quoting Deepak Rawat (2018-09-06 01:03:49) > Cairo surface creation will fail if stride of provided buffer is not > same as expected by cairo. This fails for vmwgfx odd length framebuffer > as in vmwgfx stride is always width * bpp. > > Signed-off-by: Deepak Rawat > --- > lib/igt_fb.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c > index ba995a1a..2724e323 100644 > --- a/lib/igt_fb.c > +++ b/lib/igt_fb.c > @@ -1349,6 +1349,9 @@ static void create_cairo_surface__gtt(int fd, struct > igt_fb *fb) > ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size, > PROT_READ | PROT_WRITE); > > + igt_require(fb->stride == cairo_format_stride_for_width( > + drm_format_to_cairo(fb->drm_format), fb->width)); > + > fb->cairo_surface = > cairo_image_surface_create_for_data(ptr, > > drm_format_to_cairo(fb->drm_format), Is there not a igt_require_f/igt_assert_f(cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS, "Unable to create a cairo surface: %s", cairo_status_to_string(cairo_surface_status(fb->cairo_surface))); here? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/csr: keep firmware name and required version together
Quoting Jani Nikula (2018-09-06 09:21:24)
> Having two separate if ladders gets increasingly hard to maintain. Put
> them together.
Does it even have to be an if-ladder? Something like
struct platform_requirements {
unsigned long platform_mask;
u32 required_version;
const char *path;
} [];
How does that work by patch 3?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/csr: bypass firmware request on i915.dmc_firmware_path=""
Quoting Jani Nikula (2018-09-06 09:21:26)
> With i915.dmc_firmware_path="" it's obvious the intention is to disable
> CSR firmware loading. Bypass the firmware request altogether in this
> case, with more obvious debug logging.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_csr.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c
> b/drivers/gpu/drm/i915/intel_csr.c
> index 956ac8bbf5e4..dbaeffddb5e7 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -476,6 +476,12 @@ void intel_csr_ucode_init(struct drm_i915_private
> *dev_priv)
> }
>
> if (i915_modparams.dmc_firmware_path) {
> + if (strlen(i915_modparams.dmc_firmware_path) == 0) {
> + csr->fw_path = NULL;
> + DRM_DEBUG_KMS("Disabling CSR firmare and runtime
> PM\n");
In response to a user parameter, could even be DRM_INFO().
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/csr: keep firmware name and required version together
== Series Details == Series: series starting with [1/3] drm/i915/csr: keep firmware name and required version together URL : https://patchwork.freedesktop.org/series/49256/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4778 -> Patchwork_10107 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10107 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10107, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49256/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10107: === IGT changes === Possible regressions igt@kms_psr@primary_page_flip: fi-kbl-r: PASS -> FAIL == Known issues == Here are the changes found in Patchwork_10107 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_hangcheck: fi-kbl-guc: PASS -> DMESG-FAIL (fdo#106560, fdo#107837) igt@gem_exec_suspend@basic-s3: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_psr@primary_page_flip: fi-icl-u: NOTRUN -> FAIL (fdo#107383) +3 igt@prime_vgem@basic-fence-flip: fi-ilk-650: PASS -> FAIL (fdo#104008) Possible fixes igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-byt-clapper: FAIL (fdo#107362, fdo#103191) -> PASS +1 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560 fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107837 https://bugs.freedesktop.org/show_bug.cgi?id=107837 == 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_4778 -> Patchwork_10107 CI_DRM_4778: 456cfc52e9f12423c6e597f677d8acb05851c3e3 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10107: 2475ce20b41d4d37edacba420d5ae1f475f37807 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2475ce20b41d drm/i915/csr: bypass firmware request on i915.dmc_firmware_path="" 6f9a15361f00 drm/i915/csr: keep max firmware size together with firmare name and version c232905796cd drm/i915/csr: keep firmware name and required version together == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10107/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/csr: keep firmware name and required version together
On Thu, 06 Sep 2018, Chris Wilson wrote:
> Quoting Jani Nikula (2018-09-06 09:21:24)
>> Having two separate if ladders gets increasingly hard to maintain. Put
>> them together.
>
> Does it even have to be an if-ladder? Something like
> struct platform_requirements {
> unsigned long platform_mask;
> u32 required_version;
> const char *path;
> } [];
>
> How does that work by patch 3?
I expect this to evolve to the same kind of thing as the PCH detection
and checks where you can't get away with a simple platform mask, and you
have to resort to an if ladder. Or you need separate match functions,
which get unwieldy without lambda functions.
Also, I think we may need to have several firmware blobs per platform
that get requested in sequence, to allow for more graceful updates. In
any case, I think grouping this stuff together is a good first step.
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] [igt-dev] [PATH i-g-t 2/2] tests: add slice power programming test
On 06/09/2018 08:00, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-09-05 15:25:44) From: Lionel Landwerlin Verifies that the kernel programs slices correctly based by reading the value of PWR_CLK_STATE register or MI_SET_PREDICATE on platforms before Cannonlake. v2: Add subslice tests (Lionel) Use MI_SET_PREDICATE for further verification when available (Lionel) v3: Rename to gem_ctx_rpcs (Lionel) v4: Update kernel API (Lionel) Add 0 value test (Lionel) Exercise invalid values (Lionel) v5: Add perf tests (Lionel) v6: Add new sysfs entry tests (Lionel) v7: Test rsvd fields Update for kernel series changes v8: Drop test_no_sseu_support() test (Kelvin) Drop drm_intel_*() apis (Chris) v9: by Chris: Drop all do_ioctl/do_ioctl_err() Use gem_context_[gs]et_param() Use gem_read() instead of mapping memory by Lionel: Test dynamic sseu on/off more Tvrtko Ursulin: v10: * Various style tweaks and refactorings. * New test coverage. I didn't notice any testing of: - param->size It exists in test_invalid_args. - feeding garbage into param->value user pointer (always cleared before use, perhaps just poison instead), along with abusive pointers. Also in test_invalid_args - but only the null pointer. I can add an unmapped or read-only one. E.g., how does the code fare if we pass in an unfaulted GGTT mmap? Would not fare well. :I It would be best to be able to reject them but how? We'll hit the same problem in future other patches so to support this, I think we need to refactor Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/csr: keep firmware name and required version together
Quoting Jani Nikula (2018-09-06 09:21:24)
> Having two separate if ladders gets increasingly hard to maintain. Put
> them together.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_csr.c | 54
>
> 2 files changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 767615ecdea5..368066010f94 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -464,6 +464,7 @@ struct drm_i915_display_funcs {
> struct intel_csr {
> struct work_struct work;
> const char *fw_path;
> + uint32_t required_version;
> uint32_t *dmc_payload;
> uint32_t dmc_fw_size;
> uint32_t version;
> diff --git a/drivers/gpu/drm/i915/intel_csr.c
> b/drivers/gpu/drm/i915/intel_csr.c
> index 14cf4c367e36..9a60bb9cc443 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -284,7 +284,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
> *dev_priv,
> uint32_t max_fw_size = 0;
> uint32_t i;
> uint32_t *dmc_payload;
> - uint32_t required_version;
>
> if (!fw)
> return NULL;
> @@ -299,36 +298,19 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
> *dev_priv,
> return NULL;
> }
>
> - csr->version = css_header->version;
> -
> - if (csr->fw_path == i915_modparams.dmc_firmware_path) {
> - /* Bypass version check for firmware override. */
> - required_version = csr->version;
> - } else if (IS_CANNONLAKE(dev_priv)) {
> - required_version = CNL_CSR_VERSION_REQUIRED;
> - } else if (IS_GEMINILAKE(dev_priv)) {
> - required_version = GLK_CSR_VERSION_REQUIRED;
> - } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> - required_version = KBL_CSR_VERSION_REQUIRED;
> - } else if (IS_SKYLAKE(dev_priv)) {
> - required_version = SKL_CSR_VERSION_REQUIRED;
> - } else if (IS_BROXTON(dev_priv)) {
> - required_version = BXT_CSR_VERSION_REQUIRED;
> - } else {
> - MISSING_CASE(INTEL_REVID(dev_priv));
> - required_version = 0;
> - }
> -
> - if (csr->version != required_version) {
> + if (csr->required_version &&
> + css_header->version != csr->required_version) {
> DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> " please use v%u.%u\n",
> -CSR_VERSION_MAJOR(csr->version),
> -CSR_VERSION_MINOR(csr->version),
> -CSR_VERSION_MAJOR(required_version),
> -CSR_VERSION_MINOR(required_version));
> +CSR_VERSION_MAJOR(css_header->version),
> +CSR_VERSION_MINOR(css_header->version),
> +CSR_VERSION_MAJOR(csr->required_version),
> +CSR_VERSION_MINOR(csr->required_version));
> return NULL;
> }
>
> + csr->version = css_header->version;
Ok, so far.
> +
> readcount += sizeof(struct intel_css_header);
>
> /* Extract Package Header information*/
> @@ -469,18 +451,26 @@ void intel_csr_ucode_init(struct drm_i915_private
> *dev_priv)
> if (!HAS_CSR(dev_priv))
> return;
>
> - if (i915_modparams.dmc_firmware_path)
> + if (i915_modparams.dmc_firmware_path) {
> csr->fw_path = i915_modparams.dmc_firmware_path;
> - else if (IS_CANNONLAKE(dev_priv))
> + /* Bypass version check for firmware override. */
> + csr->required_version = 0;
Ok.
> + } else if (IS_CANNONLAKE(dev_priv)) {
> csr->fw_path = I915_CSR_CNL;
> - else if (IS_GEMINILAKE(dev_priv))
> + csr->required_version = CNL_CSR_VERSION_REQUIRED;
Ok.
> + } else if (IS_GEMINILAKE(dev_priv)) {
> csr->fw_path = I915_CSR_GLK;
> - else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> + csr->required_version = GLK_CSR_VERSION_REQUIRED;
Ok.
> + } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> csr->fw_path = I915_CSR_KBL;
> - else if (IS_SKYLAKE(dev_priv))
> + csr->required_version = KBL_CSR_VERSION_REQUIRED;
Ok.
> + } else if (IS_SKYLAKE(dev_priv)) {
> csr->fw_path = I915_CSR_SKL;
> - else if (IS_BROXTON(dev_priv))
> + csr->required_version = SKL_CSR_VERSION_REQUIRED;
Ok.
> + } else if (IS_BROXTON(dev_priv)) {
> csr->fw_path = I915_CSR_BXT;
> + csr->required_version = BXT_CSR_VERSION_REQUIRED;
Ok.
Reviewed-by: Chris Wilson
-Chris
_
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Record the sseu configuration per-context & engine
On 05/09/2018 16:18, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-09-05 15:22:18)
From: Chris Wilson
We want to expose the ability to reconfigure the slices, subslice and
eu per context and per engine. To facilitate that, store the current
configuration on the context for each engine, which is initially set
to the device default upon creation.
v2: record sseu configuration per context & engine (Chris)
v3: introduce the i915_gem_context_sseu to store powergating
programming, sseu_dev_info has grown quite a bit (Lionel)
v4: rename i915_gem_sseu into intel_sseu (Chris)
use to_intel_context() (Chris)
v5: More to_intel_context() (Tvrtko)
Switch intel_sseu from union to struct (Tvrtko)
Move context default sseu in existing loop (Chris)
v6: s/intel_sseu_from_device_sseu/intel_device_default_sseu/ (Tvrtko)
Tvrtko Ursulin:
v7:
* Pass intel_sseu by pointer instead of value to make_rpcs.
* Rebase for make_rpcs changes.
v8:
* Rebase for RPCS edit on pin.
v9:
* Rebase for context image setup changes.
Signed-off-by: Chris Wilson
Signed-off-by: Lionel Landwerlin
Signed-off-by: Tvrtko Ursulin
I feel this is substantially different (since I just outlined a v1!) to
merit a
Reviewed-by: Chris Wilson
and probably deserves a different author. I think Lionel is still the
principle author here, but Tvrtko has done a lot of refactoring and
integrating in the new scheme.
Agreed Lionel is the real author here - mine were just small tweaks.
-static u32 make_rpcs(struct drm_i915_private *dev_priv);
+static u32 make_rpcs(struct drm_i915_private *dev_priv,
+struct intel_sseu *ctx_sseu);
static struct intel_context *
__execlists_context_pin(struct intel_engine_cs *engine,
@@ -1349,7 +1350,7 @@ __execlists_context_pin(struct intel_engine_cs *engine,
/* RPCS */
if (engine->class == RENDER_CLASS) {
ce->lrc_reg_state[CTX_R_PWR_CLK_STATE + 1] =
- make_rpcs(engine->i915);
+ make_rpcs(engine->i915, &ce->sseu);
We have different habits here; my vim config just gives this a single
tab indent beyond the incomplete line. (Was going to say it earlier ;)
Not sure if my approach here is always consistent, but I *think* I first
try to indent it to where it "looks good". If neither indentation looks
decidedly better, then I push it so end aligns with the wrap marker. I
in this particular case I wasn't too happy with any of the options. :(
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/csr: keep max firmware size together with firmare name and version
Quoting Jani Nikula (2018-09-06 09:21:25)
> Move max firmware size to the same if ladder with firmware name and
> required version. This allows us to detect the missing max size for a
> platform without actually loading the firmware, and makes the whole
> thing easier to maintain.
>
> We need to move the power get earlier to allow for early return in the
> missing platform case. We also need to move the module parameter
> override later to reuse the max firmware size, which is independent of
> the override. Note how this works with gen 11+ which don't have a
> specified firmware blob yet, but do have a maximum size.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> drivers/gpu/drm/i915/intel_csr.c | 41
> +---
> 2 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 368066010f94..62444f4c3c8e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -465,8 +465,9 @@ struct intel_csr {
> struct work_struct work;
> const char *fw_path;
> uint32_t required_version;
> + uint32_t max_fw_size; /* bytes */
> uint32_t *dmc_payload;
> - uint32_t dmc_fw_size;
> + uint32_t dmc_fw_size; /* dwords */
Appreciated.
> uint32_t version;
> uint32_t mmio_count;
> i915_reg_t mmioaddr[8];
> diff --git a/drivers/gpu/drm/i915/intel_csr.c
> b/drivers/gpu/drm/i915/intel_csr.c
> index 9a60bb9cc443..956ac8bbf5e4 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -281,7 +281,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
> *dev_priv,
> struct intel_csr *csr = &dev_priv->csr;
> const struct stepping_info *si = intel_get_stepping_info(dev_priv);
> uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
> - uint32_t max_fw_size = 0;
> uint32_t i;
> uint32_t *dmc_payload;
>
> @@ -378,15 +377,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
> *dev_priv,
>
> /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
> nbytes = dmc_header->fw_size * 4;
> - if (INTEL_GEN(dev_priv) >= 11)
> - max_fw_size = ICL_CSR_MAX_FW_SIZE;
> - else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> - max_fw_size = GLK_CSR_MAX_FW_SIZE;
> - else if (IS_GEN9(dev_priv))
> - max_fw_size = BXT_CSR_MAX_FW_SIZE;
> - else
> - MISSING_CASE(INTEL_REVID(dev_priv));
> - if (nbytes > max_fw_size) {
> + if (nbytes > csr->max_fw_size) {
Ok.
> DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
> return NULL;
> }
> @@ -451,32 +442,44 @@ void intel_csr_ucode_init(struct drm_i915_private
> *dev_priv)
> if (!HAS_CSR(dev_priv))
> return;
>
> - if (i915_modparams.dmc_firmware_path) {
> - csr->fw_path = i915_modparams.dmc_firmware_path;
> - /* Bypass version check for firmware override. */
> - csr->required_version = 0;
> + /*
> +* Obtain a runtime pm reference, until CSR is loaded,
> +* to avoid entering runtime-suspend.
* On error, we return with the rpm wakeref held to prevent
* runtime suspend as runtime suspend *requires* a working
* CSR for whatever reason.
> +*/
> + intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
A reminder that the leak is by design. Bonus points for integrating with
the wakeref tracking so that we demonstrate we aren't just leaking for
the fun of it.
> + if (INTEL_GEN(dev_priv) >= 11) {
> + csr->max_fw_size = ICL_CSR_MAX_FW_SIZE;
Ok.
> } else if (IS_CANNONLAKE(dev_priv)) {
> csr->fw_path = I915_CSR_CNL;
> csr->required_version = CNL_CSR_VERSION_REQUIRED;
> + csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
Ok.
> } else if (IS_GEMINILAKE(dev_priv)) {
> csr->fw_path = I915_CSR_GLK;
> csr->required_version = GLK_CSR_VERSION_REQUIRED;
> + csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
Ok.
> } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> csr->fw_path = I915_CSR_KBL;
> csr->required_version = KBL_CSR_VERSION_REQUIRED;
> + csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
Ok.
> } else if (IS_SKYLAKE(dev_priv)) {
> csr->fw_path = I915_CSR_SKL;
> csr->required_version = SKL_CSR_VERSION_REQUIRED;
> + csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
Ok, but interesting naming scheme.
> } else if (IS_BROXTON(dev_priv)) {
> csr->fw_path = I915_CSR_BXT;
> csr->required_version = BXT_CSR_VERSION_REQU
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/icl: Update result lines in correspondence with result blocks
On 9/6/2018 5:12 AM, Rodrigo Vivi wrote:
The subject here is marked as icl, but the code seems to all platforms
what am I missing?
Sorry, my mistake. Will update it accordingly.
But also I didn't check spec yet on this particular case
The spec only mentions that the result lines should be greater than the
level 0 result lines.
But this change is in line with the code comment for result blocks, to
make sure that the result lines calculated is in correspondence with the
result blocks.
On Wed, Sep 05, 2018 at 02:32:39PM +0530, Karthik B S wrote:
As the result blocks for WM1-WM7 are always kept higher than the
level below the present level, make sure result lines are also higher
than the level below for WM1-WM7.
Signed-off-by: Karthik B S
---
drivers/gpu/drm/i915/intel_pm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b5db6a3..cc41009 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4704,6 +4704,8 @@ static int skl_compute_plane_wm(const struct
drm_i915_private *dev_priv,
if (level >= 1 && level <= 7) {
if (result_prev->plane_res_b > res_blocks)
res_blocks = result_prev->plane_res_b;
+ if (result_prev->plane_res_l > res_lines)
+ res_lines = result_prev->plane_res_l;
}
if (INTEL_GEN(dev_priv) >= 11) {
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/csr: bypass firmware request on i915.dmc_firmware_path=""
Quoting Chris Wilson (2018-09-06 10:08:37)
> Quoting Jani Nikula (2018-09-06 09:21:26)
> > With i915.dmc_firmware_path="" it's obvious the intention is to disable
> > CSR firmware loading. Bypass the firmware request altogether in this
> > case, with more obvious debug logging.
> >
> > Signed-off-by: Jani Nikula
> > ---
> > drivers/gpu/drm/i915/intel_csr.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > b/drivers/gpu/drm/i915/intel_csr.c
> > index 956ac8bbf5e4..dbaeffddb5e7 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -476,6 +476,12 @@ void intel_csr_ucode_init(struct drm_i915_private
> > *dev_priv)
> > }
> >
> > if (i915_modparams.dmc_firmware_path) {
> > + if (strlen(i915_modparams.dmc_firmware_path) == 0) {
> > + csr->fw_path = NULL;
> > + DRM_DEBUG_KMS("Disabling CSR firmare and runtime
> > PM\n");
>
> In response to a user parameter, could even be DRM_INFO().
Either way (but my preference here is for DRM_INFO),
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
On 05/09/2018 16:21, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-09-05 15:22:19)
-static u32 make_rpcs(struct drm_i915_private *dev_priv,
-struct intel_sseu *ctx_sseu)
+u32 gen8_make_rpcs(struct drm_i915_private *dev_priv,
+ struct intel_sseu *req_sseu)
Should we retrospectively make this const?
Can do, but generally I try to avoid it kernel code since most of the
time it is way more pain than benefit.
(And anychance for a s/dev_priv/i915?)
Will check if it is doable without much noise at any of the stages.
{
const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
bool subslice_pg = sseu->has_subslice_pg;
- u8 slices = hweight8(ctx_sseu->slice_mask);
- u8 subslices = hweight8(ctx_sseu->subslice_mask);
+ struct intel_sseu ctx_sseu;
+ u8 slices, subslices;
u32 rpcs = 0;
+ /*
+* If i915/perf is active, we want a stable powergating configuration
+* on the system. The most natural configuration to take in that case
+* is the default (i.e maximum the hardware can do).
+*/
+ if (unlikely(dev_priv->perf.oa.exclusive_stream))
+ ctx_sseu = intel_device_default_sseu(dev_priv);
+ else
+ ctx_sseu = *req_sseu;
:(
I'm not sure if I can suggest anything better, but this does feel like a
layering violation.
It makes sense which makes it only feel worse.
It used to be a helper which applied the adjustment but I wasn't happy
with how callers then had to know to call the helper and decided
handling it at the core is better in more than one way.
I think bottom line is there is fundamental interaction between the two
so some layering violation has to happen.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/atomic: trim driver interface/docs
Am Mittwoch, 5. September 2018, 15:57:09 CEST schrieb Daniel Vetter: > Remove the kerneldoc and EXPORT_SYMBOL which aren't used and really > shouldn't ever be used by drivers directly. > > Unfortunately this means we need to move the set_writeback_fb function > around to avoid a forward decl. > > Signed-off-by: Daniel Vetter > Cc: David Airlie > Cc: Gustavo Padovan > Cc: Maarten Lankhorst > Cc: Sean Paul checked that the two moved functions keep identical content from old to new location, so Acked-by: Heiko Stuebner ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATH i-g-t 2/2] tests: add slice power programming test
Quoting Tvrtko Ursulin (2018-09-06 10:31:21) > > On 06/09/2018 08:00, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-09-05 15:25:44) > >> From: Lionel Landwerlin > >> > >> Verifies that the kernel programs slices correctly based by reading > >> the value of PWR_CLK_STATE register or MI_SET_PREDICATE on platforms > >> before Cannonlake. > >> > >> v2: Add subslice tests (Lionel) > >> Use MI_SET_PREDICATE for further verification when available (Lionel) > >> > >> v3: Rename to gem_ctx_rpcs (Lionel) > >> > >> v4: Update kernel API (Lionel) > >> Add 0 value test (Lionel) > >> Exercise invalid values (Lionel) > >> > >> v5: Add perf tests (Lionel) > >> > >> v6: Add new sysfs entry tests (Lionel) > >> > >> v7: Test rsvd fields > >> Update for kernel series changes > >> > >> v8: Drop test_no_sseu_support() test (Kelvin) > >> Drop drm_intel_*() apis (Chris) > >> > >> v9: by Chris: > >> Drop all do_ioctl/do_ioctl_err() > >> Use gem_context_[gs]et_param() > >> Use gem_read() instead of mapping memory > >> by Lionel: > >> Test dynamic sseu on/off more > >> > >> Tvrtko Ursulin: > >> > >> v10: > >> * Various style tweaks and refactorings. > >> * New test coverage. > > > > I didn't notice any testing of: > > - param->size > > It exists in test_invalid_args. > > > - feeding garbage into param->value user pointer (always cleared before > > use, perhaps just poison instead), along with abusive pointers. > > Also in test_invalid_args - but only the null pointer. I can add an > unmapped or read-only one. I think passing a pointer that starts valid and crosses into an unmapped page is essential. That makes sure we are using copy_from_user correctly. Another trivial one is param->value = -param->size + 1. As for the garbage, I wasn't sure (because I'm fully cogniscent of the sseu responses) if we would not interpret 0 as a valid response. If you are happy that we can differentiate an output 0 after passing in 0, that's all fine (as opposed to us forgetting to fill a value along some path). > > E.g., how does the code fare if we pass in an unfaulted GGTT mmap? > > Would not fare well. :I It would be best to be able to reject them but > how? We'll hit the same problem in future other patches so to support > this, I think we need to refactor Do you want my patch to drop struct_mutex entirely here :) So then you have to add it where you need it, which is after the copy_from_user() so recursive faults are not a problem. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Expose RPCS (SSEU) configuration to userspace
On 05/09/2018 16:29, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-09-05 15:22:21)
From: Chris Wilson
Now this looks nothing like my first suggestion!
I think Tvrtko should stand ad the author of the final mechanism, I
think it is substantially different from the submission method first
done by Lionel.
Okay I'll relieve you from authorship on this one. Not sure who between
Lionel and me with, but I'll think of something.
We want to allow userspace to reconfigure the subslice configuration for
its own use case. To do so, we expose a context parameter to allow
adjustment of the RPCS register stored within the context image (and
currently not accessible via LRI). If the context is adjusted before
first use, the adjustment is for "free"; otherwise if the context is
active we flush the context off the GPU (stalling all users) and forcing
the GPU to save the context to memory where we can modify it and so
ensure that the register is reloaded on next execution.
The overhead of managing additional EU subslices can be significant,
especially in multi-context workloads. Non-GPGPU contexts should
preferably disable the subslices it is not using, and others should
fine-tune the number to match their workload.
We expose complete control over the RPCS register, allowing
configuration of slice/subslice, via masks packed into a u64 for
simplicity. For example,
struct drm_i915_gem_context_param arg;
struct drm_i915_gem_context_param_sseu sseu = { .class = 0,
.instance = 0, };
memset(&arg, 0, sizeof(arg));
arg.ctx_id = ctx;
arg.param = I915_CONTEXT_PARAM_SSEU;
arg.value = (uintptr_t) &sseu;
if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &arg) == 0) {
sseu.packed.subslice_mask = 0;
drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, &arg);
}
could be used to disable all subslices where supported.
v2: Fix offset of CTX_R_PWR_CLK_STATE in intel_lr_context_set_sseu() (Lionel)
v3: Add ability to program this per engine (Chris)
v4: Move most get_sseu() into i915_gem_context.c (Lionel)
v5: Validate sseu configuration against the device's capabilities (Lionel)
v6: Change context powergating settings through MI_SDM on kernel context (Chris)
v7: Synchronize the requests following a powergating setting change using a
global
dependency (Chris)
Iterate timelines through dev_priv.gt.active_rings (Tvrtko)
Disable RPCS configuration setting for non capable users (Lionel/Tvrtko)
v8: s/union intel_sseu/struct intel_sseu/ (Lionel)
s/dev_priv/i915/ (Tvrtko)
Change uapi class/instance fields to u16 (Tvrtko)
Bump mask fields to 64bits (Lionel)
Don't return EPERM when dynamic sseu is disabled (Tvrtko)
v9: Import context image into kernel context's ppgtt only when
reconfiguring powergated slice/subslices (Chris)
Use aliasing ppgtt when needed (Michel)
Tvrtko Ursulin:
v10:
* Update for upstream changes.
* Request submit needs a RPM reference.
* Reject on !FULL_PPGTT for simplicity.
* Pull out get/set param to helpers for readability and less indent.
* Use i915_request_await_dma_fence in add_global_barrier to skip waits
on the same timeline and avoid GEM_BUG_ON.
* No need to explicitly assign a NULL pointer to engine in legacy mode.
* No need to move gen8_make_rpcs up.
* Factored out global barrier as prep patch.
* Allow to only CAP_SYS_ADMIN if !Gen11.
v11:
* Remove engine vfunc in favour of local helper. (Chris Wilson)
* Stop retiring requests before updates since it is not needed
(Chris Wilson)
* Implement direct CPU update path for idle contexts. (Chris Wilson)
* Left side dependency needs only be on the same context timeline.
(Chris Wilson)
* It is sufficient to order the timeline. (Chris Wilson)
* Reject !RCS configuration attempts with -ENODEV for now.
v12:
* Rebase for make_rpcs.
v13:
* Centralize SSEU normalization to make_rpcs.
* Type width checking (uAPI <-> implementation).
* Gen11 restrictions uAPI checks.
* Gen11 subslice count differences handling.
Chris Wilson:
* args->size handling fixes.
* Update context image from GGTT.
* Postpone context image update to pinning.
* Use i915_gem_active_raw instead of last_request_on_engine.
v14:
* Add activity tracker on intel_context to fix the lifetime issues
and simplify the code. (Chris Wilson)
v15:
* Fix context pin leak if no space in ring by simplifying the
context pinning sequence.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100899
Issue: https://github.com/intel/media-driver/issues/267
Signed-off-by: Chris Wilson
Signed-off-by: Lionel Landwerlin
Cc: Dmitry Rogozhkin
Cc: Tvrtko Ursulin
Cc: Zhipeng Gong
Cc: Joonas Lahtinen
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gem_context.c | 303 +++-
drivers/gpu/drm/i915/i9
Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake.
Quoting Rodrigo Vivi (2018-09-05 19:42:28)
> On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote:
> > Quoting Rodrigo Vivi (2018-09-04 08:27:14)
> > > On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote:
> > > > On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote:
> > > > > Add Support to load DMC on Icelake.
> > > > >
> > > > > While at it, also add support to load the firmware
> > > > > during system resume.
> > > > >
> > > > > v2: load firmware during system resume.(Imre)
> > > > >
> > > > > v3: enable has_csr for icelake.(Jyoti)
> > > > >
> > > > > v4: Only load the firmware in this patch
> > > > >
> > > > > Cc: Jyoti Yadav
> > > > > Cc: Imre Deak
> > > > > Cc: Rodrigo Vivi
> > > > > Cc: Paulo Zanoni
> > > > > Signed-off-by: Anusha Srivatsa
> > > >
> > > > Reviewed-by: Imre Deak
> > > >
> > > > Is it ok to push this already now that the ICL 1.07 firmware is in [1]
> > > > or do we have to wait until it propagates to [2]?
> > >
> > > The main motivation behind having drm-firmware is the unpredictability
> > > of linux-firmware.git pull requests acceptance. It may take 1 day or it
> > > may take 2 months.
> > >
> > > So on drm-firmware we at least have it public in a way OSVs
> > > could easisly backport. Although hopefully by the end of 4.20
> > > cycle I believe it will be there on linux-firmware.git already.
> > >
> > > So if fw is already on drm-firmware and passing all tests
> > > we should be able to push the patch to dinq.
> >
> > Was not the decision that we only gate the MODULE_FIRMWARE line until
> > the firmware is in linux-firmware.git?
> >
> > So it should be no harm to support loading firmwares that are available
> > from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which
> > would trigger false positives for distro packagers.
>
> As far as I can remember we had agreed on changing this and adding
> MODULE_FIRMWARE from the beginning.
Is there an e-mail about it? I must have missed such discussion.
> 1. the process gets complex
> 2. we forgot many times to add it afterwards
> 3. module_firmware changes nothing... only the fact that initrd generation
>wont complain if firmware is not there yet.
Yes, but isn't this the only thing we've got complaints of? At least
I've not noticed distros complaining about not having the MODULE_FIRMWARE
(which would omit the firmware from initrd).
So I'd rather stick to the process previously decided on. Assuming pull
request will be accepted in X days is always just an assumption.
Regards, Joonas
> 4. The old issue was that patches were merged without the pull request
>being sent... We fixed that by only accepting patches after pull request
>is sent.
> 5. By the time that all lands to distros linux-firmware.git will have
> the firmware because of 4.
> 6. We have now drm-firmware to mirror what official linux-firmware.git will
> be in few weeks from now.
>
> So I don't see a reason anymore why to keep with complicated process with
> split MODULE_FIRMWARE.
>
> Thanks,
> Rodrigo.
>
> >
> > Regards, Joonas
> >
> > >
> > > Thanks,
> > > Rodrigo.
> > >
> > > >
> > > > [1] https://cgit.freedesktop.org/drm/drm-firmware/
> > > > [2]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> > > >
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_csr.c| 7 +++
> > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > > > 2 files changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > > index 1ec4f09c61f6..6d9d47322405 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > > @@ -34,6 +34,9 @@
> > > > > * low-power state and comes back to normal.
> > > > > */
> > > > >
> > > > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin"
> > > > > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7)
> > > > > +
> > > > > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin"
> > > > > MODULE_FIRMWARE(I915_CSR_GLK);
> > > > > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4)
> > > > > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct
> > > > > drm_i915_private *dev_priv,
> > > > > if (csr->fw_path == i915_modparams.dmc_firmware_path) {
> > > > > /* Bypass version check for firmware override. */
> > > > > required_version = csr->version;
> > > > > + } else if (IS_ICELAKE(dev_priv)) {
> > > > > + required_version = ICL_CSR_VERSION_REQUIRED;
> > > > > } else if (IS_CANNONLAKE(dev_priv)) {
> > > > > required_version = CNL_CSR_VERSION_REQUIRED;
> > > > > } else if (IS_GEMINILAKE(dev_priv)) {
> > > > > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private
> > > > > *dev_priv)
> > > > >
> > > > > if (i915_modparams.dmc_firmware_path)
> > > > > csr->fw_path = i915_modparams.dmc_firmware_path;
> > > >
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Expose RPCS (SSEU) configuration to userspace
Quoting Tvrtko Ursulin (2018-09-06 10:50:32) > > On 05/09/2018 16:29, Chris Wilson wrote: > > I've a slight preference to setting to 0 then overwriting it afterwards. > > We can't use/validate it then. Alternative to just not clear it for ABI > where it is not used? In other words would go away from the case > branches completely. Does the ABI depend on it being zeroed on return > from get_param? It would be strange.. Bah, I was just looking at the patch hoping for the best (without thinking). I've some recollection of doing something similar and coming to the same conclusion. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
On 05/09/2018 15:22, Tvrtko Ursulin wrote:
From: Lionel Landwerlin
If some of the contexts submitting workloads to the GPU have been
configured to shutdown slices/subslices, we might loose the NOA
configurations written in the NOA muxes.
One possible solution to this problem is to reprogram the NOA muxes
when we switch to a new context. We initially tried this in the
workaround batchbuffer but some concerns where raised about the cost
of reprogramming at every context switch. This solution is also not
without consequences from the userspace point of view. Reprogramming
of the muxes can only happen once the powergating configuration has
changed (which happens after context switch). This means for a window
of time during the recording, counters recorded by the OA unit might
be invalid. This requires userspace dealing with OA reports to discard
the invalid values.
Minimizing the reprogramming could be implemented by tracking of the
last programmed configuration somewhere in GGTT and use MI_PREDICATE
to discard some of the programming commands, but the command streamer
would still have to parse all the MI_LRI instructions in the
workaround batchbuffer.
Another solution, which this change implements, is to simply disregard
the user requested configuration for the period of time when i915/perf
is active. There is no known issue with this apart from a performance
penality for some media workloads that benefit from running on a
partially powergated GPU. We already prevent RC6 from affecting the
programming so it doesn't sound completely unreasonable to hold on
powergating for the same reason.
v2: Leave RPCS programming in intel_lrc.c (Lionel)
v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel)
More to_intel_context() (Tvrtko)
s/dev_priv/i915/ (Tvrtko)
Tvrtko Ursulin:
v4:
* Rebase for make_rpcs changes.
v5:
* Apply OA restriction from make_rpcs directly.
v6:
* Rebase for context image setup changes.
Signed-off-by: Lionel Landwerlin
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_perf.c | 5 +
drivers/gpu/drm/i915/intel_lrc.c | 30 --
drivers/gpu/drm/i915/intel_lrc.h | 3 +++
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index ccb20230df2c..dd65b72bddd4 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct
i915_gem_context *ctx,
CTX_REG(reg_state, state_offset, flex_regs[i], value);
}
+
+ CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
+ gen8_make_rpcs(dev_priv,
+ &to_intel_context(ctx,
+dev_priv->engine[RCS])->sseu));
I think there is one issue I missed on the previous iterations of this
patch.
This gen8_update_reg_state_unlocked() is called when the GPU is parked
on the kernel context.
It's supposed to update all contexts, but I think we might not be able
to update the kernel context image while the GPU is using it.
Context save might happen after we edited the image and that would
override the values we just put in there.
The OA config is emitted through context image edition in this function
but also through the ring buffer in
gen8_switch_to_updated_kernel_context() for the kernel context.
Since we can't have a context modify its own RCPS value, we'll have to
resort to yet another context to do that for the kernel context.
I remember having a patch that created yet another kernel context (let's
call it rpcs edition context), which is used to reconfigure rpcs for
every context but itself and then have the kernel context reconfigure
this rpcs edition context.
Or alternatively not do anything to it, because it's only going to run
to edit other contexts at a time when we don't care about power
configuration stability.
-
Lionel
}
/*
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8a477e43dbca..9709c1fbe836 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1305,9 +1305,6 @@ static int __context_pin(struct i915_gem_context *ctx,
struct i915_vma *vma)
return i915_vma_pin(vma, 0, 0, flags);
}
-static u32 make_rpcs(struct drm_i915_private *dev_priv,
-struct intel_sseu *ctx_sseu);
-
static struct intel_context *
__execlists_context_pin(struct intel_engine_cs *engine,
struct i915_gem_context *ctx,
@@ -1350,7 +1347,7 @@ __execlists_context_pin(struct intel_engine_cs *engine,
/* RPCS */
if (engine->class == RENDER_CLASS) {
ce->lrc_reg_state[CTX_R_PWR_CLK_STATE + 1] =
- make_rpcs(engine->i915, &ce->sseu);
+ gen8_make_rpcs(engine->i91
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Expose RPCS (SSEU) configuration to userspace
On 06/09/2018 10:50, Tvrtko Ursulin wrote: On 05/09/2018 16:29, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-09-05 15:22:21) From: Chris Wilson Now this looks nothing like my first suggestion! I think Tvrtko should stand ad the author of the final mechanism, I think it is substantially different from the submission method first done by Lionel. Okay I'll relieve you from authorship on this one. Not sure who between Lionel and me with, but I'll think of something. Feel free to take over :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
Quoting Lionel Landwerlin (2018-09-06 10:57:47) > On 05/09/2018 15:22, Tvrtko Ursulin wrote: > > From: Lionel Landwerlin > > > > If some of the contexts submitting workloads to the GPU have been > > configured to shutdown slices/subslices, we might loose the NOA > > configurations written in the NOA muxes. > > > > One possible solution to this problem is to reprogram the NOA muxes > > when we switch to a new context. We initially tried this in the > > workaround batchbuffer but some concerns where raised about the cost > > of reprogramming at every context switch. This solution is also not > > without consequences from the userspace point of view. Reprogramming > > of the muxes can only happen once the powergating configuration has > > changed (which happens after context switch). This means for a window > > of time during the recording, counters recorded by the OA unit might > > be invalid. This requires userspace dealing with OA reports to discard > > the invalid values. > > > > Minimizing the reprogramming could be implemented by tracking of the > > last programmed configuration somewhere in GGTT and use MI_PREDICATE > > to discard some of the programming commands, but the command streamer > > would still have to parse all the MI_LRI instructions in the > > workaround batchbuffer. > > > > Another solution, which this change implements, is to simply disregard > > the user requested configuration for the period of time when i915/perf > > is active. There is no known issue with this apart from a performance > > penality for some media workloads that benefit from running on a > > partially powergated GPU. We already prevent RC6 from affecting the > > programming so it doesn't sound completely unreasonable to hold on > > powergating for the same reason. > > > > v2: Leave RPCS programming in intel_lrc.c (Lionel) > > > > v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel) > > More to_intel_context() (Tvrtko) > > s/dev_priv/i915/ (Tvrtko) > > > > Tvrtko Ursulin: > > > > v4: > > * Rebase for make_rpcs changes. > > > > v5: > > * Apply OA restriction from make_rpcs directly. > > > > v6: > > * Rebase for context image setup changes. > > > > Signed-off-by: Lionel Landwerlin > > Signed-off-by: Tvrtko Ursulin > > --- > > drivers/gpu/drm/i915/i915_perf.c | 5 + > > drivers/gpu/drm/i915/intel_lrc.c | 30 -- > > drivers/gpu/drm/i915/intel_lrc.h | 3 +++ > > 3 files changed, 28 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_perf.c > > b/drivers/gpu/drm/i915/i915_perf.c > > index ccb20230df2c..dd65b72bddd4 100644 > > --- a/drivers/gpu/drm/i915/i915_perf.c > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > @@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct > > i915_gem_context *ctx, > > > > CTX_REG(reg_state, state_offset, flex_regs[i], value); > > } > > + > > + CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, > > + gen8_make_rpcs(dev_priv, > > +&to_intel_context(ctx, > > + > > dev_priv->engine[RCS])->sseu)); > > > I think there is one issue I missed on the previous iterations of this > patch. > > This gen8_update_reg_state_unlocked() is called when the GPU is parked > on the kernel context. > > It's supposed to update all contexts, but I think we might not be able > to update the kernel context image while the GPU is using it. The kernel context is only ever taken in extremis (you are either parking or stalling userspace) so I don't care. > Context save might happen after we edited the image and that would > override the values we just put in there. > > > The OA config is emitted through context image edition in this function > but also through the ring buffer in > gen8_switch_to_updated_kernel_context() for the kernel context. > > Since we can't have a context modify its own RCPS value, we'll have to > resort to yet another context to do that for the kernel context. > > > I remember having a patch that created yet another kernel context (let's > call it rpcs edition context), which is used to reconfigure rpcs for > every context but itself and then have the kernel context reconfigure > this rpcs edition context. > > Or alternatively not do anything to it, because it's only going to run > to edit other contexts at a time when we don't care about power > configuration stability. Exactly. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Update todo.rst
Am Mittwoch, 5. September 2018, 20:15:09 CEST schrieb Daniel Vetter: > - drmP.h is now fully split up. > - vkms is happening (and will gain its own todo and docs under a new > vkms.rst file real soon) > - legacy cruft is completely hidden now, drm_vblank.c is split out > from drm_irq.c now. I've decided to drop the task to split out > drm_legacy.ko, partially because Dave already rejected a patch to > hide the old dri1 drivers better. Current state feels good enough to > me. > - best_encoder atomic cleanup is done (it's now the default, not even > exported anymore) > - bunch of smaller things > > v2: > - Explain why the drm_legacy.ko task is dropped (Emil). > - typos (Sam). > > v3: Fix typo (Ilia) > > Cc: Ilia Mirkin > Cc: Sam Ravnborg > Cc: Emil Velikov > Signed-off-by: Daniel Vetter > Cc: Gustavo Padovan > Cc: Maarten Lankhorst > Cc: Sean Paul > Cc: David Airlie I've read through the text changes and didn't spot any glaring typos [beware non-native speaker], so fwiw Reviewed-by: Heiko Stuebner ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
On 06/09/2018 11:10, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-09-06 10:57:47) On 05/09/2018 15:22, Tvrtko Ursulin wrote: From: Lionel Landwerlin If some of the contexts submitting workloads to the GPU have been configured to shutdown slices/subslices, we might loose the NOA configurations written in the NOA muxes. One possible solution to this problem is to reprogram the NOA muxes when we switch to a new context. We initially tried this in the workaround batchbuffer but some concerns where raised about the cost of reprogramming at every context switch. This solution is also not without consequences from the userspace point of view. Reprogramming of the muxes can only happen once the powergating configuration has changed (which happens after context switch). This means for a window of time during the recording, counters recorded by the OA unit might be invalid. This requires userspace dealing with OA reports to discard the invalid values. Minimizing the reprogramming could be implemented by tracking of the last programmed configuration somewhere in GGTT and use MI_PREDICATE to discard some of the programming commands, but the command streamer would still have to parse all the MI_LRI instructions in the workaround batchbuffer. Another solution, which this change implements, is to simply disregard the user requested configuration for the period of time when i915/perf is active. There is no known issue with this apart from a performance penality for some media workloads that benefit from running on a partially powergated GPU. We already prevent RC6 from affecting the programming so it doesn't sound completely unreasonable to hold on powergating for the same reason. v2: Leave RPCS programming in intel_lrc.c (Lionel) v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel) More to_intel_context() (Tvrtko) s/dev_priv/i915/ (Tvrtko) Tvrtko Ursulin: v4: * Rebase for make_rpcs changes. v5: * Apply OA restriction from make_rpcs directly. v6: * Rebase for context image setup changes. Signed-off-by: Lionel Landwerlin Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_perf.c | 5 + drivers/gpu/drm/i915/intel_lrc.c | 30 -- drivers/gpu/drm/i915/intel_lrc.h | 3 +++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index ccb20230df2c..dd65b72bddd4 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx, CTX_REG(reg_state, state_offset, flex_regs[i], value); } + + CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, + gen8_make_rpcs(dev_priv, +&to_intel_context(ctx, + dev_priv->engine[RCS])->sseu)); I think there is one issue I missed on the previous iterations of this patch. This gen8_update_reg_state_unlocked() is called when the GPU is parked on the kernel context. It's supposed to update all contexts, but I think we might not be able to update the kernel context image while the GPU is using it. The kernel context is only ever taken in extremis (you are either parking or stalling userspace) so I don't care. The patch exposing the RPCS configuration to userspace will make use of the kernel context while OA/perf is enabled. Even if it reprograms the locked value that will break the power configuration stability on Gen11 (because the locked configuration will be different from the kernel context configuration). - Lionel Context save might happen after we edited the image and that would override the values we just put in there. The OA config is emitted through context image edition in this function but also through the ring buffer in gen8_switch_to_updated_kernel_context() for the kernel context. Since we can't have a context modify its own RCPS value, we'll have to resort to yet another context to do that for the kernel context. I remember having a patch that created yet another kernel context (let's call it rpcs edition context), which is used to reconfigure rpcs for every context but itself and then have the kernel context reconfigure this rpcs edition context. Or alternatively not do anything to it, because it's only going to run to edit other contexts at a time when we don't care about power configuration stability. Exactly. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
Quoting Lionel Landwerlin (2018-09-06 11:18:01) > On 06/09/2018 11:10, Chris Wilson wrote: > > Quoting Lionel Landwerlin (2018-09-06 10:57:47) > >> On 05/09/2018 15:22, Tvrtko Ursulin wrote: > >>> From: Lionel Landwerlin > >>> > >>> If some of the contexts submitting workloads to the GPU have been > >>> configured to shutdown slices/subslices, we might loose the NOA > >>> configurations written in the NOA muxes. > >>> > >>> One possible solution to this problem is to reprogram the NOA muxes > >>> when we switch to a new context. We initially tried this in the > >>> workaround batchbuffer but some concerns where raised about the cost > >>> of reprogramming at every context switch. This solution is also not > >>> without consequences from the userspace point of view. Reprogramming > >>> of the muxes can only happen once the powergating configuration has > >>> changed (which happens after context switch). This means for a window > >>> of time during the recording, counters recorded by the OA unit might > >>> be invalid. This requires userspace dealing with OA reports to discard > >>> the invalid values. > >>> > >>> Minimizing the reprogramming could be implemented by tracking of the > >>> last programmed configuration somewhere in GGTT and use MI_PREDICATE > >>> to discard some of the programming commands, but the command streamer > >>> would still have to parse all the MI_LRI instructions in the > >>> workaround batchbuffer. > >>> > >>> Another solution, which this change implements, is to simply disregard > >>> the user requested configuration for the period of time when i915/perf > >>> is active. There is no known issue with this apart from a performance > >>> penality for some media workloads that benefit from running on a > >>> partially powergated GPU. We already prevent RC6 from affecting the > >>> programming so it doesn't sound completely unreasonable to hold on > >>> powergating for the same reason. > >>> > >>> v2: Leave RPCS programming in intel_lrc.c (Lionel) > >>> > >>> v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel) > >>> More to_intel_context() (Tvrtko) > >>> s/dev_priv/i915/ (Tvrtko) > >>> > >>> Tvrtko Ursulin: > >>> > >>> v4: > >>>* Rebase for make_rpcs changes. > >>> > >>> v5: > >>>* Apply OA restriction from make_rpcs directly. > >>> > >>> v6: > >>>* Rebase for context image setup changes. > >>> > >>> Signed-off-by: Lionel Landwerlin > >>> Signed-off-by: Tvrtko Ursulin > >>> --- > >>>drivers/gpu/drm/i915/i915_perf.c | 5 + > >>>drivers/gpu/drm/i915/intel_lrc.c | 30 -- > >>>drivers/gpu/drm/i915/intel_lrc.h | 3 +++ > >>>3 files changed, 28 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/i915_perf.c > >>> b/drivers/gpu/drm/i915/i915_perf.c > >>> index ccb20230df2c..dd65b72bddd4 100644 > >>> --- a/drivers/gpu/drm/i915/i915_perf.c > >>> +++ b/drivers/gpu/drm/i915/i915_perf.c > >>> @@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct > >>> i915_gem_context *ctx, > >>> > >>>CTX_REG(reg_state, state_offset, flex_regs[i], value); > >>>} > >>> + > >>> + CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, > >>> + gen8_make_rpcs(dev_priv, > >>> +&to_intel_context(ctx, > >>> + > >>> dev_priv->engine[RCS])->sseu)); > >> > >> I think there is one issue I missed on the previous iterations of this > >> patch. > >> > >> This gen8_update_reg_state_unlocked() is called when the GPU is parked > >> on the kernel context. > >> > >> It's supposed to update all contexts, but I think we might not be able > >> to update the kernel context image while the GPU is using it. > > The kernel context is only ever taken in extremis (you are either > > parking or stalling userspace) so I don't care. > > > The patch exposing the RPCS configuration to userspace will make use of > the kernel context while OA/perf is enabled. Even if it reprograms the > locked value that will break the power configuration stability on Gen11 > (because the locked configuration will be different from the kernel > context configuration). Sure, but as you point out that's only on changing configuration. What's missing in the patch is that we only bail early if the new sseu matches the ce->sseu, but that doesn't necessarily match whats in the context due to OA. (Or maybe I missed the conversion to rpcs value and checking.) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/csr: keep max firmware size together with firmare name and version
On Thu, 06 Sep 2018, Chris Wilson wrote:
> Quoting Jani Nikula (2018-09-06 09:21:25)
>> Move max firmware size to the same if ladder with firmware name and
>> required version. This allows us to detect the missing max size for a
>> platform without actually loading the firmware, and makes the whole
>> thing easier to maintain.
>>
>> We need to move the power get earlier to allow for early return in the
>> missing platform case. We also need to move the module parameter
>> override later to reuse the max firmware size, which is independent of
>> the override. Note how this works with gen 11+ which don't have a
>> specified firmware blob yet, but do have a maximum size.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>> drivers/gpu/drm/i915/intel_csr.c | 41
>> +---
>> 2 files changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 368066010f94..62444f4c3c8e 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -465,8 +465,9 @@ struct intel_csr {
>> struct work_struct work;
>> const char *fw_path;
>> uint32_t required_version;
>> + uint32_t max_fw_size; /* bytes */
>> uint32_t *dmc_payload;
>> - uint32_t dmc_fw_size;
>> + uint32_t dmc_fw_size; /* dwords */
>
> Appreciated.
>
>> uint32_t version;
>> uint32_t mmio_count;
>> i915_reg_t mmioaddr[8];
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c
>> b/drivers/gpu/drm/i915/intel_csr.c
>> index 9a60bb9cc443..956ac8bbf5e4 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -281,7 +281,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
>> *dev_priv,
>> struct intel_csr *csr = &dev_priv->csr;
>> const struct stepping_info *si = intel_get_stepping_info(dev_priv);
>> uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>> - uint32_t max_fw_size = 0;
>> uint32_t i;
>> uint32_t *dmc_payload;
>>
>> @@ -378,15 +377,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
>> *dev_priv,
>>
>> /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>> nbytes = dmc_header->fw_size * 4;
>> - if (INTEL_GEN(dev_priv) >= 11)
>> - max_fw_size = ICL_CSR_MAX_FW_SIZE;
>> - else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> - max_fw_size = GLK_CSR_MAX_FW_SIZE;
>> - else if (IS_GEN9(dev_priv))
>> - max_fw_size = BXT_CSR_MAX_FW_SIZE;
>> - else
>> - MISSING_CASE(INTEL_REVID(dev_priv));
>> - if (nbytes > max_fw_size) {
>> + if (nbytes > csr->max_fw_size) {
>
> Ok.
>> DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>> return NULL;
>> }
>> @@ -451,32 +442,44 @@ void intel_csr_ucode_init(struct drm_i915_private
>> *dev_priv)
>> if (!HAS_CSR(dev_priv))
>> return;
>>
>> - if (i915_modparams.dmc_firmware_path) {
>> - csr->fw_path = i915_modparams.dmc_firmware_path;
>> - /* Bypass version check for firmware override. */
>> - csr->required_version = 0;
>> + /*
>> +* Obtain a runtime pm reference, until CSR is loaded,
>> +* to avoid entering runtime-suspend.
>
> * On error, we return with the rpm wakeref held to prevent
> * runtime suspend as runtime suspend *requires* a working
> * CSR for whatever reason.
>> +*/
>> + intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
>
> A reminder that the leak is by design. Bonus points for integrating with
> the wakeref tracking so that we demonstrate we aren't just leaking for
> the fun of it.
>
>> + if (INTEL_GEN(dev_priv) >= 11) {
>> + csr->max_fw_size = ICL_CSR_MAX_FW_SIZE;
>
> Ok.
>
>> } else if (IS_CANNONLAKE(dev_priv)) {
>> csr->fw_path = I915_CSR_CNL;
>> csr->required_version = CNL_CSR_VERSION_REQUIRED;
>> + csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
> Ok.
>
>> } else if (IS_GEMINILAKE(dev_priv)) {
>> csr->fw_path = I915_CSR_GLK;
>> csr->required_version = GLK_CSR_VERSION_REQUIRED;
>> + csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
> Ok.
>
>> } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
>> csr->fw_path = I915_CSR_KBL;
>> csr->required_version = KBL_CSR_VERSION_REQUIRED;
>> + csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
> Ok.
>
>> } else if (IS_SKYLAKE(dev_priv)) {
>> csr->fw_path = I915_CSR_SKL;
>> csr->required_version = SKL_CSR_VERSION_REQUIRED;
>> + csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
> Ok, but intere
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: add LG eDP panel to quirk database
== Series Details == Series: drm/i915: add LG eDP panel to quirk database URL : https://patchwork.freedesktop.org/series/49251/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4776_full -> Patchwork_10105_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10105_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10105_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_10105_full: === IGT changes === Possible regressions igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-blt: shard-kbl: PASS -> DMESG-FAIL == Known issues == Here are the changes found in Patchwork_10105_full that come from known issues: === IGT changes === Issues hit igt@gem_exec_schedule@preempt-queue-contexts-chain-blt: shard-kbl: PASS -> INCOMPLETE (fdo#103665) igt@kms_cursor_legacy@cursor-vs-flip-toggle: shard-hsw: PASS -> FAIL (fdo#103355) Possible fixes igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: FAIL (fdo#105363) -> PASS igt@kms_frontbuffer_tracking@fbc-badstride: shard-glk: FAIL (fdo#103167) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend: shard-apl: INCOMPLETE (fdo#103927) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 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_4776 -> Patchwork_10105 CI_DRM_4776: 18e1a6da2ae33504c83d1e5dfc7a08a3940e7b58 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10105: e5c2ff993f03a1fc46874810f8c730f222d9e7ca @ 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_10105/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
On 06/09/2018 11:22, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-09-06 11:18:01) On 06/09/2018 11:10, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-09-06 10:57:47) On 05/09/2018 15:22, Tvrtko Ursulin wrote: From: Lionel Landwerlin If some of the contexts submitting workloads to the GPU have been configured to shutdown slices/subslices, we might loose the NOA configurations written in the NOA muxes. One possible solution to this problem is to reprogram the NOA muxes when we switch to a new context. We initially tried this in the workaround batchbuffer but some concerns where raised about the cost of reprogramming at every context switch. This solution is also not without consequences from the userspace point of view. Reprogramming of the muxes can only happen once the powergating configuration has changed (which happens after context switch). This means for a window of time during the recording, counters recorded by the OA unit might be invalid. This requires userspace dealing with OA reports to discard the invalid values. Minimizing the reprogramming could be implemented by tracking of the last programmed configuration somewhere in GGTT and use MI_PREDICATE to discard some of the programming commands, but the command streamer would still have to parse all the MI_LRI instructions in the workaround batchbuffer. Another solution, which this change implements, is to simply disregard the user requested configuration for the period of time when i915/perf is active. There is no known issue with this apart from a performance penality for some media workloads that benefit from running on a partially powergated GPU. We already prevent RC6 from affecting the programming so it doesn't sound completely unreasonable to hold on powergating for the same reason. v2: Leave RPCS programming in intel_lrc.c (Lionel) v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel) More to_intel_context() (Tvrtko) s/dev_priv/i915/ (Tvrtko) Tvrtko Ursulin: v4: * Rebase for make_rpcs changes. v5: * Apply OA restriction from make_rpcs directly. v6: * Rebase for context image setup changes. Signed-off-by: Lionel Landwerlin Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_perf.c | 5 + drivers/gpu/drm/i915/intel_lrc.c | 30 -- drivers/gpu/drm/i915/intel_lrc.h | 3 +++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index ccb20230df2c..dd65b72bddd4 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx, CTX_REG(reg_state, state_offset, flex_regs[i], value); } + + CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, + gen8_make_rpcs(dev_priv, +&to_intel_context(ctx, + dev_priv->engine[RCS])->sseu)); I think there is one issue I missed on the previous iterations of this patch. This gen8_update_reg_state_unlocked() is called when the GPU is parked on the kernel context. It's supposed to update all contexts, but I think we might not be able to update the kernel context image while the GPU is using it. The kernel context is only ever taken in extremis (you are either parking or stalling userspace) so I don't care. The patch exposing the RPCS configuration to userspace will make use of the kernel context while OA/perf is enabled. Even if it reprograms the locked value that will break the power configuration stability on Gen11 (because the locked configuration will be different from the kernel context configuration). Sure, but as you point out that's only on changing configuration. What's missing in the patch is that we only bail early if the new sseu matches the ce->sseu, but that doesn't necessarily match whats in the context due to OA. (Or maybe I missed the conversion to rpcs value and checking.) -Chris Yep, because the gen8_make_rpcs() post processes the values store at the gem context level, we risk rerunning the kernel context to write the exiting value. Sorry this is all so messy :( - Lionel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/2] drm/i915/skl+: Update result lines in correspondence with result blocks
As the result blocks for WM1-WM7 are always kept higher than the
level below the present level, make sure result lines are also higher
than the level below for WM1-WM7.
V3: Updated the commit message.
Signed-off-by: Karthik B S
---
drivers/gpu/drm/i915/intel_pm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b5db6a3..cc41009 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4704,6 +4704,8 @@ static int skl_compute_plane_wm(const struct
drm_i915_private *dev_priv,
if (level >= 1 && level <= 7) {
if (result_prev->plane_res_b > res_blocks)
res_blocks = result_prev->plane_res_b;
+ if (result_prev->plane_res_l > res_lines)
+ res_lines = result_prev->plane_res_l;
}
if (INTEL_GEN(dev_priv) >= 11) {
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/3] drm/i915/csr: keep firmware name and required version together
Having two separate if ladders gets increasingly hard to maintain. Put
them together.
Reviewed-by: Chris Wilson
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_csr.c | 54
2 files changed, 23 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 767615ecdea5..368066010f94 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -464,6 +464,7 @@ struct drm_i915_display_funcs {
struct intel_csr {
struct work_struct work;
const char *fw_path;
+ uint32_t required_version;
uint32_t *dmc_payload;
uint32_t dmc_fw_size;
uint32_t version;
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 14cf4c367e36..9a60bb9cc443 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -284,7 +284,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
uint32_t max_fw_size = 0;
uint32_t i;
uint32_t *dmc_payload;
- uint32_t required_version;
if (!fw)
return NULL;
@@ -299,36 +298,19 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
return NULL;
}
- csr->version = css_header->version;
-
- if (csr->fw_path == i915_modparams.dmc_firmware_path) {
- /* Bypass version check for firmware override. */
- required_version = csr->version;
- } else if (IS_CANNONLAKE(dev_priv)) {
- required_version = CNL_CSR_VERSION_REQUIRED;
- } else if (IS_GEMINILAKE(dev_priv)) {
- required_version = GLK_CSR_VERSION_REQUIRED;
- } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
- required_version = KBL_CSR_VERSION_REQUIRED;
- } else if (IS_SKYLAKE(dev_priv)) {
- required_version = SKL_CSR_VERSION_REQUIRED;
- } else if (IS_BROXTON(dev_priv)) {
- required_version = BXT_CSR_VERSION_REQUIRED;
- } else {
- MISSING_CASE(INTEL_REVID(dev_priv));
- required_version = 0;
- }
-
- if (csr->version != required_version) {
+ if (csr->required_version &&
+ css_header->version != csr->required_version) {
DRM_INFO("Refusing to load DMC firmware v%u.%u,"
" please use v%u.%u\n",
-CSR_VERSION_MAJOR(csr->version),
-CSR_VERSION_MINOR(csr->version),
-CSR_VERSION_MAJOR(required_version),
-CSR_VERSION_MINOR(required_version));
+CSR_VERSION_MAJOR(css_header->version),
+CSR_VERSION_MINOR(css_header->version),
+CSR_VERSION_MAJOR(csr->required_version),
+CSR_VERSION_MINOR(csr->required_version));
return NULL;
}
+ csr->version = css_header->version;
+
readcount += sizeof(struct intel_css_header);
/* Extract Package Header information*/
@@ -469,18 +451,26 @@ void intel_csr_ucode_init(struct drm_i915_private
*dev_priv)
if (!HAS_CSR(dev_priv))
return;
- if (i915_modparams.dmc_firmware_path)
+ if (i915_modparams.dmc_firmware_path) {
csr->fw_path = i915_modparams.dmc_firmware_path;
- else if (IS_CANNONLAKE(dev_priv))
+ /* Bypass version check for firmware override. */
+ csr->required_version = 0;
+ } else if (IS_CANNONLAKE(dev_priv)) {
csr->fw_path = I915_CSR_CNL;
- else if (IS_GEMINILAKE(dev_priv))
+ csr->required_version = CNL_CSR_VERSION_REQUIRED;
+ } else if (IS_GEMINILAKE(dev_priv)) {
csr->fw_path = I915_CSR_GLK;
- else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
+ csr->required_version = GLK_CSR_VERSION_REQUIRED;
+ } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
csr->fw_path = I915_CSR_KBL;
- else if (IS_SKYLAKE(dev_priv))
+ csr->required_version = KBL_CSR_VERSION_REQUIRED;
+ } else if (IS_SKYLAKE(dev_priv)) {
csr->fw_path = I915_CSR_SKL;
- else if (IS_BROXTON(dev_priv))
+ csr->required_version = SKL_CSR_VERSION_REQUIRED;
+ } else if (IS_BROXTON(dev_priv)) {
csr->fw_path = I915_CSR_BXT;
+ csr->required_version = BXT_CSR_VERSION_REQUIRED;
+ }
/*
* Obtain a runtime pm reference, until CSR is loaded,
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/3] drm/i915/csr: keep max firmware size together with firmare name and version
Move max firmware size to the same if ladder with firmware name and
required version. This allows us to detect the missing max size for a
platform without actually loading the firmware, and makes the whole
thing easier to maintain.
We need to move the power get earlier to allow for early return in the
missing platform case. While at it, extend the comment on why we return
with the reference held on errors.
We also need to move the module parameter override later to reuse the
max firmware size, which is independent of the override. Note how this
works with gen 11+ which don't have a specified firmware blob yet, but
do have a maximum size.
v2: Add comment on why we leak the wakeref on errors (Chris)
Reviewed-by: Chris Wilson
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_csr.c | 45 +++-
2 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 368066010f94..62444f4c3c8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -465,8 +465,9 @@ struct intel_csr {
struct work_struct work;
const char *fw_path;
uint32_t required_version;
+ uint32_t max_fw_size; /* bytes */
uint32_t *dmc_payload;
- uint32_t dmc_fw_size;
+ uint32_t dmc_fw_size; /* dwords */
uint32_t version;
uint32_t mmio_count;
i915_reg_t mmioaddr[8];
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9a60bb9cc443..f6ad94d8b441 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -281,7 +281,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
struct intel_csr *csr = &dev_priv->csr;
const struct stepping_info *si = intel_get_stepping_info(dev_priv);
uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
- uint32_t max_fw_size = 0;
uint32_t i;
uint32_t *dmc_payload;
@@ -378,15 +377,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
nbytes = dmc_header->fw_size * 4;
- if (INTEL_GEN(dev_priv) >= 11)
- max_fw_size = ICL_CSR_MAX_FW_SIZE;
- else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
- max_fw_size = GLK_CSR_MAX_FW_SIZE;
- else if (IS_GEN9(dev_priv))
- max_fw_size = BXT_CSR_MAX_FW_SIZE;
- else
- MISSING_CASE(INTEL_REVID(dev_priv));
- if (nbytes > max_fw_size) {
+ if (nbytes > csr->max_fw_size) {
DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
return NULL;
}
@@ -451,32 +442,48 @@ void intel_csr_ucode_init(struct drm_i915_private
*dev_priv)
if (!HAS_CSR(dev_priv))
return;
- if (i915_modparams.dmc_firmware_path) {
- csr->fw_path = i915_modparams.dmc_firmware_path;
- /* Bypass version check for firmware override. */
- csr->required_version = 0;
+ /*
+* Obtain a runtime pm reference, until CSR is loaded, to avoid entering
+* runtime-suspend.
+*
+* On error, we return with the rpm wakeref held to prevent runtime
+* suspend as runtime suspend *requires* a working CSR for whatever
+* reason.
+*/
+ intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+
+ if (INTEL_GEN(dev_priv) >= 11) {
+ csr->max_fw_size = ICL_CSR_MAX_FW_SIZE;
} else if (IS_CANNONLAKE(dev_priv)) {
csr->fw_path = I915_CSR_CNL;
csr->required_version = CNL_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
} else if (IS_GEMINILAKE(dev_priv)) {
csr->fw_path = I915_CSR_GLK;
csr->required_version = GLK_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = GLK_CSR_MAX_FW_SIZE;
} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
csr->fw_path = I915_CSR_KBL;
csr->required_version = KBL_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
} else if (IS_SKYLAKE(dev_priv)) {
csr->fw_path = I915_CSR_SKL;
csr->required_version = SKL_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
} else if (IS_BROXTON(dev_priv)) {
csr->fw_path = I915_CSR_BXT;
csr->required_version = BXT_CSR_VERSION_REQUIRED;
+ csr->max_fw_size = BXT_CSR_MAX_FW_SIZE;
+ } else {
+ MISSING_CASE(INTEL_REVID(dev_priv));
+ return;
}
- /*
-* Obtain a runtime pm reference, until CSR is loaded,
-* to avoid entering runtime-suspend.
-*/
-
[Intel-gfx] [PATCH v2 3/3] drm/i915/csr: bypass firmware request on i915.dmc_firmware_path=""
With i915.dmc_firmware_path="" it's obvious the intention is to disable
CSR firmware loading. Bypass the firmware request altogether in this
case, with more obvious debug logging.
v2: Use DRM_INFO for logging (Chris)
Reviewed-by: Chris Wilson
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_csr.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index f6ad94d8b441..2ce55f68ea2d 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -480,6 +480,12 @@ void intel_csr_ucode_init(struct drm_i915_private
*dev_priv)
}
if (i915_modparams.dmc_firmware_path) {
+ if (strlen(i915_modparams.dmc_firmware_path) == 0) {
+ csr->fw_path = NULL;
+ DRM_INFO("Disabling CSR firmare and runtime PM\n");
+ return;
+ }
+
csr->fw_path = i915_modparams.dmc_firmware_path;
/* Bypass version check for firmware override. */
csr->required_version = 0;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/5] lib/igt_vmwgfx: Add vmwgfx device
On Thu, 06 Sep 2018, Petri Latvala wrote:
> On Wed, Sep 05, 2018 at 05:03:46PM -0700, Deepak Rawat wrote:
>> Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
>>
>> Signed-off-by: Deepak Rawat
>> ---
>> lib/drmtest.c | 9 -
>> lib/drmtest.h | 3 +++
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/drmtest.c b/lib/drmtest.c
>> index bfa2e0f0..563d5b8b 100644
>> --- a/lib/drmtest.c
>> +++ b/lib/drmtest.c
>> @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
>> return __is_device(fd, "i915");
>> }
>>
>> +bool is_vmwgfx_device(int fd)
>> +{
>> +return __is_device(fd, "vmwg");
>> +}
>> +
>> static bool has_known_intel_chipset(int fd)
>> {
>> struct drm_i915_getparam gp;
>> @@ -205,7 +210,7 @@ static const struct module {
>> { DRIVER_VC4, "vc4" },
>> { DRIVER_VGEM, "vgem" },
>> { DRIVER_VIRTIO, "virtio-gpu" },
>> -{ DRIVER_VIRTIO, "virtio_gpu" },
>> +{ DRIVER_VMWGFX, "vmwgfx" },
>> {}
>
>
> Don't remove the second virtio-gpu line, there's a reason for it.
>
> we should probably add a comment there.
Don't keep us in suspense!
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 v5 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp
On Wed, Sep 05, 2018 at 01:12:00PM -0700, Radhakrishna Sripada wrote:
> Use the newly added "max bpc" connector property to limit pipe bpp.
>
> V3: Use drm_connector_state to access the "max bpc" property
> V4: Initialize the drm property, add suuport to DP(Ville)
> V5: Use the property in the connector and fix CI failure(Ville)
>
> 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 | 31 +++
> drivers/gpu/drm/i915/intel_dp.c | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_hdmi.c| 7 +++
> drivers/gpu/drm/i915/intel_modes.c | 20
> 5 files changed, 61 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1bd14c61dab5..a890aade094c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10787,6 +10787,34 @@ 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:
> + case 13:
> + case 14:
> + case 15:
> + pipe_config->pipe_bpp = 12*3;
> + break;
> + case 16:
> + pipe_config->pipe_bpp = 16*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)
> @@ -10815,6 +10843,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
> if (connector_state->crtc != &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);
> }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 436c22de33b6..3955745a4d9f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5719,6 +5719,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp,
> struct drm_connector *connect
> intel_attach_force_audio_property(connector);
>
> intel_attach_broadcast_rgb_property(connector);
> + intel_attach_max_bpc_property(connector, 8, 16);
IIRC gmch platforms can't do more than 10bpc, and the rest
are limited to 12bpc.
>
> if (intel_dp_is_edp(intel_dp)) {
> u32 allowed_scalers;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f5731215210a..b3c703dacc92 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1869,6 +1869,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 a2dab0b6bde6..e649bbf07642 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2109,11 +2109,18 @@ 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 (HAS_GMCH_DISPLAY(dev_priv))
> + intel_attach_max_bpc_property(connector, 8, 8);
Not sure exposing the prop makes much sense when you can't modify it.
> + else
> + intel_attach_max_bpc_property(connector, 8, 12);
> }
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_modes.c
> b/drivers/gpu/drm/i915
[Intel-gfx] Updated drm-intel-testing
Hi all, The following changes tagged drm-intel-testing-2018-09-06: drm-intel-next-2018-09-06: UAPI Changes: - GGTT coherency GETPARAM: GGTT has turned out to be non-coherent for some platforms, which we've failed to communicate to userspace so far. SNA was modified to do extra flushing on non-coherent GGTT access, while Mesa will mitigate by always requiring WC mapping (which is non-coherent anyway). - Neuter Resource Streamer uAPI: There never really were users for the feature, so neuter it while keeping the interface bits for compatibility. This is a long due item from past. Cross-subsystem Changes: - Backmerge of branch drm-next-4.19 for DP_DPCD_REV_14 changes Core Changes: - None Driver Changes: - A load of Icelake (ICL) enabling patches (Paulo, Manasi) - Enabled full PPGTT for IVB,VLV and HSW (Chris) - Bugzilla #107113: Distribute DDB based on display resolutions (Mahesh) - Bugzillas #100023,#107476,#94921: Support limited range DP displays (Jani) - Bugzilla #107503: Increase LSPCON timeout (Fredrik) - Avoid boosting GPU due to an occasional stall in interactive workloads (Chris) - Apply GGTT coherency W/A only for affected systems instead of all (Chris) - Fix for infinite link training loop for faulty USB-C MST hubs (Nathan) - Keep KMS functional on Gen4 and earlier when GPU is wedged (Chris) - Stop holding ppGTT reference from closed VMAs (Chris) - Clear error registers after error capture (Lionel) - Various Icelake fixes (Anusha, Jyoti, Ville, Tvrtko) - Add missing Coffeelake (CFL) PCI IDs (Rodrigo) - Flush execlists tasklet directly from reset-finish (Chris) - Fix LPE audio runtime PM (Chris) - Fix detection of out of range surface positions (GLK/CNL) (Ville) - Remove wait-for-idle for PSR2 (Dhinakaran) - Power down existing display hardware resources when display is disabled (Chris) - Don't allow runtime power management if RC6 doesn't exist (Chris) - Add debugging checks for runtime power management paths (Imre) - Increase symmetry in display power init/fini paths (Imre) - Isolate GVT specific macros from i915_reg.h (Lucas) - Increase symmetry in power management enable/disable paths (Chris) - Increase IP disable timeout to 100 ms to avoid DRM_ERROR (Imre) - Fix memory leak from HDMI HDCP write function (Brian, Rodrigo) - Reject Y/Yf tiling on interlaced modes (Ville) - Use a cached mapping for the physical HWS on older gens (Chris) - Force slow path of writing relocations to buffer if unable to write to userspace (Chris) - Do a full device reset after being wedged (Chris) - Keep forcewake counts over reset (in case of debugfs user) (Imre, Chris) - Avoid false-positive errors from power wells during init (Imre) - Reset engines forcibly in exchange of declaring whole device wedged (Mika) - Reduce context HW ID lifetime in preparation for Icelake (Chris) - Attempt to recover from module load failures (Chris) - Keep select interrupts over a reset to avoid missing/losing them (Chris) - GuC submission backend improvements (Jakub) - Terminate context images with BB_END (Chris, Lionel) - Make GCC evaluate GGTT view struct size assertions again (Ville) - Add selftest to exercise suspend/hibernate code-paths for GEM (Chris) - Use a full emulation of a user ppgtt context in selftests (Chris) - Exercise resetting in the middle of a wait-on-fence in selftests (Chris) - Fix coherency issues on selftests for Baytrail (Chris) - Various other GEM fixes / self-test updates (Chris, Matt) - GuC doorbell self-tests (Daniele) - PSR mode control through debugfs for IGTs (Maarten) - Degrade expected WM latency errors to DRM_DEBUG_KMS (Chris) - Cope with errors better in MST link training (Dhinakaran) - Fix WARN on KBL external displays (Azhar) - Power well code cleanups (Imre) - Fixes to PSR debugging (Dhinakaran) - Make forcewake errors louder for easier catching in CI (WARNs) (Chris) - Fortify tiling code against programmer errors (Chris) - Bunch of fixes for CI exposed corner cases (multiple authors, mostly Chris) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] gvt-next for 4.20
Quoting Zhenyu Wang (2018-09-04 06:01:54) > > Hi, > > Here's initial gvt-next for 4.20 with two optimization for > guest context shadowing and command parser, and with W=1 build fixes. Thanks, pulled this, but it had one merge conflict (in gvt/reg.h). Please make sure the resolution I put in into drm-intel-next is correct. Regards, Joonas > > Thanks > -- > The following changes since commit 279ce5d117078ee8ea40c40199399889981fd808: > > drm/i915/gvt: declare gvt as i915's soft dependency (2018-07-10 11:13:11 > +0800) > > are available in the Git repository at: > > https://github.com/intel/gvt-linux.git tags/gvt-next-2018-09-04 > > for you to fetch changes up to 69ca5af4ff9a3ff96e4595c2b7522c01a2641779: > > drm/i915/gvt: Move some MMIO definitions to reg.h (2018-08-07 10:40:11 > +0800) > > > gvt-next-2018-09-04 > > - guest context shadow optimization for restore inhibit one (Yan) > - cmd parser optimization (Yan) > - W=1 warning fixes (Zhenyu) > > > Zhao Yan (2): > drm/i915/gvt: add a fastpath for cmd parsing on MI_NOOP > drm/i915/gvt: only copy the first page for restore inhibit context > > Zhenyu Wang (3): > drm/i915/gvt: make dma map/unmap kvmgt functions as static > drm/i915/gvt: Fix function comment doc errors > drm/i915/gvt: Move some MMIO definitions to reg.h > > drivers/gpu/drm/i915/gvt/cfg_space.c| 12 +++ > drivers/gpu/drm/i915/gvt/cmd_parser.c | 11 +- > drivers/gpu/drm/i915/gvt/display.c | 1 + > drivers/gpu/drm/i915/gvt/edid.c | 9 + > drivers/gpu/drm/i915/gvt/gtt.c | 9 +++-- > drivers/gpu/drm/i915/gvt/gvt.c | 3 +- > drivers/gpu/drm/i915/gvt/handlers.c | 1 + > drivers/gpu/drm/i915/gvt/kvmgt.c| 4 +-- > drivers/gpu/drm/i915/gvt/mmio.c | 3 +- > drivers/gpu/drm/i915/gvt/mmio_context.c | 13 --- > drivers/gpu/drm/i915/gvt/mmio_context.h | 3 ++ > drivers/gpu/drm/i915/gvt/opregion.c | 1 - > drivers/gpu/drm/i915/gvt/page_track.c | 2 ++ > drivers/gpu/drm/i915/gvt/reg.h | 9 + > drivers/gpu/drm/i915/gvt/scheduler.c| 64 > + > 15 files changed, 92 insertions(+), 53 deletions(-) > > > -- > Open Source Technology Center, Intel ltd. > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Update todo.rst
On 6 September 2018 at 10:40, Heiko Stuebner wrote: > Am Mittwoch, 5. September 2018, 20:15:09 CEST schrieb Daniel Vetter: >> - drmP.h is now fully split up. >> - vkms is happening (and will gain its own todo and docs under a new >> vkms.rst file real soon) >> - legacy cruft is completely hidden now, drm_vblank.c is split out >> from drm_irq.c now. I've decided to drop the task to split out >> drm_legacy.ko, partially because Dave already rejected a patch to >> hide the old dri1 drivers better. Current state feels good enough to >> me. >> - best_encoder atomic cleanup is done (it's now the default, not even >> exported anymore) >> - bunch of smaller things >> >> v2: >> - Explain why the drm_legacy.ko task is dropped (Emil). >> - typos (Sam). >> >> v3: Fix typo (Ilia) >> >> Cc: Ilia Mirkin >> Cc: Sam Ravnborg >> Cc: Emil Velikov >> Signed-off-by: Daniel Vetter >> Cc: Gustavo Padovan >> Cc: Maarten Lankhorst >> Cc: Sean Paul >> Cc: David Airlie > > I've read through the text changes and didn't spot any glaring typos > [beware non-native speaker], so fwiw Most of the people in the CC list are ;-) Fwiw Reviewed-by: Emil Velikov Thanks Emil ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/icl: Avoid Gen10 watermark workarounds in Gen11 (rev2)
== Series Details == Series: series starting with [v2,1/2] drm/i915/icl: Avoid Gen10 watermark workarounds in Gen11 (rev2) URL : https://patchwork.freedesktop.org/series/49170/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4780 -> Patchwork_10108 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49170/revisions/2/mbox/ == Known issues == Here are the changes found in Patchwork_10108 that come from known issues: === IGT changes === Possible fixes igt@drv_module_reload@basic-reload: fi-glk-j4005: DMESG-WARN (fdo#106248, fdo#106725) -> PASS igt@drv_selftest@live_hangcheck: fi-skl-guc: DMESG-FAIL (fdo#107710, fdo#107837, fdo#107174) -> PASS igt@kms_flip@basic-flip-vs-modeset: fi-glk-j4005: DMESG-WARN (fdo#106000) -> PASS igt@pm_rpm@module-reload: fi-glk-j4005: DMESG-FAIL (fdo#104767) -> PASS fdo#104767 https://bugs.freedesktop.org/show_bug.cgi?id=104767 fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000 fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248 fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725 fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174 fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710 fdo#107837 https://bugs.freedesktop.org/show_bug.cgi?id=107837 == 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_4780 -> Patchwork_10108 CI_DRM_4780: de91607e0352454c2bcbaa8560214d95c70b8f75 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10108: 57b3d962912e95f7137280c67d79bc96f7e53d1f @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 57b3d962912e drm/i915/skl+: Update result lines in correspondence with result blocks 54946090438b drm/i915/icl: Avoid Gen10 watermark workarounds in Gen11 == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10108/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Updated drm-intel-testing (this time its done right)
Hi all, Disregard the previous message, and look at this tag instead :P I'll still apply the gvt-next pull and do one more tag. Regards, Joonas The following changes tagged drm-intel-testing-2018-09-06-1: drm-intel-next-2018-09-06-1: UAPI Changes: - GGTT coherency GETPARAM: GGTT has turned out to be non-coherent for some platforms, which we've failed to communicate to userspace so far. SNA was modified to do extra flushing on non-coherent GGTT access, while Mesa will mitigate by always requiring WC mapping (which is non-coherent anyway). - Neuter Resource Streamer uAPI: There never really were users for the feature, so neuter it while keeping the interface bits for compatibility. This is a long due item from past. Cross-subsystem Changes: - Backmerge of branch drm-next-4.19 for DP_DPCD_REV_14 changes Core Changes: - None Driver Changes: - A load of Icelake (ICL) enabling patches (Paulo, Manasi) - Enabled full PPGTT for IVB,VLV and HSW (Chris) - Bugzilla #107113: Distribute DDB based on display resolutions (Mahesh) - Bugzillas #100023,#107476,#94921: Support limited range DP displays (Jani) - Bugzilla #107503: Increase LSPCON timeout (Fredrik) - Avoid boosting GPU due to an occasional stall in interactive workloads (Chris) - Apply GGTT coherency W/A only for affected systems instead of all (Chris) - Fix for infinite link training loop for faulty USB-C MST hubs (Nathan) - Keep KMS functional on Gen4 and earlier when GPU is wedged (Chris) - Stop holding ppGTT reference from closed VMAs (Chris) - Clear error registers after error capture (Lionel) - Various Icelake fixes (Anusha, Jyoti, Ville, Tvrtko) - Add missing Coffeelake (CFL) PCI IDs (Rodrigo) - Flush execlists tasklet directly from reset-finish (Chris) - Fix LPE audio runtime PM (Chris) - Fix detection of out of range surface positions (GLK/CNL) (Ville) - Remove wait-for-idle for PSR2 (Dhinakaran) - Power down existing display hardware resources when display is disabled (Chris) - Don't allow runtime power management if RC6 doesn't exist (Chris) - Add debugging checks for runtime power management paths (Imre) - Increase symmetry in display power init/fini paths (Imre) - Isolate GVT specific macros from i915_reg.h (Lucas) - Increase symmetry in power management enable/disable paths (Chris) - Increase IP disable timeout to 100 ms to avoid DRM_ERROR (Imre) - Fix memory leak from HDMI HDCP write function (Brian, Rodrigo) - Reject Y/Yf tiling on interlaced modes (Ville) - Use a cached mapping for the physical HWS on older gens (Chris) - Force slow path of writing relocations to buffer if unable to write to userspace (Chris) - Do a full device reset after being wedged (Chris) - Keep forcewake counts over reset (in case of debugfs user) (Imre, Chris) - Avoid false-positive errors from power wells during init (Imre) - Reset engines forcibly in exchange of declaring whole device wedged (Mika) - Reduce context HW ID lifetime in preparation for Icelake (Chris) - Attempt to recover from module load failures (Chris) - Keep select interrupts over a reset to avoid missing/losing them (Chris) - GuC submission backend improvements (Jakub) - Terminate context images with BB_END (Chris, Lionel) - Make GCC evaluate GGTT view struct size assertions again (Ville) - Add selftest to exercise suspend/hibernate code-paths for GEM (Chris) - Use a full emulation of a user ppgtt context in selftests (Chris) - Exercise resetting in the middle of a wait-on-fence in selftests (Chris) - Fix coherency issues on selftests for Baytrail (Chris) - Various other GEM fixes / self-test updates (Chris, Matt) - GuC doorbell self-tests (Daniele) - PSR mode control through debugfs for IGTs (Maarten) - Degrade expected WM latency errors to DRM_DEBUG_KMS (Chris) - Cope with errors better in MST link training (Dhinakaran) - Fix WARN on KBL external displays (Azhar) - Power well code cleanups (Imre) - Fixes to PSR debugging (Dhinakaran) - Make forcewake errors louder for easier catching in CI (WARNs) (Chris) - Fortify tiling code against programmer errors (Chris) - Bunch of fixes for CI exposed corner cases (multiple authors, mostly Chris) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v5, 02/13] drm/i915/icl: DSI vswing programming sequence
On 7/10/2018 3:10 PM, Madhav Chauhan wrote:
This patch setup voltage swing before enabling
combo PHY DDI (shared with DSI).
Note that DSI voltage swing programming is for
high speed data buffers. HW automatically handles
the voltage swing for the low power data buffers.
v2: Rebase
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/icl_dsi.c | 114 +
1 file changed, 114 insertions(+)
diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index a571339..dc16c1f 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -27,6 +27,65 @@
#include "intel_dsi.h"
+static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ enum port port;
+ u32 tmp;
+ int lane;
+
+ for_each_dsi_port(port, intel_dsi->ports) {
+
+ /* Bspec: set scaling mode to 0x6 */
+ tmp = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+ tmp |= SCALING_MODE_SEL(6);
+ I915_WRITE(ICL_PORT_TX_DW5_GRP(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW5_AUX(port));
+ tmp |= SCALING_MODE_SEL(6);
+ I915_WRITE(ICL_PORT_TX_DW5_AUX(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+ tmp |= TAP2_DISABLE | TAP3_DISABLE;
+ I915_WRITE(ICL_PORT_TX_DW5_GRP(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW5_AUX(port));
+ tmp |= TAP2_DISABLE | TAP3_DISABLE;
+ I915_WRITE(ICL_PORT_TX_DW5_AUX(port), tmp);
+
+ /*
+* swing and scaling values are taken from DSI
+* table under vswing programming sequence for
+* combo phy ddi in BSPEC.
+* program swing values
+*/
+ tmp = I915_READ(ICL_PORT_TX_DW2_LN0(port));
+ tmp |= SWING_SEL_UPPER(0x2);
+ tmp |= SWING_SEL_LOWER(0x2);
+ tmp |= RCOMP_SCALAR(0x98);
+ I915_WRITE(ICL_PORT_TX_DW2_GRP(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW2_AUX(port));
+ tmp |= SWING_SEL_UPPER(0x2);
+ tmp |= SWING_SEL_LOWER(0x2);
+ tmp |= RCOMP_SCALAR(0x98);
+ I915_WRITE(ICL_PORT_TX_DW2_AUX(port), tmp);
+
+ /* program scaling values */
+ tmp = I915_READ(ICL_PORT_TX_DW4_AUX(port));
+ tmp |= POST_CURSOR_1(0x0);
+ tmp |= POST_CURSOR_2(0x0);
+ tmp |= CURSOR_COEFF(0x18);
+ I915_WRITE(ICL_PORT_TX_DW4_AUX(port), tmp);
+
+ for (lane = 0; lane <= 3; lane++) {
+ /* Bspec: must not use GRP register for write */
+ tmp = I915_READ(ICL_PORT_TX_DW4_LN(port, lane));
+ tmp |= POST_CURSOR_1(0x0);
+ tmp |= POST_CURSOR_2(0x0);
+ tmp |= CURSOR_COEFF(0x18);
+ I915_WRITE(ICL_PORT_TX_DW4_LN(port, lane), tmp);
+ }
+ }
+}
+
static void gen11_dsi_program_esc_clk_div(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -140,6 +199,58 @@ static void gen11_dsi_config_phy_lanes_sequence(struct
intel_encoder *encoder)
}
}
I see from the bspec that except for the Loadgen Select and Latency
Optimization all other DDI buffer programming can be taken from the DDI
Buffer section.
Can we use this function "icl_ddi_combo_vswing_program" function which
is already there
patch for reference:
https://patchwork.freedesktop.org/patch/213515/
Thanks,
Vandita
+static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ u32 tmp;
+ enum port port;
+
+ /* Step C.1:clear common keeper enable bit */
+ for_each_dsi_port(port, intel_dsi->ports) {
+ tmp = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
+ tmp &= ~COMMON_KEEPER_EN;
+ I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), tmp);
+ tmp = I915_READ(ICL_PORT_PCS_DW1_AUX(port));
+ tmp &= ~COMMON_KEEPER_EN;
+ I915_WRITE(ICL_PORT_PCS_DW1_AUX(port), tmp);
+ }
+
+ /*
+* Step C.3: Set SUS Clock Config bitfield to 11b
+* Note: Step C.2 (loadgen select program) is done
+* as part of lane phy sequence configuration
+*/
+ for_each_dsi_port(port, intel_dsi->ports) {
+ tmp = I915_READ(ICL_PORT_CL_DW5(port));
+ tmp |= SUS_CLOCK_CONFIG;
+ I915_WRITE(ICL_PORT_CL_DW5(port), tmp);
+ }
+
+ /* Step
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915/csr: keep firmware name and required version together
== Series Details == Series: series starting with [v2,1/3] drm/i915/csr: keep firmware name and required version together URL : https://patchwork.freedesktop.org/series/49269/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/csr: keep firmware name and required version together -drivers/gpu/drm/i915/selftests/../i915_drv.h:3688:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3689:16: warning: expression using sizeof(void) Commit: drm/i915/csr: keep max firmware size together with firmare name and version -drivers/gpu/drm/i915/selftests/../i915_drv.h:3689:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3690:16: warning: expression using sizeof(void) Commit: drm/i915/csr: bypass firmware request on i915.dmc_firmware_path="" Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add LG eDP panel to quirk database
On Thu, 06 Sep 2018, Jani Nikula wrote:
>> The N value was computed by kernel driver that based on synchronous
>> clock mode. But only specific N value (0x8000) would be acceptable for
>> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
>
>As I wrote before, it's the DP source that determines between synchronous and
>asynchronous clock mode, not the sink. The sink in question appears to expect a
>constant N related to asynchronous clock mode even when we're using and
>advertising synchronous clock mode in DP Main Stream Attributes.
>
> (It's possible a certain other OS uses the same constant N regardless of
> clock mode, leading to the panel working by coincidence.)
>
Yes, this sink device should not expect source will give specific N value. I
have no idea what the other OS to calculate M/V value but vendor said they
don't have similar issue before.
>> With the other N value, Tcon will enter BITS mode and display black screen.
>> Add this panel into quirk database and give particular N value when
>> calculate M/N divider.
>>
>> Cc: Jani Nikula
> Cc: Cooper Chiou
>> Cc: Matt Atwood
>> Signed-off-by: Lee, Shawn C
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 10 +-
>> drivers/gpu/drm/i915/intel_display.c | 16 ++--
>> drivers/gpu/drm/i915/intel_display.h | 3 ++-
>> drivers/gpu/drm/i915/intel_dp.c | 8 ++--
>> drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
>> include/drm/drm_dp_helper.h | 1 +
>> 6 files changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c index 0cccbcb2d03e..a0259bbbe9df
>> 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
>> u8 oui[3];
>> bool is_branch;
>> u32 quirks;
>> +u8 dev_id[6];
>
>Your commit message says nothing about the need to extend the quirk mechanism
>to use the device ID. Do we really need it?
>
>If we do need it, please name it device_id like it is in struct
>drm_dp_dpcd_ident, and place it after oui.
>
If we just recognize OUI. And didn't use device_id to identify which
sink/branch device need specific M/N value.
That means all the sink/branch device with Analogix OUI will apply this work
around.
00-22-B9 (hex)Analogix Seminconductor, Inc
>> };
>>
>> #define OUI(first, second, third) { (first), (second), (third) }
>> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
>> +{ (first), (second), (third), (fourth), (fifth), (sixth) }
>>
>> static const struct dpcd_quirk dpcd_quirk_list[] = {
>> /* Analogix 7737 needs reduced M and N at HBR2 link rates */
>> -{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
>> +{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N),
>> +DEVICE_ID(0, 0, 0, 0, 0, 0) },
>
>Maybe add #define DEVICE_ID_ANY to initialize all to zero.
>
>> +/* LG LP140WF6-SPM1 eDP panel */
>> +{ OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N),
>> +DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
>> };
>>
>> #undef OUI
>
>#undef DEVICE_ID
>#undef DEVICE_ID_ANY
>
>> @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident
>> *ident, bool is_branch)
>> if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>> continue;
>>
>> +if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) !=
>> 0)
>> +continue;
>> +
>
>Why not branch devices? This is really a trick to avoid matching on the
>Analogix device which you set to zeros, and now you require device id match
>for all non-branch quirks. Not good. Please check if the >quirk table has all
>zeros, and compare if not.
>
It will not compare device_id for branch device because of I don't have the
device_id for Analogix 7737 branch device you debug before.
So I set it to zero for default setup.
>> quirks |= quirk->quirks;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index ec3e24f07486..ff01a742b054 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct
>> intel_crtc *intel_crtc,
>> pipe_config->fdi_lanes = lane;
>>
>> intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
>> - link_bw, &pipe_config->fdi_m_n, false);
>> + link_bw, &pipe_config->fdi_m_n, false, false);
>
>This is getting pretty ugly, to be honest. Which is why I was wondering if we
>could actually turn the Analogix quirk into a constant N...
>
Seems Anaglogix 7737 can't handle full 24-bit main link Mdiv and Ndiv main link
attributes properly.
Using constant N (0x8000) value should be acceptable. It would not exceed the
An
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915/csr: keep firmware name and required version together
== Series Details == Series: series starting with [v2,1/3] drm/i915/csr: keep firmware name and required version together URL : https://patchwork.freedesktop.org/series/49269/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4782 -> Patchwork_10109 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10109 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10109, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49269/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10109: === IGT changes === Possible regressions igt@drv_selftest@live_hangcheck: fi-icl-u: PASS -> INCOMPLETE == Known issues == Here are the changes found in Patchwork_10109 that come from known issues: === IGT changes === Issues hit igt@debugfs_test@read_all_entries: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: PASS -> FAIL (fdo#103167) igt@kms_pipe_crc_basic@read-crc-pipe-a: fi-ilk-650: PASS -> DMESG-WARN (fdo#106387) +2 igt@prime_vgem@basic-fence-flip: fi-ilk-650: PASS -> FAIL (fdo#104008) Possible fixes igt@amdgpu/amd_cs_nop@sync-fork-gfx0: fi-kbl-8809g: DMESG-WARN (fdo#107762) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-7560u: FAIL (fdo#107336) -> PASS Warnings igt@amdgpu/amd_prime@amd-to-i915: fi-kbl-8809g: DMESG-FAIL (fdo#107762) -> FAIL (fdo#107341) fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341 fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762 == Participating hosts (52 -> 49) == Additional (2): fi-byt-j1900 fi-gdg-551 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4782 -> Patchwork_10109 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10109: 2507eb176574e43548bfe6a046126b80cc1f0046 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2507eb176574 drm/i915/csr: bypass firmware request on i915.dmc_firmware_path="" 227e00472dc5 drm/i915/csr: keep max firmware size together with firmare name and version 7d5b2b0a80f1 drm/i915/csr: keep firmware name and required version together == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10109/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [v2,1/2] drm/i915/icl: Avoid Gen10 watermark workarounds in Gen11 (rev2)
== Series Details == Series: series starting with [v2,1/2] drm/i915/icl: Avoid Gen10 watermark workarounds in Gen11 (rev2) URL : https://patchwork.freedesktop.org/series/49170/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4780_full -> Patchwork_10108_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10108_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10108_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_10108_full: === IGT changes === Possible regressions igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt: shard-snb: NOTRUN -> DMESG-FAIL Warnings igt@perf_pmu@rc6: shard-kbl: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10108_full that come from known issues: === IGT changes === Issues hit igt@gem_busy@extended-bsd: shard-snb: NOTRUN -> INCOMPLETE (fdo#105411) igt@kms_flip@2x-flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) Possible fixes igt@drv_suspend@shrink: shard-hsw: INCOMPLETE (fdo#103540, fdo#106886) -> PASS igt@gem_persistent_relocs@forked-interruptible-thrashing: shard-snb: INCOMPLETE (fdo#105411) -> PASS igt@kms_flip@flip-vs-expired-vblank: shard-glk: FAIL (fdo#102887, fdo#105363) -> PASS igt@kms_vblank@pipe-b-ts-continuation-modeset-rpm: shard-apl: DMESG-WARN (fdo#105602, fdo#103558) -> PASS +4 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4780 -> Patchwork_10108 CI_DRM_4780: de91607e0352454c2bcbaa8560214d95c70b8f75 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10108: 57b3d962912e95f7137280c67d79bc96f7e53d1f @ 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_10108/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/14] drm/amdgpu: Remove default best_encoder hook from DC
> -Original Message-
> From: Daniel Vetter
> Sent: Wednesday, September 5, 2018 9:48 AM
> To: Li, Sun peng (Leo)
> Cc: DRI Development ; Intel Graphics
> Development ; Daniel Vetter
> ; Deucher, Alexander
> ; Wentland, Harry
> ; Grodzovsky, Andrey
> ; Cheng, Tony ; S,
> Shirish
> Subject: Re: [PATCH 08/14] drm/amdgpu: Remove default best_encoder
> hook from DC
>
> On Wed, Sep 5, 2018 at 3:45 PM, Leo Li wrote:
> >
> >
> > On 2018-09-03 12:54 PM, Daniel Vetter wrote:
> >>
> >> For atomic driver this is the default, no need to reimplement it. We
> >> still need to keep the copypasta for not-atomic drivers though, since
> >> no one polished the legacy crtc helpers as much as the atomic ones.
> >
> >
> > Thanks for the patch! It seems I made an identical change here:
> > https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html
> >
> > One line difference though. I wasn't aware that the default is used
> > when .best_encoder is null, so I just hooked it to the default helper.
> >
> > If it's OK with you, I'll do a follow-up to drop the hook, so we don't
> > have two conflicting patches in flight.
>
> I have a follow-up patches to unexport the helper, so would be good if Alex
> sends out the pull request so I can sort this all out in drm-misc-next. I
> don't
> want to split up the patch series if possible.
> Or we just deal with the conflict, it's minor.
>
> Alex?
I'm planning to send the PR next week when I'm back in the office. I can drop
Leo's patch when I send the PR if that makes things easier.
Alex
> -Daniel
>
> > Leo
> >
> >
> >>
> >> Signed-off-by: Daniel Vetter
> >> Cc: Alex Deucher
> >> Cc: Harry Wentland
> >> Cc: Andrey Grodzovsky
> >> Cc: Tony Cheng
> >> Cc: "Leo (Sunpeng) Li"
> >> Cc: Shirish S
> >> ---
> >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23
> ---
> >> 1 file changed, 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> index af6adffba788..333f9904f135 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs
> >> amdgpu_dm_connector_funcs = {
> >> .atomic_get_property =
> amdgpu_dm_connector_atomic_get_property
> >> };
> >> -static struct drm_encoder *best_encoder(struct drm_connector
> >> *connector)
> >> -{
> >> - int enc_id = connector->encoder_ids[0];
> >> - struct drm_mode_object *obj;
> >> - struct drm_encoder *encoder;
> >> -
> >> - DRM_DEBUG_DRIVER("Finding the best encoder\n");
> >> -
> >> - /* pick the encoder ids */
> >> - if (enc_id) {
> >> - obj = drm_mode_object_find(connector->dev, NULL, enc_id,
> >> DRM_MODE_OBJECT_ENCODER);
> >> - if (!obj) {
> >> - DRM_ERROR("Couldn't find a matching encoder for
> >> our connector\n");
> >> - return NULL;
> >> - }
> >> - encoder = obj_to_encoder(obj);
> >> - return encoder;
> >> - }
> >> - DRM_ERROR("No encoder id\n");
> >> - return NULL;
> >> -}
> >> -
> >> static int get_modes(struct drm_connector *connector)
> >> {
> >> return amdgpu_dm_connector_get_modes(connector);
> >> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
> >> */
> >> .get_modes = get_modes,
> >> .mode_valid = amdgpu_dm_connector_mode_valid,
> >> - .best_encoder = best_encoder
> >> };
> >> static void dm_crtc_helper_disable(struct drm_crtc *crtc)
> >>
> >
>
>
>
> --
> 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
[Intel-gfx] [PATCH i-g-t] igt/gem_exec_capture: Capture many, many objects
Exercise O(N^2) behaviour in reading the error state, and push it to the
extreme.
Reported-by: Jason Ekstrand
Signed-off-by: Chris Wilson
---
lib/meson.build | 1 +
tests/gem_exec_capture.c | 320 ++-
tests/intel-ci/blacklist.txt | 1 +
3 files changed, 319 insertions(+), 3 deletions(-)
diff --git a/lib/meson.build b/lib/meson.build
index e60e5e02f..ce5465b63 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -66,6 +66,7 @@ lib_deps = [
math,
realtime,
ssl,
+ zlib
]
if libdrm_intel.found()
diff --git a/tests/gem_exec_capture.c b/tests/gem_exec_capture.c
index 2dc06ce43..2d0839468 100644
--- a/tests/gem_exec_capture.c
+++ b/tests/gem_exec_capture.c
@@ -21,8 +21,11 @@
* IN THE SOFTWARE.
*/
+#include
+
#include "igt.h"
#include "igt_device.h"
+#include "igt_rand.h"
#include "igt_sysfs.h"
#define LOCAL_OBJECT_CAPTURE (1 << 7)
@@ -54,10 +57,11 @@ static void check_error_state(int dir, struct
drm_i915_gem_exec_object2 *obj)
found = true;
}
+ free(error);
igt_assert(found);
}
-static void __capture(int fd, int dir, unsigned ring, uint32_t target)
+static void __capture1(int fd, int dir, unsigned ring, uint32_t target)
{
const int gen = intel_gen(intel_get_drm_devid(fd));
struct drm_i915_gem_exec_object2 obj[4];
@@ -167,10 +171,295 @@ static void capture(int fd, int dir, unsigned ring)
uint32_t handle;
handle = gem_create(fd, 4096);
- __capture(fd, dir, ring, handle);
+ __capture1(fd, dir, ring, handle);
gem_close(fd, handle);
}
+static uint64_t *__captureN(int fd, int dir, unsigned ring,
+ unsigned int size, int count,
+ unsigned int flags)
+#define INCREMENTAL 0x1
+{
+ const int gen = intel_gen(intel_get_drm_devid(fd));
+ struct drm_i915_gem_exec_object2 *obj;
+ struct drm_i915_gem_relocation_entry reloc[2];
+ struct drm_i915_gem_execbuffer2 execbuf;
+ uint32_t *batch, *seqno;
+ uint64_t *offsets;
+ int i;
+
+ offsets = calloc(count , sizeof(*offsets));
+ igt_assert(offsets);
+
+ obj = calloc(count + 2, sizeof(*obj));
+ igt_assert(obj);
+
+ obj[0].handle = gem_create(fd, 4096);
+ for (i = 0; i < count; i++) {
+ obj[i + 1].handle = gem_create(fd, size);
+ obj[i + 1].flags =
+ LOCAL_OBJECT_CAPTURE | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+ if (flags & INCREMENTAL) {
+ uint32_t *ptr;
+
+ ptr = gem_mmap__cpu(fd, obj[i + 1].handle,
+ 0, size, PROT_WRITE);
+ for (unsigned int n = 0; n < size / sizeof(*ptr); n++)
+ ptr[n] = i * size + n;
+ munmap(ptr, size);
+ }
+ }
+
+ obj[count + 1].handle = gem_create(fd, 4096);
+ obj[count + 1].relocs_ptr = (uintptr_t)reloc;
+ obj[count + 1].relocation_count = ARRAY_SIZE(reloc);
+
+ memset(reloc, 0, sizeof(reloc));
+ reloc[0].target_handle = obj[count + 1].handle; /* recurse */
+ reloc[0].presumed_offset = 0;
+ reloc[0].offset = 5*sizeof(uint32_t);
+ reloc[0].delta = 0;
+ reloc[0].read_domains = I915_GEM_DOMAIN_COMMAND;
+ reloc[0].write_domain = 0;
+
+ reloc[1].target_handle = obj[0].handle; /* breadcrumb */
+ reloc[1].presumed_offset = 0;
+ reloc[1].offset = sizeof(uint32_t);
+ reloc[1].delta = 0;
+ reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
+ reloc[1].write_domain = I915_GEM_DOMAIN_RENDER;
+
+ seqno = gem_mmap__wc(fd, obj[0].handle, 0, 4096, PROT_READ);
+ gem_set_domain(fd, obj[0].handle,
+ I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+
+ batch = gem_mmap__cpu(fd, obj[count + 1].handle, 0, 4096, PROT_WRITE);
+ gem_set_domain(fd, obj[count + 1].handle,
+ I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+ i = 0;
+ batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
+ if (gen >= 8) {
+ batch[++i] = 0;
+ batch[++i] = 0;
+ } else if (gen >= 4) {
+ batch[++i] = 0;
+ batch[++i] = 0;
+ reloc[1].offset += sizeof(uint32_t);
+ } else {
+ batch[i]--;
+ batch[++i] = 0;
+ }
+ batch[++i] = 0xc0ffee;
+ if (gen < 3)
+ batch[++i] = MI_NOOP;
+
+ batch[++i] = MI_BATCH_BUFFER_START; /* not crashed? try again! */
+ if (gen >= 8) {
+ batch[i] |= 1 << 8 | 1;
+ batch[++i] = 0;
+ batch[++i] = 0;
+ } else if (gen >= 6) {
+ batch[i] |= 1 << 8;
+ batch[++i] = 0;
+ } else {
+ batch[i] |= 2 << 6;
+ ba
Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake.
On Thu, Sep 06, 2018 at 09:46:13AM +0300, Jani Nikula wrote: > On Wed, 05 Sep 2018, Rodrigo Vivi wrote: > > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > >> Was not the decision that we only gate the MODULE_FIRMWARE line until > >> the firmware is in linux-firmware.git? > >> > >> So it should be no harm to support loading firmwares that are available > >> from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which > >> would trigger false positives for distro packagers. > > > > As far as I can remember we had agreed on changing this and adding > > MODULE_FIRMWARE from the beginning. > > > > 1. the process gets complex > > 2. we forgot many times to add it afterwards > > 3. module_firmware changes nothing... only the fact that initrd generation > >wont complain if firmware is not there yet. > > 4. The old issue was that patches were merged without the pull request > >being sent... We fixed that by only accepting patches after pull request > >is sent. > > 5. By the time that all lands to distros linux-firmware.git will have > > the firmware because of 4. > > 6. We have now drm-firmware to mirror what official linux-firmware.git will > > be in few weeks from now. > > > > So I don't see a reason anymore why to keep with complicated process with > > split MODULE_FIRMWARE. > > I've changed my mind, I agree with *not* splitting MODULE_FIRMWARE > changes to another patch anymore. > > However, with that I think we need to redefine when we can add > request_firmware and MODULE_FIRMWARE for a specific firmware > blob. Here's my proposal. > > - Before merging a patch adding or changing a firmware blob in dinq, > said blob must be publicly available at a known location, and no known > blockers for linux-firmware.git merge may exist. > > - We take care to get the blob to linux-firmware.git before the changes > hit Linus' -rc1 after the merge window. With our -rc5 feature > deadline, this should give us at least 4-5 weeks to get the blob > merged to linux-firmware.git. > > - It follows that linux-next and drm-tip may contain MODULE_FIRMWARE for > blobs that don't exist in linux-firmware.git. Agreed. And we should document this somewhere. > > 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
[Intel-gfx] ✗ Fi.CI.BAT: failure for igt/gem_exec_capture: Capture many, many objects
== Series Details == Series: igt/gem_exec_capture: Capture many, many objects URL : https://patchwork.freedesktop.org/series/49288/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4782 -> IGTPW_1804 = == Summary - FAILURE == Serious unknown changes coming with IGTPW_1804 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_1804, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49288/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in IGTPW_1804: === IGT changes === Possible regressions igt@pm_rpm@module-reload: fi-glk-j4005: PASS -> DMESG-FAIL == Known issues == Here are the changes found in IGTPW_1804 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106248, fdo#106725) Possible fixes igt@amdgpu/amd_cs_nop@sync-fork-gfx0: fi-kbl-8809g: DMESG-WARN (fdo#107762) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-7560u: FAIL (fdo#107336) -> PASS Warnings igt@amdgpu/amd_prime@amd-to-i915: fi-kbl-8809g: DMESG-FAIL (fdo#107762) -> FAIL (fdo#107341) fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248 fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341 fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762 == Participating hosts (52 -> 49) == Additional (2): fi-byt-j1900 fi-gdg-551 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * IGT: IGT_4632 -> IGTPW_1804 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_1804: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1804/ IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Testlist changes == +igt@gem_exec_capture@many-2m-incremental +igt@gem_exec_capture@many-2m-zero +igt@gem_exec_capture@many-4k-incremental +igt@gem_exec_capture@many-4k-zero +igt@gem_exec_capture@many-256m-incremental == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1804/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Limit number of capture objects
If we fail to allocate an array for a large number of user requested
capture objects, reduce the array size and try to grab at least some of
the objects!
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gpu_error.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index f7f2aa71d8d9..b28f753f7293 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1365,15 +1365,20 @@ static void request_record_user_bo(struct i915_request
*request,
{
struct i915_capture_list *c;
struct drm_i915_error_object **bo;
- long count;
+ long count, max;
- count = 0;
+ max = 0;
for (c = request->capture_list; c; c = c->next)
- count++;
+ max++;
+ if (!max)
+ return;
- bo = NULL;
- if (count)
- bo = kcalloc(count, sizeof(*bo), GFP_ATOMIC);
+ bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
+ if (!bo) {
+ /* If we can't capture everything, try to capture something. */
+ max = min_t(long, max, PAGE_SIZE/sizeof(*bo));
+ bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
+ }
if (!bo)
return;
@@ -1382,7 +1387,8 @@ static void request_record_user_bo(struct i915_request
*request,
bo[count] = i915_error_object_create(request->i915, c->vma);
if (!bo[count])
break;
- count++;
+ if (++count == max)
+ break;
}
ee->user_bo = bo;
--
2.19.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
If the caller supplies more than 4G of objects and than one that has to be in the low 4G, it is possible for the low 4G to be full before we attempt to find room for the last object that must be there. As we don't reorder the two types, every pass hits the same problem and we fail with ENOSPC. However, if we impose a little bit of ordering between the two classes of objects, on the second pass we will be able to fit the special object as we do it first. For setups that only use !48b objects, we now reverse the order between passes, hopefully making the subsequent passes more likely to succeed given that we are trying a different order (rather than repeating the previous pass!) Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 22b4cb775576..d70d142f5338 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -696,6 +696,8 @@ static int eb_reserve(struct i915_execbuffer *eb) list_add(&vma->exec_link, &eb->unbound); else if (flags & __EXEC_OBJECT_NEEDS_MAP) list_add_tail(&vma->exec_link, &eb->unbound); + else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) + list_add(&vma->exec_link, &last); else list_add_tail(&vma->exec_link, &last); } -- 2.19.0.rc2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
== Series Details == Series: series starting with [1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G URL : https://patchwork.freedesktop.org/series/49290/ State : warning == Summary == $ dim checkpatch origin/drm-tip d1fd66113f8d drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G 28b603c0986d drm/i915: Limit number of capture objects -:37: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV) #37: FILE: drivers/gpu/drm/i915/i915_gpu_error.c:1379: + max = min_t(long, max, PAGE_SIZE/sizeof(*bo)); ^ total: 0 errors, 0 warnings, 1 checks, 35 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
== Series Details == Series: series starting with [1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G URL : https://patchwork.freedesktop.org/series/49290/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G Okay! Commit: drm/i915: Limit number of capture objects +./include/linux/slab.h:631:13: error: not a function ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
== Series Details == Series: series starting with [1/2] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G URL : https://patchwork.freedesktop.org/series/49290/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4782 -> Patchwork_10110 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10110 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10110, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49290/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10110: === IGT changes === Possible regressions igt@drv_selftest@live_requests: fi-byt-j1900: NOTRUN -> DMESG-WARN Warnings igt@pm_rpm@module-reload: fi-hsw-4770r: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10110 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload-inject: fi-hsw-4770r: PASS -> DMESG-WARN (fdo#107425) igt@gem_exec_suspend@basic-s4-devices: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_flip@basic-plain-flip: fi-ilk-650: PASS -> DMESG-WARN (fdo#106387) +1 igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: PASS -> FAIL (fdo#103167) igt@kms_pipe_crc_basic@hang-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) Possible fixes igt@amdgpu/amd_cs_nop@sync-fork-gfx0: fi-kbl-8809g: DMESG-WARN (fdo#107762) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-7560u: FAIL (fdo#107336) -> PASS Warnings igt@amdgpu/amd_prime@amd-to-i915: fi-kbl-8809g: DMESG-FAIL (fdo#107762) -> FAIL (fdo#107341) fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762 == Participating hosts (52 -> 49) == Additional (2): fi-byt-j1900 fi-gdg-551 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4782 -> Patchwork_10110 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10110: 28b603c0986d7d0768998240c52fd299aa6fa608 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 28b603c0986d drm/i915: Limit number of capture objects d1fd66113f8d drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10110/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax
From: Kai Chen
On GEN9LP, raise the RPS FUp Interrupt Limiter above softmax so that the
HW won't miss interrupt when requested max_freq is set back to RP0
value.
Signed-off-by: Kai Chen
---
drivers/gpu/drm/i915/intel_pm.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d99e5fabe93c..bf2494294c9d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6276,7 +6276,20 @@ static u32 intel_rps_limits(struct drm_i915_private
*dev_priv, u8 val)
* frequency, if the down threshold expires in that window we will not
* receive a down interrupt. */
if (INTEL_GEN(dev_priv) >= 9) {
- limits = (rps->max_freq_softlimit) << 23;
+ int max_freq = rps->max_freq_softlimit;
+ int rp0_freq = rps->rp0_freq;
+
+ if (IS_GEN9_LP(dev_priv) && (max_freq == rp0_freq))
+ /*
+* For GEN9_LP, it is suggested to increase the upper
+* interrupt limiter by 1 (16.6MHz) so that the HW will
+* generate an interrupt when we are near or just below
+* the upper limit.
+*/
+ limits = (rps->max_freq_softlimit + 1) << 23;
+ else
+ limits = (rps->max_freq_softlimit) << 23;
+
if (val <= rps->min_freq_softlimit)
limits |= (rps->min_freq_softlimit) << 14;
} else {
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax
== Series Details == Series: drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax URL : https://patchwork.freedesktop.org/series/49293/ State : warning == Summary == $ dim checkpatch origin/drm-tip e67dcf0dbc2b drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax -:25: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'max_freq == rp0_freq' #25: FILE: drivers/gpu/drm/i915/intel_pm.c:6282: + if (IS_GEN9_LP(dev_priv) && (max_freq == rp0_freq)) total: 0 errors, 0 warnings, 1 checks, 21 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax
Quoting kai.c...@intel.com (2018-09-06 19:19:02)
> From: Kai Chen
>
> On GEN9LP, raise the RPS FUp Interrupt Limiter above softmax so that the
> HW won't miss interrupt when requested max_freq is set back to RP0
> value.
>
> Signed-off-by: Kai Chen
> ---
> drivers/gpu/drm/i915/intel_pm.c | 15 ++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d99e5fabe93c..bf2494294c9d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6276,7 +6276,20 @@ static u32 intel_rps_limits(struct drm_i915_private
> *dev_priv, u8 val)
> * frequency, if the down threshold expires in that window we will not
> * receive a down interrupt. */
> if (INTEL_GEN(dev_priv) >= 9) {
> - limits = (rps->max_freq_softlimit) << 23;
> + int max_freq = rps->max_freq_softlimit;
> + int rp0_freq = rps->rp0_freq;
> +
> + if (IS_GEN9_LP(dev_priv) && (max_freq == rp0_freq))
> + /*
> +* For GEN9_LP, it is suggested to increase the upper
> +* interrupt limiter by 1 (16.6MHz) so that the HW
> will
> +* generate an interrupt when we are near or just
> below
> +* the upper limit.
By that explanation there is nothing peculiar to rp0. If the HW
calculation is unstable, it is unstable. One can postulate any number of
rounding errors that could cause an obo error, so without a better
explanation, one would say just increase it by one always.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915/guc: Update GuC power domain states
We should update GuC power domain states also when GuC submission
is disabled, otherwise GuC might complain or ignore our requests.
This seems to be required for all currently released GuC firmwares.
v2: it is only needed by pre-Gen11 firmwares
Signed-off-by: Michal Wajdeczko
Cc: John Spotswood
Cc: Anusha Srivatsa
Cc: Tomasz Lis
Cc: Daniele Ceraolo Spurio
---
drivers/gpu/drm/i915/intel_uc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 7c95697..b1b3e81 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -401,6 +401,10 @@ int intel_uc_init_hw(struct drm_i915_private *i915)
ret = intel_guc_submission_enable(guc);
if (ret)
goto err_communication;
+ } else if (INTEL_GEN(i915) < 11) {
+ ret = intel_guc_sample_forcewake(guc);
+ if (ret)
+ goto err_communication;
}
dev_info(i915->drm.dev, "GuC firmware version %u.%u\n",
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] HAX Enable HuC testing without GuC submission
This will let the driver decide where GuC can be used Signed-off-by: Michal Wajdeczko --- drivers/gpu/drm/i915/i915_params.h | 2 +- drivers/gpu/drm/i915/intel_uc.c| 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 6c4d4a2..323fdc2 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -46,7 +46,7 @@ param(int, disable_power_well, -1) \ param(int, enable_ips, 1) \ param(int, invert_brightness, 0) \ - param(int, enable_guc, 0) \ + param(int, enable_guc, -1) \ param(int, guc_log_level, -1) \ param(char *, guc_firmware_path, NULL) \ param(char *, huc_firmware_path, NULL) \ diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index b1b3e81..87be3c7 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -64,6 +64,9 @@ static int __get_platform_enable_guc(struct drm_i915_private *i915) /* Any platform specific fine-tuning can be done here */ + /* HAX: Do not enable GuC submission in auto mode */ + enable_guc &= ~ENABLE_GUC_SUBMISSION; + return enable_guc; } -- 1.9.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/overlay: Allocate physical registers from stolen
Given that we are now reasonably confident in our ability to detect and
reserve the stolen memory (physical memory reserved for graphics by the
BIOS) for ourselves on most machines, we can put it to use. In this
case, we need a page to hold the overlay registers.
On an i915g running MythTv, H Buus noticed that
commit 6a2c4232ece145d8b5a8f95f767bd6d0d2d2f2bb
Author: Chris Wilson
Date: Tue Nov 4 04:51:40 2014 -0800
drm/i915: Make the physical object coherent with GTT
introduced stuttering into his video playback. After discarding the
likely suspect of it being the physical cursor updates, we were left
with the use of the phys object for the overlay. And lo, if we
completely avoid using the phys object (allocated just once on module
load!) by switching to stolen memory, the stuttering goes away.
For lack of a better explanation, claim victory and kill two birds with
one stone.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600
Fixes: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 228 +--
1 file changed, 75 insertions(+), 153 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index c2f10d899329..443dfaefd7a6 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -181,8 +181,9 @@ struct intel_overlay {
u32 brightness, contrast, saturation;
u32 old_xscale, old_yscale;
/* register access */
- u32 flip_addr;
struct drm_i915_gem_object *reg_bo;
+ struct overlay_registers __iomem *regs;
+ u32 flip_addr;
/* flip handling */
struct i915_gem_active last_flip;
};
@@ -210,29 +211,6 @@ static void i830_overlay_clock_gating(struct
drm_i915_private *dev_priv,
PCI_DEVFN(0, 0), I830_CLOCK_GATE, val);
}
-static struct overlay_registers __iomem *
-intel_overlay_map_regs(struct intel_overlay *overlay)
-{
- struct drm_i915_private *dev_priv = overlay->i915;
- struct overlay_registers __iomem *regs;
-
- if (OVERLAY_NEEDS_PHYSICAL(dev_priv))
- regs = (struct overlay_registers __iomem
*)overlay->reg_bo->phys_handle->vaddr;
- else
- regs = io_mapping_map_wc(&dev_priv->ggtt.iomap,
-overlay->flip_addr,
-PAGE_SIZE);
-
- return regs;
-}
-
-static void intel_overlay_unmap_regs(struct intel_overlay *overlay,
-struct overlay_registers __iomem *regs)
-{
- if (!OVERLAY_NEEDS_PHYSICAL(overlay->i915))
- io_mapping_unmap(regs);
-}
-
static void intel_overlay_submit_request(struct intel_overlay *overlay,
struct i915_request *rq,
i915_gem_retire_fn retire)
@@ -784,13 +762,13 @@ static int intel_overlay_do_put_image(struct
intel_overlay *overlay,
struct drm_i915_gem_object *new_bo,
struct put_image_params *params)
{
- int ret, tmp_width;
- struct overlay_registers __iomem *regs;
- bool scale_changed = false;
+ struct overlay_registers __iomem *regs = overlay->regs;
struct drm_i915_private *dev_priv = overlay->i915;
u32 swidth, swidthsw, sheight, ostride;
enum pipe pipe = overlay->crtc->pipe;
+ bool scale_changed = false;
struct i915_vma *vma;
+ int ret, tmp_width;
lockdep_assert_held(&dev_priv->drm.struct_mutex);
WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
@@ -815,30 +793,19 @@ static int intel_overlay_do_put_image(struct
intel_overlay *overlay,
if (!overlay->active) {
u32 oconfig;
- regs = intel_overlay_map_regs(overlay);
- if (!regs) {
- ret = -ENOMEM;
- goto out_unpin;
- }
+
oconfig = OCONF_CC_OUT_8BIT;
if (IS_GEN4(dev_priv))
oconfig |= OCONF_CSC_MODE_BT709;
oconfig |= pipe == 0 ?
OCONF_PIPE_A : OCONF_PIPE_B;
iowrite32(oconfig, ®s->OCONFIG);
- intel_overlay_unmap_regs(overlay, regs);
ret = intel_overlay_on(overlay);
if (ret != 0)
goto out_unpin;
}
- regs = intel_overlay_map_regs(overlay);
- if (!regs) {
- ret = -ENOMEM;
- goto out_unpin;
- }
-
iowrite32((params->dst_y << 16) | params->dst_x, ®s->DWINPOS);
iowrite32((params->dst_h << 16) | params->dst_w, ®s->DWINSZ);
@@ -882,8 +849,6 @@ static int intel_overlay_do_put_image(struct i
[Intel-gfx] [PATCH 2/2] drm/i915/overlay: Use the ioctl parameters directly
The user parameters to put_image are not copied back to userspace
(DRM_IOW), and so we can modify the ioctl parameters (having already been
copied to a temporary kernel struct) directly and use those in place,
avoiding another temporary malloc and lots of manual copying.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_overlay.c | 147 ++-
1 file changed, 54 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 443dfaefd7a6..72eb7e48e8bc 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -487,23 +487,6 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv)
overlay->active = false;
}
-struct put_image_params {
- int format;
- short dst_x;
- short dst_y;
- short dst_w;
- short dst_h;
- short src_w;
- short src_scan_h;
- short src_scan_w;
- short src_h;
- short stride_Y;
- short stride_UV;
- int offset_Y;
- int offset_U;
- int offset_V;
-};
-
static int packed_depth_bytes(u32 format)
{
switch (format & I915_OVERLAY_DEPTH_MASK) {
@@ -618,25 +601,25 @@ static void update_polyphase_filter(struct
overlay_registers __iomem *regs)
static bool update_scaling_factors(struct intel_overlay *overlay,
struct overlay_registers __iomem *regs,
- struct put_image_params *params)
+ struct drm_intel_overlay_put_image *params)
{
/* fixed point with a 12 bit shift */
u32 xscale, yscale, xscale_UV, yscale_UV;
#define FP_SHIFT 12
#define FRACT_MASK 0xfff
bool scale_changed = false;
- int uv_hscale = uv_hsubsampling(params->format);
- int uv_vscale = uv_vsubsampling(params->format);
+ int uv_hscale = uv_hsubsampling(params->flags);
+ int uv_vscale = uv_vsubsampling(params->flags);
- if (params->dst_w > 1)
- xscale = ((params->src_scan_w - 1) << FP_SHIFT)
- /(params->dst_w);
+ if (params->dst_width > 1)
+ xscale = ((params->src_scan_width - 1) << FP_SHIFT) /
+ params->dst_width;
else
xscale = 1 << FP_SHIFT;
- if (params->dst_h > 1)
- yscale = ((params->src_scan_h - 1) << FP_SHIFT)
- /(params->dst_h);
+ if (params->dst_height > 1)
+ yscale = ((params->src_scan_height - 1) << FP_SHIFT) /
+ params->dst_height;
else
yscale = 1 << FP_SHIFT;
@@ -713,12 +696,12 @@ static void update_colorkey(struct intel_overlay *overlay,
iowrite32(flags, ®s->DCLRKM);
}
-static u32 overlay_cmd_reg(struct put_image_params *params)
+static u32 overlay_cmd_reg(struct drm_intel_overlay_put_image *params)
{
u32 cmd = OCMD_ENABLE | OCMD_BUF_TYPE_FRAME | OCMD_BUFFER0;
- if (params->format & I915_OVERLAY_YUV_PLANAR) {
- switch (params->format & I915_OVERLAY_DEPTH_MASK) {
+ if (params->flags & I915_OVERLAY_YUV_PLANAR) {
+ switch (params->flags & I915_OVERLAY_DEPTH_MASK) {
case I915_OVERLAY_YUV422:
cmd |= OCMD_YUV_422_PLANAR;
break;
@@ -731,7 +714,7 @@ static u32 overlay_cmd_reg(struct put_image_params *params)
break;
}
} else { /* YUV packed */
- switch (params->format & I915_OVERLAY_DEPTH_MASK) {
+ switch (params->flags & I915_OVERLAY_DEPTH_MASK) {
case I915_OVERLAY_YUV422:
cmd |= OCMD_YUV_422_PACKED;
break;
@@ -740,7 +723,7 @@ static u32 overlay_cmd_reg(struct put_image_params *params)
break;
}
- switch (params->format & I915_OVERLAY_SWAP_MASK) {
+ switch (params->flags & I915_OVERLAY_SWAP_MASK) {
case I915_OVERLAY_NO_SWAP:
break;
case I915_OVERLAY_UV_SWAP:
@@ -760,7 +743,7 @@ static u32 overlay_cmd_reg(struct put_image_params *params)
static int intel_overlay_do_put_image(struct intel_overlay *overlay,
struct drm_i915_gem_object *new_bo,
- struct put_image_params *params)
+ struct drm_intel_overlay_put_image
*params)
{
struct overlay_registers __iomem *regs = overlay->regs;
struct drm_i915_private *dev_priv = overlay->i915;
@@ -806,35 +789,40 @@ static int intel_overlay_do_put_image(struct
intel_overlay *overlay,
goto out_unpin;
}
- iowrite32((params->dst_y << 16) | params->dst_x, ®s->DWINPOS);
- iowrite32((params->dst_h << 16) | params->dst_w, ®s->DWINSZ);
+ iow
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax
== Series Details == Series: drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax URL : https://patchwork.freedesktop.org/series/49293/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4782 -> Patchwork_10111 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49293/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10111 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_coherency: fi-gdg-551: NOTRUN -> DMESG-FAIL (fdo#107164) igt@gem_exec_suspend@basic-s3: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: PASS -> FAIL (fdo#103167) igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: 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) igt@prime_vgem@basic-fence-flip: fi-ilk-650: PASS -> FAIL (fdo#104008) Possible fixes igt@amdgpu/amd_cs_nop@sync-fork-gfx0: fi-kbl-8809g: DMESG-WARN (fdo#107762) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-7560u: FAIL (fdo#107336) -> PASS Warnings igt@amdgpu/amd_prime@amd-to-i915: fi-kbl-8809g: DMESG-FAIL (fdo#107762) -> FAIL (fdo#107341) fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762 == Participating hosts (52 -> 48) == Additional (2): fi-byt-j1900 fi-gdg-551 Missing(6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4782 -> Patchwork_10111 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10111: e67dcf0dbc2bbde9872cb385bd50dd38c695f2e4 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == e67dcf0dbc2b drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10111/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 series starting with [v2,1/2] drm/i915/guc: Update GuC power domain states
== Series Details == Series: series starting with [v2,1/2] drm/i915/guc: Update GuC power domain states URL : https://patchwork.freedesktop.org/series/49294/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4782 -> Patchwork_10112 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10112 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10112, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49294/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10112: === IGT changes === Possible regressions igt@drv_selftest@live_hangcheck: fi-skl-gvtdvm: PASS -> DMESG-FAIL == Known issues == Here are the changes found in Patchwork_10112 that come from known issues: === IGT changes === Issues hit igt@amdgpu/amd_basic@userptr: fi-kbl-8809g: PASS -> INCOMPLETE (fdo#107402) igt@drv_selftest@live_evict: fi-bsw-kefka: PASS -> DMESG-WARN (fdo#107709) igt@drv_selftest@live_hangcheck: fi-skl-6770hq: PASS -> DMESG-FAIL (fdo#107174) igt@gem_exec_suspend@basic-s3: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_chamelium@dp-crc-fast: fi-kbl-7500u: PASS -> FAIL (fdo#103841) igt@kms_frontbuffer_tracking@basic: fi-icl-u: SKIP -> FAIL (fdo#103167) fi-byt-clapper: PASS -> FAIL (fdo#103167) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927) Possible fixes igt@kms_psr@primary_page_flip: fi-kbl-7560u: FAIL (fdo#107336) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402 fdo#107709 https://bugs.freedesktop.org/show_bug.cgi?id=107709 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 == Participating hosts (52 -> 49) == Additional (2): fi-byt-j1900 fi-gdg-551 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4782 -> Patchwork_10112 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10112: ee9aa96d5c38e5114ab5263bf3bc76b157d29d9a @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == ee9aa96d5c38 HAX Enable HuC testing without GuC submission d1f9bbe76ce7 drm/i915/guc: Update GuC power domain states == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10112/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 [1/2] drm/i915/overlay: Allocate physical registers from stolen
== Series Details ==
Series: series starting with [1/2] drm/i915/overlay: Allocate physical
registers from stolen
URL : https://patchwork.freedesktop.org/series/49295/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
35d7413847d3 drm/i915/overlay: Allocate physical registers from stolen
-:16: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit 6a2c4232ece1 ("drm/i915: Make
the physical object coherent with GTT")'
#16:
commit 6a2c4232ece145d8b5a8f95f767bd6d0d2d2f2bb
-:193: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!obj"
#193: FILE: drivers/gpu/drm/i915/intel_overlay.c:1349:
+ if (obj == NULL)
total: 1 errors, 0 warnings, 1 checks, 358 lines checked
564d9d661882 drm/i915/overlay: Use the ioctl parameters directly
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/overlay: Allocate physical registers from stolen
== Series Details == Series: series starting with [1/2] drm/i915/overlay: Allocate physical registers from stolen URL : https://patchwork.freedesktop.org/series/49295/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/overlay: Allocate physical registers from stolen Okay! Commit: drm/i915/overlay: Use the ioctl parameters directly -O:drivers/gpu/drm/i915/intel_overlay.c:832:29: warning: expression using sizeof(void) -O:drivers/gpu/drm/i915/intel_overlay.c:832:29: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_overlay.c:819:29: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_overlay.c:819:29: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v11 0/7] Per context dynamic (sub)slice power-gating
Quoting Tvrtko Ursulin (2018-09-05 15:22:15) > From: Tvrtko Ursulin > > Updated series after continuing Lionel's work. > > Userspace for the feature is the media-driver project on GitHub. Please see > https://github.com/intel/media-driver/pull/271/commits. > > No headline changes this time. > > Some review feedback, some refactoring, some patches got merged and two new > appeared to help with the simplified implementation and also lock SSEU config > to a workable set on Icelake. Scanning through the buglist, caught this little gem https://bugs.freedesktop.org/show_bug.cgi?id=103484 in which Lionel mentioned that he wanted to fix up as part of this series. The conclusion is that we can remove the i915_sseu_status debugfs if we are happy with the testing and runtime adjustment. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v11 0/7] Per context dynamic (sub)slice power-gating
Quoting Chris Wilson (2018-09-06 20:33:35) > Quoting Tvrtko Ursulin (2018-09-05 15:22:15) > > From: Tvrtko Ursulin > > > > Updated series after continuing Lionel's work. > > > > Userspace for the feature is the media-driver project on GitHub. Please see > > https://github.com/intel/media-driver/pull/271/commits. > > > > No headline changes this time. > > > > Some review feedback, some refactoring, some patches got merged and two new > > appeared to help with the simplified implementation and also lock SSEU > > config > > to a workable set on Icelake. > > Scanning through the buglist, caught this little gem > https://bugs.freedesktop.org/show_bug.cgi?id=103484 > in which Lionel mentioned that he wanted to fix up as part of this > series. The conclusion is that we can remove the i915_sseu_status > debugfs if we are happy with the testing and runtime adjustment. Please also note https://bugs.freedesktop.org/show_bug.cgi?id=100899 in the changelog somewhere and update when done. Thanks, -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/overlay: Allocate physical registers from stolen
== Series Details == Series: series starting with [1/2] drm/i915/overlay: Allocate physical registers from stolen URL : https://patchwork.freedesktop.org/series/49295/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4782 -> Patchwork_10113 = == Summary - WARNING == Minor unknown changes coming with Patchwork_10113 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10113, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49295/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10113: === IGT changes === Warnings igt@pm_rpm@module-reload: fi-hsw-4770r: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10113 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload-inject: fi-hsw-4770r: PASS -> DMESG-WARN (fdo#107425) igt@kms_chamelium@hdmi-edid-read: fi-kbl-7500u: SKIP -> FAIL (fdo#103841) igt@kms_chamelium@hdmi-hpd-fast: fi-kbl-7500u: SKIP -> FAIL (fdo#102672, fdo#103841) igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b: fi-byt-clapper: PASS -> FAIL (fdo#107362) Possible fixes igt@amdgpu/amd_cs_nop@sync-fork-gfx0: fi-kbl-8809g: DMESG-WARN (fdo#107762) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-7560u: FAIL (fdo#107336) -> PASS Warnings igt@amdgpu/amd_prime@amd-to-i915: fi-kbl-8809g: DMESG-FAIL (fdo#107762) -> FAIL (fdo#107341) fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425 fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762 == Participating hosts (52 -> 49) == Additional (2): fi-byt-j1900 fi-gdg-551 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4782 -> Patchwork_10113 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10113: 564d9d66188242e3afbe3b0016a545dfb277f094 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 564d9d661882 drm/i915/overlay: Use the ioctl parameters directly 35d7413847d3 drm/i915/overlay: Allocate physical registers from stolen == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10113/issues.html ___ 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: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax
== Series Details == Series: drm/i915: Raise RPS FUp Interrupt Limiter for GEN9LP above softmax URL : https://patchwork.freedesktop.org/series/49293/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4782_full -> Patchwork_10111_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10111_full that come from known issues: === IGT changes === Issues hit igt@kms_frontbuffer_tracking@fbc-suspend: shard-apl: PASS -> DMESG-WARN (fdo#103558) igt@kms_vblank@pipe-b-ts-continuation-modeset-rpm: shard-apl: PASS -> DMESG-WARN (fdo#105602, fdo#103558) +5 igt@pm_rpm@system-suspend: shard-apl: PASS -> DMESG-WARN (fdo#103841) Possible fixes igt@drv_suspend@shrink: shard-snb: INCOMPLETE (fdo#106886, fdo#105411) -> PASS shard-glk: FAIL (fdo#106886) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4782 -> Patchwork_10111 CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10111: e67dcf0dbc2bbde9872cb385bd50dd38c695f2e4 @ 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_10111/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v2)
48 bit ppgtt device configuration is really just extended address
range full ppgtt and may actually be something other than 48 bits.
Change USES_FULL_48BIT_PPGTT() to USES_FULL_4LVL_PPGTT() to better
describe that a 4 level walk table extended range PPGTT is being
used. Add a new device info field that specifies the number of
bits to prepare for cases where the range is not 32 or 48 bits.
v2: keep USES_FULL_PPGTT() unchanged (Chris)
Signed-off-by: Bob Paauwe
CC: Rodrigo Vivi
CC: Michel Thierry
CC: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 19 ++-
drivers/gpu/drm/i915/i915_pci.c | 7 +--
drivers/gpu/drm/i915/intel_device_info.h | 3 +++
drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +-
drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++
6 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5a4da5b723fd..a367686fd735 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2569,7 +2569,7 @@ intel_info(const struct drm_i915_private *dev_priv)
#define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt)
#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2)
-#define USES_FULL_48BIT_PPGTT(dev_priv)(i915_modparams.enable_ppgtt ==
3)
+#define USES_FULL_4LVL_PPGTT(dev_priv) ((dev_priv)->info.full_ppgtt_bits > 32)
#define HAS_PAGE_SIZES(dev_priv, sizes) ({ \
GEM_BUG_ON((sizes) == 0); \
((sizes) & ~(dev_priv)->info.page_sizes) == 0; \
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index eb0e446d6482..530a4c1452b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -137,18 +137,18 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private
*dev_priv,
int enable_ppgtt)
{
bool has_full_ppgtt;
- bool has_full_48bit_ppgtt;
+ bool has_full_4lvl_ppgtt;
if (!dev_priv->info.has_aliasing_ppgtt)
return 0;
has_full_ppgtt = dev_priv->info.has_full_ppgtt;
- has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt;
+ has_full_4lvl_ppgtt = USES_FULL_4LVL_PPGTT(dev_priv);
if (intel_vgpu_active(dev_priv)) {
/* GVT-g has no support for 32bit ppgtt */
has_full_ppgtt = false;
- has_full_48bit_ppgtt =
intel_vgpu_has_full_48bit_ppgtt(dev_priv);
+ has_full_4lvl_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv);
}
/*
@@ -164,7 +164,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private
*dev_priv,
if (enable_ppgtt == 2 && has_full_ppgtt)
return 2;
- if (enable_ppgtt == 3 && has_full_48bit_ppgtt)
+ if (enable_ppgtt == 3 && has_full_4lvl_ppgtt)
return 3;
/* Disable ppgtt on SNB if VT-d is on. */
@@ -173,7 +173,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private
*dev_priv,
return 0;
}
- if (has_full_48bit_ppgtt)
+ if (has_full_4lvl_ppgtt)
return 3;
if (has_full_ppgtt)
@@ -1647,9 +1647,10 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct
drm_i915_private *i915)
ppgtt->vm.i915 = i915;
ppgtt->vm.dma = &i915->drm.pdev->dev;
- ppgtt->vm.total = USES_FULL_48BIT_PPGTT(i915) ?
- 1ULL << 48 :
- 1ULL << 32;
+ if ((i915_modparams.enable_ppgtt < 3) && USES_FULL_4LVL_PPGTT(i915))
+ ppgtt->vm.total = 1ULL << 32;
+ else
+ ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits;
/*
* From bdw, there is support for read-only pages in the PPGTT.
@@ -1788,7 +1789,7 @@ static void gen8_ppgtt_enable(struct drm_i915_private
*dev_priv)
enum intel_engine_id id;
for_each_engine(engine, dev_priv, id) {
- u32 four_level = USES_FULL_48BIT_PPGTT(dev_priv) ?
+ u32 four_level = USES_FULL_4LVL_PPGTT(dev_priv) ?
GEN8_GFX_PPGTT_48B : 0;
I915_WRITE(RING_MODE_GEN7(engine),
_MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level));
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index d6f7b9fe1d26..a99c1f6de64e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -299,6 +299,7 @@ static const struct intel_device_info
intel_sandybridge_m_gt2_info = {
.has_rc6p = 1, \
.has_aliasing_ppgtt = 1, \
.has_full_ppgtt = 1, \
+ .full_ppgtt_bits = 32, \
GEN_DEFAULT_PIPEOFFSETS, \
GEN_DEFAULT_PAGE_SIZES, \
IVB_CURSOR_OFFSETS
@@ -353,6 +354,7 @@ static const struct intel_device_info intel_valleyview_info
= {
.has
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v2)
Quoting Bob Paauwe (2018-09-06 21:04:09) > @@ -1647,9 +1647,10 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct > drm_i915_private *i915) > ppgtt->vm.i915 = i915; > ppgtt->vm.dma = &i915->drm.pdev->dev; > > - ppgtt->vm.total = USES_FULL_48BIT_PPGTT(i915) ? > - 1ULL << 48 : > - 1ULL << 32; > + if ((i915_modparams.enable_ppgtt < 3) && USES_FULL_4LVL_PPGTT(i915)) (brackets (because(?)) > + ppgtt->vm.total = 1ULL << 32; > + else > + ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits; How about ppgtt->vm.total = BIT_ULL(i915->info.full_ppgtt_bits); if (i915_modparams.enable_ppgtt < 3) ppgtt->vm.total = min(ppgtt->vm.total, SZ_4G); Although let me complain loudly about introducing more modparams. Please no. If you want to configure it, do so at runtime via context parameters or creation. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Rename full ppgtt configuration to be more generic (rev2)
== Series Details == Series: drm/i915: Rename full ppgtt configuration to be more generic (rev2) URL : https://patchwork.freedesktop.org/series/49021/ State : warning == Summary == $ dim checkpatch origin/drm-tip 6e4a835ff874 drm/i915: Make 48bit full ppgtt configuration generic (v2) -:85: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'i915_modparams.enable_ppgtt < 3' #85: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:1650: + if ((i915_modparams.enable_ppgtt < 3) && USES_FULL_4LVL_PPGTT(i915)) -:117: WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations #117: FILE: drivers/gpu/drm/i915/i915_pci.c:357: + .full_ppgtt_bits = 32, \ -:134: WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations #134: FILE: drivers/gpu/drm/i915/i915_pci.c:450: + .full_ppgtt_bits = 32, \ total: 0 errors, 2 warnings, 1 checks, 128 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/2] drm: Add detection of changing of edid on between suspend and resume
The hotplug detection routine of drm_helper_hpd_irq_event() can detect
changing of status of connector, but it can not detect changing of edid.
Following scenario requires detection of changing of edid.
1) plug display device to a connector
2) system suspend
3) unplug 1)'s display device and plug the other display device to a
connector
4) system resume
It adds edid check routine when a connector status still remains as
"connector_status_connected".
v2: Add NULL check before comparing of EDIDs.
v3: Make it as part of existing drm_helper_hpd_irq_event() (Stan, Mika)
Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
Signed-off-by: Gwan-gyeong Mun
---
drivers/gpu/drm/drm_probe_helper.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index a1bb157bfdfa..2705a5a0e4d6 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -742,7 +742,16 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini);
* panels.
*
* This helper function is useful for drivers which can't or don't track
hotplug
- * interrupts for each connector.
+ * interrupts for each connector. And it also supports a detection of changing
+ * of edid on between suspend and resume when a connector status still remains
+ * as "connector_status_connected".
+ *
+ * Following scenario requires detection of changing of edid.
+ * 1) plug display device to a connector
+ * 2) system suspend
+ * 3) unplug 1)'s display device and plug the other display device to a
+ connector
+ * 4) system resume
*
* Drivers which support hotplug interrupts for each connector individually and
* which have a more fine-grained detect logic should bypass this code and
@@ -760,6 +769,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
enum drm_connector_status old_status;
+ struct edid *old_edid;
bool changed = false;
if (!dev->mode_config.poll_enabled)
@@ -773,6 +783,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
continue;
old_status = connector->status;
+ old_edid = connector->detect_edid;
connector->status = drm_helper_probe_detect(connector, NULL,
false);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
%s\n",
@@ -782,6 +793,22 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
drm_get_connector_status_name(connector->status));
if (old_status != connector->status)
changed = true;
+
+ /* Check changing of edid when a connector status still remains
+* as "connector_status_connected".
+*/
+ if (old_status == connector->status &&
+ old_status == connector_status_connected) {
+ if (!old_edid || !connector->detect_edid)
+ continue;
+
+ if (memcmp(old_edid, connector->detect_edid,
sizeof(*old_edid))) {
+ changed = true;
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] edid
updated\n",
+ connector->base.id,
+ connector->name);
+ }
+ }
}
drm_connector_list_iter_end(&conn_iter);
mutex_unlock(&dev->mode_config.mutex);
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/2] drm: move a detected edid member to drm_connector from intel_connector
In order to use a detected edid on drm helper functions, it moves
a detected edid member to drm_connector structure from intel_connector
structure.
Signed-off-by: Gwan-gyeong Mun
---
drivers/gpu/drm/i915/intel_dp.c | 18 +-
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_hdmi.c | 10 +-
include/drm/drm_connector.h | 7 +++
4 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 436c22de33b6..c117f552f7d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4157,7 +4157,7 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
struct intel_connector *intel_connector = intel_dp->attached_connector;
struct drm_connector *connector = &intel_connector->base;
- if (intel_connector->detect_edid == NULL ||
+ if (connector->detect_edid == NULL ||
connector->edid_corrupt ||
intel_dp->aux.i2c_defer_count > 6) {
/* Check EDID read for NACKs, DEFERs and corruption
@@ -4174,12 +4174,12 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
intel_dp->aux.i2c_defer_count);
intel_dp->compliance.test_data.edid =
INTEL_DP_RESOLUTION_FAILSAFE;
} else {
- struct edid *block = intel_connector->detect_edid;
+ struct edid *block = connector->detect_edid;
/* We have to write the checksum
* of the last block read
*/
- block += intel_connector->detect_edid->extensions;
+ block += connector->detect_edid->extensions;
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM,
block->checksum) <= 0)
@@ -4993,7 +4993,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
intel_dp_unset_edid(intel_dp);
edid = intel_dp_get_edid(intel_dp);
- intel_connector->detect_edid = edid;
+ intel_connector->base.detect_edid = edid;
intel_dp->has_audio = drm_detect_monitor_audio(edid);
drm_dp_cec_set_edid(&intel_dp->aux, edid);
@@ -5005,8 +5005,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
struct intel_connector *intel_connector = intel_dp->attached_connector;
drm_dp_cec_unset_edid(&intel_dp->aux);
- kfree(intel_connector->detect_edid);
- intel_connector->detect_edid = NULL;
+ kfree(intel_connector->base.detect_edid);
+ intel_connector->base.detect_edid = NULL;
intel_dp->has_audio = false;
}
@@ -5099,7 +5099,7 @@ intel_dp_long_pulse(struct intel_connector *connector,
intel_dp->aux.i2c_defer_count = 0;
intel_dp_set_edid(intel_dp);
- if (intel_dp_is_edp(intel_dp) || connector->detect_edid)
+ if (intel_dp_is_edp(intel_dp) || connector->base.detect_edid)
status = connector_status_connected;
intel_dp->detect_done = true;
@@ -5183,7 +5183,7 @@ static int intel_dp_get_modes(struct drm_connector
*connector)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct edid *edid;
- edid = intel_connector->detect_edid;
+ edid = connector->detect_edid;
if (edid) {
int ret = intel_connector_update_modes(connector, edid);
if (ret)
@@ -5245,7 +5245,7 @@ intel_dp_connector_destroy(struct drm_connector
*connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
- kfree(intel_connector->detect_edid);
+ kfree(connector->detect_edid);
if (!IS_ERR_OR_NULL(intel_connector->edid))
kfree(intel_connector->edid);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5731215210a..19edd7ec4138 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -400,7 +400,6 @@ struct intel_connector {
/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
struct edid *edid;
- struct edid *detect_edid;
/* since POLL and HPD connectors may use the same HPD line keep the
native
state of connector->polled in case hotplug storm detection changes
it */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a2dab0b6bde6..0bedfc0ade49 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1815,8 +1815,8 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
- kfree(to_intel_connector(connector)->detect_edid);
- to_intel_connector(connector)->detect_edid = NULL;
+ kfree(connector->detect_edid);
+ connector->detect_edid = NULL;
}
static void
@@
