[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/ehl: Add support for DPLL4 (v10) (rev2)
== Series Details == Series: drm/i915/ehl: Add support for DPLL4 (v10) (rev2) URL : https://patchwork.freedesktop.org/series/63171/ State : success == Summary == CI Bug Log - changes from CI_DRM_6495_full -> Patchwork_13667_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13667_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_isolation@vecs0-s3: - shard-apl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +2 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-apl3/igt@gem_ctx_isolat...@vecs0-s3.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-apl7/igt@gem_ctx_isolat...@vecs0-s3.html * igt@gem_tiled_swapping@non-threaded: - shard-kbl: [PASS][3] -> [DMESG-WARN][4] ([fdo#108686]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-kbl6/igt@gem_tiled_swapp...@non-threaded.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-kbl7/igt@gem_tiled_swapp...@non-threaded.html * igt@i915_pm_rpm@system-suspend: - shard-skl: [PASS][5] -> [INCOMPLETE][6] ([fdo#104108] / [fdo#107807]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-skl4/igt@i915_pm_...@system-suspend.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-skl10/igt@i915_pm_...@system-suspend.html * igt@kms_flip@2x-plain-flip-ts-check: - shard-glk: [PASS][7] -> [FAIL][8] ([fdo#100368]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-glk1/igt@kms_f...@2x-plain-flip-ts-check.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-glk3/igt@kms_f...@2x-plain-flip-ts-check.html * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-indfb-pgflip-blt: - shard-glk: [PASS][9] -> [FAIL][10] ([fdo#103167]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-glk8/igt@kms_frontbuffer_track...@fbc-2p-primscrn-indfb-pgflip-blt.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-glk8/igt@kms_frontbuffer_track...@fbc-2p-primscrn-indfb-pgflip-blt.html * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-fullscreen: - shard-hsw: [PASS][11] -> [SKIP][12] ([fdo#109271]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-hsw7/igt@kms_frontbuffer_track...@fbc-2p-scndscrn-spr-indfb-fullscreen.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-hsw4/igt@kms_frontbuffer_track...@fbc-2p-scndscrn-spr-indfb-fullscreen.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render: - shard-iclb: [PASS][13] -> [FAIL][14] ([fdo#103167]) +4 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb6/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-spr-indfb-draw-render.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-iclb7/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-spr-indfb-draw-render.html * igt@kms_frontbuffer_tracking@fbcpsr-suspend: - shard-skl: [PASS][15] -> [INCOMPLETE][16] ([fdo#104108] / [fdo#106978]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-skl6/igt@kms_frontbuffer_track...@fbcpsr-suspend.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-skl4/igt@kms_frontbuffer_track...@fbcpsr-suspend.html * igt@kms_plane_lowres@pipe-a-tiling-y: - shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103166]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb1/igt@kms_plane_low...@pipe-a-tiling-y.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-iclb1/igt@kms_plane_low...@pipe-a-tiling-y.html * igt@kms_setmode@basic: - shard-apl: [PASS][19] -> [FAIL][20] ([fdo#99912]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-apl4/igt@kms_setm...@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-apl8/igt@kms_setm...@basic.html Possible fixes * igt@gem_mmap_gtt@basic-small-copy-odd: - shard-iclb: [INCOMPLETE][21] ([fdo#107713]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb7/igt@gem_mmap_...@basic-small-copy-odd.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-iclb1/igt@gem_mmap_...@basic-small-copy-odd.html * igt@i915_pm_rc6_residency@rc6-accuracy: - shard-kbl: [SKIP][23] ([fdo#109271]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-kbl2/igt@i915_pm_rc6_reside...@rc6-accuracy.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/shard-kbl3/igt@i915_pm_rc6_reside...@rc6-accuracy.html - shard-snb:
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/ehl: Add support for DPLL4 (v10) (rev2)
== Series Details ==
Series: drm/i915/ehl: Add support for DPLL4 (v10) (rev2)
URL : https://patchwork.freedesktop.org/series/63171/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6495 -> Patchwork_13667
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/
Known issues
Here are the changes found in Patchwork_13667 that come from known issues:
### IGT changes ###
Possible fixes
* igt@i915_module_load@reload-with-fault-injection:
- {fi-icl-u4}:[DMESG-WARN][1] ([fdo#105602] / [fdo#106107] /
[fdo#106350]) -> [PASS][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-icl-u4/igt@i915_module_l...@reload-with-fault-injection.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/fi-icl-u4/igt@i915_module_l...@reload-with-fault-injection.html
* igt@i915_selftest@live_hangcheck:
- fi-icl-u2: [INCOMPLETE][3] ([fdo#107713] / [fdo#108569]) ->
[PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
* igt@kms_psr@sprite_plane_onoff:
- {fi-icl-u4}:[DMESG-WARN][5] ([fdo#105602]) -> [PASS][6] +30
similar issues
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-icl-u4/igt@kms_psr@sprite_plane_onoff.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/fi-icl-u4/igt@kms_psr@sprite_plane_onoff.html
* igt@prime_vgem@basic-fence-flip:
- fi-ilk-650: [DMESG-WARN][7] ([fdo#106387]) -> [PASS][8] +1
similar issue
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-ilk-650/igt@prime_v...@basic-fence-flip.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/fi-ilk-650/igt@prime_v...@basic-fence-flip.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
[fdo#106350]: https://bugs.freedesktop.org/show_bug.cgi?id=106350
[fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046
[fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
Participating hosts (54 -> 47)
--
Additional (1): fi-icl-dsi
Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-
* Linux: CI_DRM_6495 -> Patchwork_13667
CI_DRM_6495: b782c53ecfa06fdbe9b310dca3f0d477fc833496 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_5100: 0ea68a1efbfcc4961f2f816ab59e4ad8136c6250 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13667: 8c1db3015d5380d54d1711f54325e7978cb4bf30 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8c1db3015d53 drm/i915/ehl: Use an id of 4 while accessing DPLL4's CR0 and CR1
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13667/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/ehl: Use an id of 4 while accessing DPLL4's CR0 and CR1
Although, DPLL4 enable and disable is associated with MGPLL1_ENABLE
register, we can use ICL_DPLL_CFGCR0/CR1 macros to access this dpll's
CR0 and CR1 registers by passing an id of 4 to these macros.
Reported-by: Ville Syrjälä
Cc: Ville Syrjälä
Cc: José Roberto de Souza
Cc: Matt Roper
Cc: Imre Deak
Signed-off-by: Vivek Kasireddy
---
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 319a26a1ec10..f9bdf8514a53 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3127,8 +3127,13 @@ static bool icl_pll_get_hw_state(struct drm_i915_private
*dev_priv,
hw_state->cfgcr0 = I915_READ(TGL_DPLL_CFGCR0(id));
hw_state->cfgcr1 = I915_READ(TGL_DPLL_CFGCR1(id));
} else {
- hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
- hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
+ if (IS_ELKHARTLAKE(dev_priv) && id == DPLL_ID_EHL_DPLL4) {
+ hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(4));
+ hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(4));
+ } else {
+ hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
+ hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
+ }
}
ret = true;
@@ -3169,8 +3174,13 @@ static void icl_dpll_write(struct drm_i915_private
*dev_priv,
cfgcr0_reg = TGL_DPLL_CFGCR0(id);
cfgcr1_reg = TGL_DPLL_CFGCR1(id);
} else {
- cfgcr0_reg = ICL_DPLL_CFGCR0(id);
- cfgcr1_reg = ICL_DPLL_CFGCR1(id);
+ if (IS_ELKHARTLAKE(dev_priv) && id == DPLL_ID_EHL_DPLL4) {
+ cfgcr0_reg = ICL_DPLL_CFGCR0(4);
+ cfgcr1_reg = ICL_DPLL_CFGCR1(4);
+ } else {
+ cfgcr0_reg = ICL_DPLL_CFGCR0(id);
+ cfgcr1_reg = ICL_DPLL_CFGCR1(id);
+ }
}
I915_WRITE(cfgcr0_reg, hw_state->cfgcr0);
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ehl: Add support for DPLL4 (v10)
On Wed, 10 Jul 2019 21:47:52 +0300
Ville Syrjälä wrote:
Hi Ville,
> On Wed, Jul 03, 2019 at 04:03:53PM -0700, Vivek Kasireddy wrote:
> > This patch adds support for DPLL4 on EHL that include the
> > following restrictions:
> >
> > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
> > DPLL4 can be used with other DDIs, including DDID
> > (combo port A external usage).
> >
> > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> >
> > - The DPLL4 enable, lock, power enabled, and power state are
> > connected to the MGPLL1_ENABLE register.
> >
> > v2: (suggestions from Bob Paauwe)
> > - Rework ehl_get_dpll() function to call intel_find_shared_dpll()
> > and iterate twice: once for Combo plls and once for MG plls.
> >
> > - Use MG pll funcs for DPLL4 instead of creating new ones and modify
> > mg_pll_enable to include the restrictions for EHL.
> >
> > v3: Fix compilation error
> >
> > v4: (suggestions from Lucas and Ville)
> > - Treat DPLL4 as a combo phy PLL and not as MG PLL
> > - Disable DC states when this DPLL is being enabled
> > - Reuse icl_get_dpll instead of creating a separate one for EHL
> >
> > v5: (suggestion from Ville)
> > - Refcount the DC OFF power domains during the enabling and
> > disabling of this DPLL.
> >
> > v6: rebase
> >
> > v7: (suggestion from Imre)
> > - Add a new power domain instead of iterating over the domains
> > assoicated with DC OFF power well.
> >
> > v8: (Ville and Imre)
> > - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF
> > - Grab a reference in intel_modeset_setup_hw_state() if this
> > DPLL was already enabled perhaps by BIOS.
> > - Check for the port type instead of the encoder
> >
> > v9: (Ville)
> > - Move the block of code that grabs a reference to the power domain
> > POWER_DOMAIN_DPLL_DC_OFF to intel_modeset_readout_hw_state() to
> > ensure that there is a reference present before this DPLL might get
> > disabled.
> >
> > v10: rebase
> >
> > Cc: José Roberto de Souza
> > Cc: Ville Syrjälä
> > Cc: Matt Roper
> > Cc: Imre Deak
> > Signed-off-by: Vivek Kasireddy
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 7 +++
> > .../drm/i915/display/intel_display_power.c| 3 ++
> > .../drm/i915/display/intel_display_power.h| 1 +
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 47
> > +-- drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > | 6 +++ 5 files changed, 60 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c index
> > 919f5ac844c8..557462208462 100644 ---
> > a/drivers/gpu/drm/i915/display/intel_display.c +++
> > b/drivers/gpu/drm/i915/display/intel_display.c @@ -16653,6
> > +16653,13 @@ static void intel_modeset_readout_hw_state(struct
> > drm_device *dev) pll->on = pll->info->funcs->get_hw_state(dev_priv,
> > pll, >state.hw_state);
> > +
> > + if (IS_ELKHARTLAKE(dev_priv) && pll->on &&
> > + pll->info->id == DPLL_ID_EHL_DPLL4) {
> > + pll->wakeref =
> > intel_display_power_get(dev_priv,
> > +
> > POWER_DOMAIN_DPLL_DC_OFF);
> > + }
> > +
> > pll->state.crtc_mask = 0;
> > for_each_intel_crtc(dev, crtc) {
> > struct intel_crtc_state *crtc_state =
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c index
> > c19b958461ca..7437fc71d289 100644 ---
> > a/drivers/gpu/drm/i915/display/intel_display_power.c +++
> > b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -118,6
> > +118,8 @@ intel_display_power_domain_str(enum
> > intel_display_power_domain domain) return "MODESET"; case
> > POWER_DOMAIN_GT_IRQ: return "GT_IRQ";
> > + case POWER_DOMAIN_DPLL_DC_OFF:
> > + return "DPLL_DC_OFF";
> > default:
> > MISSING_CASE(domain);
> > return "?";
> > @@ -2455,6 +2457,7 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv, ICL_PW_2_POWER_DOMAINS
> > | \ BIT_ULL(POWER_DOMAIN_MODESET)
> > | \ BIT_ULL(POWER_DOMAIN_AUX_A)
> > | \
> > + BIT_ULL(POWER_DOMAIN_DPLL_DC_OFF) |
> > \ BIT_ULL(POWER_DOMAIN_INIT))
> >
> > #define ICL_DDI_IO_A_POWER_DOMAINS ( \
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> > b/drivers/gpu/drm/i915/display/intel_display_power.h index
> > ff57b0a7fe59..8f43f7051a16 100644 ---
> > a/drivers/gpu/drm/i915/display/intel_display_power.h +++
> > b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -59,6 +59,7
> > @@ enum intel_display_power_domain { POWER_DOMAIN_GMBUS,
> > POWER_DOMAIN_MODESET,
> > POWER_DOMAIN_GT_IRQ,
> > + POWER_DOMAIN_DPLL_DC_OFF,
> > POWER_DOMAIN_INIT,
> >
> > POWER_DOMAIN_NUM,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c index
Re: [Intel-gfx] [igt-dev] [PATCH V6 i-g-t 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning
On 07/12, Ser, Simon wrote:
> So, to test these last two patches we'd need specific hardware right?
> Because VKMS doesn't support cloning yet (does it?).
hmmm... actually, VKMS successfully pass in this test. However, if you
compare "writeback-check-output" and "writeback-check-output-clone", you
will notice they are very similar. Maybe, this test does not correctly
validating cloning feature?
> What kind of hardware supports cloned writeback outputs? I have a
> Raspberry Pi which supports writeback via VC4, but I don't think it has
> writeback cloning. I'm also not willing to install any proprietary
> driver.
>
> I guess we could land the first part of the series, and wait for VKMS
> to support cloned outputs to land the last two patches.
>
> Any other ideas?
btw, I'm totally comfortable with the idea of focusing on the first part
of this series.
Thanks
> On Wed, 2019-06-12 at 23:18 -0300, Brian Starkey wrote:
> > An output can be added as a clone of any other output(s) attached to a
> > pipe using igt_output_clone_pipe()
> >
> > v5: Drop field out_fence_requested from struct igt_pipe (Brian Starkey)
> >
> > Signed-off-by: Brian Starkey
> > ---
> > lib/igt_kms.c | 100 +++---
> > lib/igt_kms.h | 4 ++
> > 2 files changed, 66 insertions(+), 38 deletions(-)
> >
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 140db346..b85a0404 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -1765,6 +1765,17 @@ static void igt_display_log_shift(igt_display_t
> > *display, int shift)
> > igt_assert(display->log_shift >= 0);
> > }
> >
> > +static int igt_output_idx(igt_output_t *output)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < output->display->n_outputs; i++)
> > + if (>display->outputs[i] == output)
> > + return i;
> > +
> > + return -1;
> > +}
> > +
> > static void igt_output_refresh(igt_output_t *output)
> > {
> > igt_display_t *display = output->display;
> > @@ -2317,42 +2328,6 @@ void igt_display_fini(igt_display_t *display)
> > display->planes = NULL;
> > }
> >
> > -static void igt_display_refresh(igt_display_t *display)
> > -{
> > - igt_output_t *output;
> > - int i;
> > -
> > - unsigned long pipes_in_use = 0;
> > -
> > - /* Check that two outputs aren't trying to use the same pipe */
> > - for (i = 0; i < display->n_outputs; i++) {
> > - output = >outputs[i];
> > -
> > - if (output->pending_pipe != PIPE_NONE) {
> > - if (pipes_in_use & (1 << output->pending_pipe))
> > - goto report_dup;
> > -
> > - pipes_in_use |= 1 << output->pending_pipe;
> > - }
> > -
> > - if (output->force_reprobe)
> > - igt_output_refresh(output);
> > - }
> > -
> > - return;
> > -
> > -report_dup:
> > - for (; i > 0; i--) {
> > - igt_output_t *b = >outputs[i - 1];
> > -
> > - igt_assert_f(output->pending_pipe !=
> > -b->pending_pipe,
> > -"%s and %s are both trying to use pipe %s\n",
> > -igt_output_name(output), igt_output_name(b),
> > -kmstest_pipe_name(output->pending_pipe));
> > - }
> > -}
> > -
> > static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
> > {
> > igt_display_t *display = output->display;
> > @@ -2376,6 +2351,40 @@ static igt_pipe_t
> > *igt_output_get_driving_pipe(igt_output_t *output)
> > return >pipes[pipe];
> > }
> >
> > +static void igt_display_refresh(igt_display_t *display)
> > +{
> > + igt_output_t *output;
> > + igt_pipe_t *pipe;
> > + int i;
> > +
> > + unsigned long pipes_in_use = 0;
> > + unsigned long pending_crtc_idx_mask;
> > +
> > + /* Check that outputs and pipes agree wrt. cloning */
> > + for (i = 0; i < display->n_outputs; i++) {
> > + output = >outputs[i];
> > + pending_crtc_idx_mask = 1 << output->pending_pipe;
> > +
> > + pipe = igt_output_get_driving_pipe(output);
> > + if (pipe) {
> > + igt_assert_f(pipe->outputs & (1 <<
> > igt_output_idx(output)),
> > +"Output %s not expected to be using pipe
> > %s\n",
> > +igt_output_name(output),
> > +kmstest_pipe_name(pipe->pipe));
> > +
> > + if (pipes_in_use & pending_crtc_idx_mask)
> > + LOG(display, "Output %s clones pipe %s\n",
> > + igt_output_name(output),
> > + kmstest_pipe_name(pipe->pipe));
> > + }
> > +
> > + pipes_in_use |= pending_crtc_idx_mask;
> > +
> > + if (output->force_reprobe)
> > + igt_output_refresh(output);
> > + }
> > +}
> > +
> > static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int
Re: [Intel-gfx] [igt-dev] [PATCH V6 i-g-t 3/6] lib: Add function to hash a framebuffer
On 07/12, Ser, Simon wrote:
> On Thu, 2019-07-11 at 23:49 -0300, Rodrigo Siqueira wrote:
> > On 07/10, Ser, Simon wrote:
> > > On Wed, 2019-07-10 at 15:30 +, Ser, Simon wrote:
> > > > Mostly LGTM, here are a few nits.
> > > >
> > > > On Wed, 2019-06-12 at 23:17 -0300, Brian Starkey wrote:
> > > > > To use writeback buffers as a CRC source, we need to be able to hash
> > > > > them. Implement a simple FVA-1a hashing routine for this purpose.
> > > > >
> > > > > Doing a bytewise hash on the framebuffer directly can be very slow if
> > > > > the memory is noncached. By making a copy of each line in the FB first
> > > > > (which can take advantage of word-access speedup), we can do the hash
> > > > > on a cached copy, which is much faster (10x speedup on my platform).
> > > > >
> > > > > v6: use igt_memcpy_from_wc() instead of plain memcpy, as suggested by
> > > > > Chris Wilson
> > > > >
> > > > > Signed-off-by: Brian Starkey
> > > > > [rebased and updated to the most recent API]
> > > > > Signed-off-by: Liviu Dudau
> > > > > ---
> > > > > lib/igt_fb.c | 66
> > > > >
> > > > > lib/igt_fb.h | 3 +++
> > > > > 2 files changed, 69 insertions(+)
> > > > >
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 9d4f905e..d07dae39 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -3256,6 +3256,72 @@ bool igt_fb_supported_format(uint32_t
> > > > > drm_format)
> > > > > return false;
> > > > > }
> > > > >
> > > > > +/*
> > > > > + * This implements the FNV-1a hashing algorithm instead of CRC, for
> > > > > + * simplicity
> > > > > + * http://www.isthe.com/chongo/tech/comp/fnv/index.html
> > > > > + *
> > > > > + * hash = offset_basis
> > > > > + * for each octet_of_data to be hashed
> > > > > + * hash = hash xor octet_of_data
> > > > > + * hash = hash * FNV_prime
> > > > > + * return hash
> > > > > + *
> > > > > + * 32 bit offset_basis = 2166136261
> > > > > + * 32 bit FNV_prime = 224 + 28 + 0x93 = 16777619
> > > > > + */
> > > > > +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> > > > > +{
> > > > > +#define FNV1a_OFFSET_BIAS 2166136261
> > > > > +#define FNV1a_PRIME 16777619
> > > >
> > > > I'd just use plain uint32_t variables for those, but no big deal.
> > > >
> > > > > + uint32_t hash;
> > > > > + void *map;
> > > > > + char *ptr, *line = NULL;
> > > > > + int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> > > > > + uint32_t stride = calc_plane_stride(fb, 0);
> > > >
> > > > We could return -EINVAL in case fb->num_planes != 1.
> > >
> > > Let's not waste cycles. With this ^ fixed, this patch is:
> > >
> > > Reviewed-by: Simon Ser
> > >
> > > Other nits are optional.
> >
> > I agreed with all your suggestions, and I already applied all of them.
> > Should I wait for the other patches review, or should I resend the new
> > version?
>
> I'm fine with waiting for the full review before a new version of the
> whole patchset, but you can also send an updated version of a single
> patch with:
>
> git send-email
> --in-reply-to="" -1
>
>
> where In-Reply-To is the Message-Id of the patch you want to update. I
> agree it's a little tedious since you need to extract the Message-Id
> from the message header.
Thanks for the tip with git-send-mail. Since I already applied most of
your suggestions, I'll send a full version soon.
> > Thanks for all the feedback
>
> :)
>
> > Best Regards
> >
> > > > > + if (fb->is_dumb)
> > > > > + map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle,
> > > > > fb->size,
> > > > > + PROT_READ);
> > > > > + else
> > > > > + map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> > > > > + PROT_READ);
> > > > > + ptr = map;
> > > >
> > > > Nit: no need for this, can assign the result of mmap directly to ptr.
> > > >
> > > > > +
> > > > > + /*
> > > > > + * Framebuffers are often uncached, which can make byte-wise
> > > > > accesses
> > > > > + * very slow. We copy each line of the FB into a local buffer
> > > > > to speed
> > > > > + * up the hashing.
> > > > > + */
> > > > > + line = malloc(stride);
> > > > > + if (!line) {
> > > > > + munmap(map, fb->size);
> > > > > + return -ENOMEM;
> > > > > + }
> > > > > +
> > > > > + hash = FNV1a_OFFSET_BIAS;
> > > > > +
> > > > > + for (y = 0; y < fb->height; y++, ptr += stride) {
> > > > > +
> > > > > + igt_memcpy_from_wc(line, ptr, stride);
> > > >
> > > > Nit: no need to copy the whole stride actually, we can just copy
> > > > fb->width * cpp since we're only going to read that.
> > > >
> > > > > +
> > > > > + for (x = 0; x < fb->width * cpp; x++) {
> > > > > + hash ^= line[x];
> > > > > + hash *= FNV1a_PRIME;
Re: [Intel-gfx] [PATCH] x86/gpu: add TGL stolen memory support
On Wed, Jul 17, 2019 at 12:46:42AM +0200, Thomas Gleixner wrote:
On Fri, 12 Jul 2019, Lucas De Marchi wrote:
From: Michel Thierry
Reuse Gen11 stolen memory changes since Tiger Lake uses the same BSM
register (and format).
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: x...@kernel.org
Signed-off-by: Michel Thierry
Signed-off-by: Lucas De Marchi
Reviewed-by: Rodrigo Vivi
---
arch/x86/kernel/early-quirks.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 6c4f01540833..6f6b1d04dadf 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -549,6 +549,7 @@ static const struct pci_device_id intel_early_ids[]
__initconst = {
INTEL_CNL_IDS(_early_ops),
INTEL_ICL_11_IDS(_early_ops),
INTEL_EHL_IDS(_early_ops),
+ INTEL_TGL_12_IDS(_early_ops),
How exactly is this supposed to build?
The define for this new platform is on drm-intel repository. For
previous platforms we waited for an ack and merged through our tree.
Is that ok?
thanks
Lucas De Marchi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH V6 i-g-t 2/6] kms_writeback: Add initial writeback tests
On 07/12, Ser, Simon wrote:
> On Thu, 2019-07-11 at 23:44 -0300, Rodrigo Siqueira wrote:
> > On 07/10, Ser, Simon wrote:
> > > Hi,
> > >
> > > Thanks for the patch! Here are a few comments.
> > >
> > > For bonus points, it would be nice to add igt_describe descriptions of
> > > each sub-test.
> >
> > Hi Simon,
> >
> > First of all, thanks for your feedback; I already applied most of your
> > suggestions. I just have some inline comments/questions.
> >
> > > On Wed, 2019-06-12 at 23:16 -0300, Brian Starkey wrote:
> > > > Add tests for the WRITEBACK_PIXEL_FORMATS, WRITEBACK_OUT_FENCE_PTR and
> > > > WRITEBACK_FB_ID properties on writeback connectors, ensuring their
> > > > behaviour is correct.
> > > >
> > > > Signed-off-by: Brian Starkey
> > > > [rebased and updated do_writeback_test() function to address feedback]
> > > > Signed-off-by: Liviu Dudau
> > > > ---
> > > > tests/Makefile.sources | 1 +
> > > > tests/kms_writeback.c | 314 +
> > > > tests/meson.build | 1 +
> > > > 3 files changed, 316 insertions(+)
> > > > create mode 100644 tests/kms_writeback.c
> > > >
> > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > index 027ed82f..03cc8efa 100644
> > > > --- a/tests/Makefile.sources
> > > > +++ b/tests/Makefile.sources
> > > > @@ -77,6 +77,7 @@ TESTS_progs = \
> > > > kms_universal_plane \
> > > > kms_vblank \
> > > > kms_vrr \
> > > > + kms_writeback \
> > > > meta_test \
> > > > perf \
> > > > perf_pmu \
> > > > diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
> > > > new file mode 100644
> > > > index ..66ef48a6
> > > > --- /dev/null
> > > > +++ b/tests/kms_writeback.c
> > > > @@ -0,0 +1,314 @@
> > > > +/*
> > > > + * (C) COPYRIGHT 2017 ARM Limited. All rights reserved.
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > > > obtaining a
> > > > + * copy of this software and associated documentation files (the
> > > > "Software"),
> > > > + * to deal in the Software without restriction, including without
> > > > limitation
> > > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > > sublicense,
> > > > + * and/or sell copies of the Software, and to permit persons to whom
> > > > the
> > > > + * Software is furnished to do so, subject to the following conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice (including
> > > > the next
> > > > + * paragraph) shall be included in all copies or substantial portions
> > > > of the
> > > > + * Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > > SHALL
> > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> > > > OR OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > > ARISING
> > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > > DEALINGS
> > > > + * IN THE SOFTWARE.
> > > > + *
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include "igt.h"
> > > > +#include "igt_core.h"
> > > > +#include "igt_fb.h"
> > > > +
> > > > +static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t
> > > > *output)
> > > > +{
> > > > + drmModePropertyBlobRes *blob = NULL;
> > > > + uint64_t blob_id;
> > > > + int ret;
> > > > +
> > > > + ret = kmstest_get_property(output->display->drm_fd,
> > > > +
> > > > output->config.connector->connector_id,
> > > > + DRM_MODE_OBJECT_CONNECTOR,
> > > > +
> > > > igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
> > > > + NULL, _id, NULL);
> > > > + if (ret)
> > > > + blob = drmModeGetPropertyBlob(output->display->drm_fd,
> > > > blob_id);
> > > > +
> > > > + igt_assert(blob);
> > > > +
> > > > + return blob;
> > > > +}
> > > > +
> > > > +static bool check_writeback_config(igt_display_t *display,
> > > > igt_output_t *output)
> > > > +{
> > > > + igt_fb_t input_fb, output_fb;
> > > > + igt_plane_t *plane;
> > > > + uint32_t writeback_format = DRM_FORMAT_XRGB;
> > > > + uint64_t tiling = igt_fb_mod_to_tiling(0);
> > > > + int width, height, ret;
> > > > + drmModeModeInfo override_mode = {
> > > > + .clock = 25175,
> > > > + .hdisplay = 640,
> > > > + .hsync_start = 656,
> > > > + .hsync_end = 752,
> > > > + .htotal = 800,
> > > > + .hskew = 0,
> > >
Re: [Intel-gfx] [PATCH 06/22] drm/i915/tgl: handle DP aux interrupts
>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lucas De Marchi
>Sent: Friday, July 12, 2019 6:09 PM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH 06/22] drm/i915/tgl: handle DP aux interrupts
>
>For Tiger Lake the DE Port Interrupt Definition bits changed, so use the new
>bit
>definitions.
>
>Cc: Jose Souza
>Signed-off-by: Lucas De Marchi
Reviewed-by: Anusha Srivatsa
>---
> drivers/gpu/drm/i915/i915_irq.c | 16 +++-
>drivers/gpu/drm/i915/i915_reg.h | 3 +++
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index 256bd2c072c1..6350e9dee653 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -2939,19 +2939,25 @@ static void gen11_hpd_irq_handler(struct
>drm_i915_private *dev_priv, u32 iir)
>
> static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv) {
>- u32 mask = GEN8_AUX_CHANNEL_A;
>+ u32 mask;
>+
>+ if (INTEL_GEN(dev_priv) >= 12)
>+ /* TODO: Add AUX entries for USBC */
>+ return TGL_DE_PORT_AUX_DDIA |
>+ TGL_DE_PORT_AUX_DDIB |
>+ TGL_DE_PORT_AUX_DDIC;
>
>+ mask = GEN8_AUX_CHANNEL_A;
> if (INTEL_GEN(dev_priv) >= 9)
> mask |= GEN9_AUX_CHANNEL_B |
> GEN9_AUX_CHANNEL_C |
> GEN9_AUX_CHANNEL_D;
>
>- if (IS_CNL_WITH_PORT_F(dev_priv))
>+ if (IS_CNL_WITH_PORT_F(dev_priv) || IS_GEN(dev_priv, 11))
> mask |= CNL_AUX_CHANNEL_F;
>
>- if (INTEL_GEN(dev_priv) >= 11)
>- mask |= ICL_AUX_CHANNEL_E |
>- CNL_AUX_CHANNEL_F;
>+ if (IS_GEN(dev_priv, 11))
>+ mask |= ICL_AUX_CHANNEL_E;
>
> return mask;
> }
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index ff703baf105f..41c8b40eebd5 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -7428,6 +7428,9 @@ enum {
> #define GEN8_PORT_DP_A_HOTPLUG (1 << 3)
> #define BXT_DE_PORT_GMBUS(1 << 1)
> #define GEN8_AUX_CHANNEL_A (1 << 0)
>+#define TGL_DE_PORT_AUX_DDIC (1 << 2)
>+#define TGL_DE_PORT_AUX_DDIB (1 << 1)
>+#define TGL_DE_PORT_AUX_DDIA (1 << 0)
>
> #define GEN8_DE_MISC_ISR _MMIO(0x44460) #define GEN8_DE_MISC_IMR
>_MMIO(0x44464)
>--
>2.21.0
>
>___
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/vgem: use normal cached mmap'ings
On Tue, Jul 16, 2019 at 4:39 PM Eric Anholt wrote: > > Rob Clark writes: > > > From: Rob Clark > > > > Since there is no real device associated with VGEM, it is impossible to > > end up with appropriate dev->dma_ops, meaning that we have no way to > > invalidate the shmem pages allocated by VGEM. So, at least on platforms > > without drm_cflush_pages(), we end up with corruption when cache lines > > from previous usage of VGEM bo pages get evicted to memory. > > > > The only sane option is to use cached mappings. > > This may be an improvement, but... > > pin/unpin is only on attaching/closing the dma-buf, right? So, great, > you flushed the cached map once after exporting the vgem dma-buf to the > actual GPU device, but from then on you still have no interface for > getting coherent access through VGEM's mapping again, which still > exists. In *theory* one would detach before doing further CPU access to buffer, and then re-attach when passing back to GPU. Ofc that isn't how actual drivers do things. But maybe it is enough for vgem to serve it's purpose (ie. test code). > I feel like this is papering over something that's really just broken, > and we should stop providing VGEM just because someone wants to write > dma-buf test code without driver-specific BO alloc ioctl code. yup, it is vgem that is fundamentally broken (or maybe more specifically doesn't fit in w/ dma-mappings view of how to do cache maint), and I'm just papering over it because people and CI systems want to be able to use it to do some dma-buf tests ;-) I'm kinda wondering, at least for arm/dt based systems, if there is a way (other than in early boot) that we can inject a vgem device node into the dtb. That isn't a thing drivers should normally do, but (if possible) since vgem is really just test infrastructure, it could be a way to make dma-mapping happily think vgem is a real device. BR, -R ___ 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/oa: Reconfigure contexts on the fly (rev3)
== Series Details == Series: drm/i915/oa: Reconfigure contexts on the fly (rev3) URL : https://patchwork.freedesktop.org/series/63276/ State : success == Summary == CI Bug Log - changes from CI_DRM_6495_full -> Patchwork_13665_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13665_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_pwrite@big-gtt-fbr: - shard-apl: [PASS][1] -> [INCOMPLETE][2] ([fdo#103927]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-apl7/igt@gem_pwr...@big-gtt-fbr.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-apl4/igt@gem_pwr...@big-gtt-fbr.html * igt@gem_shrink@reclaim: - shard-iclb: [PASS][3] -> [INCOMPLETE][4] ([fdo#107713]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb6/igt@gem_shr...@reclaim.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-iclb7/igt@gem_shr...@reclaim.html * igt@gem_tiled_swapping@non-threaded: - shard-apl: [PASS][5] -> [DMESG-WARN][6] ([fdo#108686]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-apl8/igt@gem_tiled_swapp...@non-threaded.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-apl3/igt@gem_tiled_swapp...@non-threaded.html * igt@i915_pm_rpm@system-suspend-modeset: - shard-iclb: [PASS][7] -> [INCOMPLETE][8] ([fdo#107713] / [fdo#108840]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb7/igt@i915_pm_...@system-suspend-modeset.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-iclb2/igt@i915_pm_...@system-suspend-modeset.html * igt@kms_cursor_crc@pipe-a-cursor-suspend: - shard-apl: [PASS][9] -> [DMESG-WARN][10] ([fdo#108566]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-apl6/igt@kms_cursor_...@pipe-a-cursor-suspend.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-apl5/igt@kms_cursor_...@pipe-a-cursor-suspend.html * igt@kms_cursor_crc@pipe-b-cursor-suspend: - shard-kbl: [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-kbl6/igt@kms_cursor_...@pipe-b-cursor-suspend.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-kbl3/igt@kms_cursor_...@pipe-b-cursor-suspend.html * igt@kms_flip@flip-vs-suspend: - shard-iclb: [PASS][13] -> [INCOMPLETE][14] ([fdo#107713] / [fdo#109507]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb6/igt@kms_f...@flip-vs-suspend.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-iclb7/igt@kms_f...@flip-vs-suspend.html * igt@kms_flip@flip-vs-suspend-interruptible: - shard-skl: [PASS][15] -> [INCOMPLETE][16] ([fdo#109507]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-skl3/igt@kms_f...@flip-vs-suspend-interruptible.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-skl4/igt@kms_f...@flip-vs-suspend-interruptible.html * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw: - shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103167]) +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-iclb6/igt@kms_frontbuffer_track...@fbc-1p-pri-indfb-multidraw.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-iclb4/igt@kms_frontbuffer_track...@fbc-1p-pri-indfb-multidraw.html * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-indfb-pgflip-blt: - shard-glk: [PASS][19] -> [FAIL][20] ([fdo#103167]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-glk8/igt@kms_frontbuffer_track...@fbc-2p-primscrn-indfb-pgflip-blt.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-glk2/igt@kms_frontbuffer_track...@fbc-2p-primscrn-indfb-pgflip-blt.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: - shard-skl: [PASS][21] -> [INCOMPLETE][22] ([fdo#104108]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-skl2/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-c.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-skl6/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-c.html * igt@kms_setmode@basic: - shard-apl: [PASS][23] -> [FAIL][24] ([fdo#99912]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/shard-apl4/igt@kms_setm...@basic.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/shard-apl8/igt@kms_setm...@basic.html * igt@perf@blocking: - shard-skl: [PASS][25] -> [FAIL][26] ([fdo#110728]) [25]:
Re: [Intel-gfx] [PATCH v3 3/3] drm/vgem: use normal cached mmap'ings
Rob Clark writes: > From: Rob Clark > > Since there is no real device associated with VGEM, it is impossible to > end up with appropriate dev->dma_ops, meaning that we have no way to > invalidate the shmem pages allocated by VGEM. So, at least on platforms > without drm_cflush_pages(), we end up with corruption when cache lines > from previous usage of VGEM bo pages get evicted to memory. > > The only sane option is to use cached mappings. This may be an improvement, but... pin/unpin is only on attaching/closing the dma-buf, right? So, great, you flushed the cached map once after exporting the vgem dma-buf to the actual GPU device, but from then on you still have no interface for getting coherent access through VGEM's mapping again, which still exists. I feel like this is papering over something that's really just broken, and we should stop providing VGEM just because someone wants to write dma-buf test code without driver-specific BO alloc ioctl code. signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] x86/gpu: add TGL stolen memory support
On Fri, 12 Jul 2019, Lucas De Marchi wrote:
> From: Michel Thierry
>
> Reuse Gen11 stolen memory changes since Tiger Lake uses the same BSM
> register (and format).
>
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> Cc: Borislav Petkov
> Cc: "H. Peter Anvin"
> Cc: x...@kernel.org
> Signed-off-by: Michel Thierry
> Signed-off-by: Lucas De Marchi
> Reviewed-by: Rodrigo Vivi
> ---
> arch/x86/kernel/early-quirks.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index 6c4f01540833..6f6b1d04dadf 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -549,6 +549,7 @@ static const struct pci_device_id intel_early_ids[]
> __initconst = {
> INTEL_CNL_IDS(_early_ops),
> INTEL_ICL_11_IDS(_early_ops),
> INTEL_EHL_IDS(_early_ops),
> + INTEL_TGL_12_IDS(_early_ops),
How exactly is this supposed to build?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/gem: don't force writecombine mmap'ing
Rob Clark writes: > From: Rob Clark > > The driver should be in control of this. > > Signed-off-by: Rob Clark > --- > It is possible that this was masking bugs (ie. not setting appropriate > pgprot) in drivers. I don't have a particularly good idea for tracking > those down (since I don't have the hw for most drivers). Unless someone > has a better idea, maybe land this and let driver maintainers fix any > potential fallout in their drivers? > > This is necessary for the last patch to fix VGEM brokenness on arm. This will break at least v3d and panfrost, and it looks like cirrus as well, since you're now promoting the mapping to cached by default and drm_gem_shmem_helper now produces cached mappings. That's all I could find that would break, but don't trust me on that. signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/22] drm/i915/tgl: Update north display hotplug detection to TGL connections
>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lucas De Marchi
>Sent: Friday, July 12, 2019 6:09 PM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH 05/22] drm/i915/tgl: Update north display hotplug
>detection to TGL connections
>
>From: José Roberto de Souza
>
>TGL has 3 combophys and 6 TC/TBT ports, so it has 2 more TC/TBT ports than ICL
>and the PORT_C on TGL is a combophy.
>So here adding a new hpd north table and function to detect long pulse for TGL.
>
>Signed-off-by: José Roberto de Souza
>Signed-off-by: Lucas De Marchi
Reviewed-by: Anusha Srivatsa
>---
> drivers/gpu/drm/i915/i915_irq.c | 51 +
>drivers/gpu/drm/i915/i915_reg.h | 12 ++--
> 2 files changed, 56 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index a7a90674db89..256bd2c072c1 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -56,6 +56,8 @@
> * and related files, but that will be described in separate chapters.
> */
>
>+typedef bool (*long_pulse_detect_func)(enum hpd_pin pin, u32 val);
>+
> static const u32 hpd_ilk[HPD_NUM_PINS] = {
> [HPD_PORT_A] = DE_DP_A_HOTPLUG,
> };
>@@ -133,6 +135,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = {
> [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG };
>
>+static const u32 hpd_gen12[HPD_NUM_PINS] = {
>+ [HPD_PORT_D] = GEN11_TC1_HOTPLUG | GEN11_TBT1_HOTPLUG,
>+ [HPD_PORT_E] = GEN11_TC2_HOTPLUG | GEN11_TBT2_HOTPLUG,
>+ [HPD_PORT_F] = GEN11_TC3_HOTPLUG | GEN11_TBT3_HOTPLUG,
>+ [HPD_PORT_G] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG,
>+ [HPD_PORT_H] = GEN12_TC5_HOTPLUG | GEN12_TBT5_HOTPLUG,
>+ [HPD_PORT_I] = GEN12_TC6_HOTPLUG | GEN12_TBT6_HOTPLUG };
>+
> static const u32 hpd_icp[HPD_NUM_PINS] = {
> [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
> [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
>@@ -1676,6 +1687,26 @@ static bool gen11_port_hotplug_long_detect(enum
>hpd_pin pin, u32 val)
> }
> }
>
>+static bool gen12_port_hotplug_long_detect(enum hpd_pin pin, u32 val) {
>+ switch (pin) {
>+ case HPD_PORT_D:
>+ return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
>+ case HPD_PORT_E:
>+ return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
>+ case HPD_PORT_F:
>+ return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
>+ case HPD_PORT_G:
>+ return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
>+ case HPD_PORT_H:
>+ return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC5);
>+ case HPD_PORT_I:
>+ return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC6);
>+ default:
>+ return false;
>+ }
>+}
>+
> static bool bxt_port_hotplug_long_detect(enum hpd_pin pin, u32 val) {
> switch (pin) {
>@@ -2869,6 +2900,16 @@ static void gen11_hpd_irq_handler(struct
>drm_i915_private *dev_priv, u32 iir)
> u32 pin_mask = 0, long_mask = 0;
> u32 trigger_tc = iir & GEN11_DE_TC_HOTPLUG_MASK;
> u32 trigger_tbt = iir & GEN11_DE_TBT_HOTPLUG_MASK;
>+ long_pulse_detect_func long_pulse_detect;
>+ const u32 *hpd;
>+
>+ if (INTEL_GEN(dev_priv) >= 12) {
>+ long_pulse_detect = gen12_port_hotplug_long_detect;
>+ hpd = hpd_gen12;
>+ } else {
>+ long_pulse_detect = gen11_port_hotplug_long_detect;
>+ hpd = hpd_gen11;
>+ }
>
> if (trigger_tc) {
> u32 dig_hotplug_reg;
>@@ -2877,8 +2918,7 @@ static void gen11_hpd_irq_handler(struct
>drm_i915_private *dev_priv, u32 iir)
> I915_WRITE(GEN11_TC_HOTPLUG_CTL, dig_hotplug_reg);
>
> intel_get_hpd_pins(dev_priv, _mask, _mask,
>trigger_tc,
>- dig_hotplug_reg, hpd_gen11,
>- gen11_port_hotplug_long_detect);
>+ dig_hotplug_reg, hpd, long_pulse_detect);
> }
>
> if (trigger_tbt) {
>@@ -2888,8 +2928,7 @@ static void gen11_hpd_irq_handler(struct
>drm_i915_private *dev_priv, u32 iir)
> I915_WRITE(GEN11_TBT_HOTPLUG_CTL, dig_hotplug_reg);
>
> intel_get_hpd_pins(dev_priv, _mask, _mask,
>trigger_tbt,
>- dig_hotplug_reg, hpd_gen11,
>- gen11_port_hotplug_long_detect);
>+ dig_hotplug_reg, hpd, long_pulse_detect);
> }
>
> if (pin_mask)
>@@ -3915,9 +3954,11 @@ static void gen11_hpd_detection_setup(struct
>drm_i915_private *dev_priv) static void gen11_hpd_irq_setup(struct
>drm_i915_private *dev_priv) {
> u32 hotplug_irqs, enabled_irqs;
>+ const u32 *hpd;
> u32 val;
>
>- enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_gen11);
>+ hpd = INTEL_GEN(dev_priv) >= 12 ? hpd_gen12 : hpd_gen11;
>+ enabled_irqs =
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v3,1/3] drm/gem: don't force writecombine mmap'ing
== Series Details == Series: series starting with [v3,1/3] drm/gem: don't force writecombine mmap'ing URL : https://patchwork.freedesktop.org/series/63770/ State : failure == Summary == CI Bug Log - changes from CI_DRM_6495 -> Patchwork_13666 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_13666 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_13666, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_13666: ### IGT changes ### Possible regressions * igt@prime_vgem@basic-busy-default: - fi-bsw-n3050: [PASS][1] -> [FAIL][2] +7 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-bsw-n3050/igt@prime_v...@basic-busy-default.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-bsw-n3050/igt@prime_v...@basic-busy-default.html - fi-apl-guc: [PASS][3] -> [FAIL][4] +3 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-apl-guc/igt@prime_v...@basic-busy-default.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-apl-guc/igt@prime_v...@basic-busy-default.html * igt@prime_vgem@basic-fence-mmap: - fi-ilk-650: [PASS][5] -> [INCOMPLETE][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-ilk-650/igt@prime_v...@basic-fence-mmap.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-ilk-650/igt@prime_v...@basic-fence-mmap.html - fi-blb-e6850: [PASS][7] -> [INCOMPLETE][8] +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-blb-e6850/igt@prime_v...@basic-fence-mmap.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-blb-e6850/igt@prime_v...@basic-fence-mmap.html * igt@prime_vgem@basic-fence-read: - fi-bsw-kefka: [PASS][9] -> [INCOMPLETE][10] +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-bsw-kefka/igt@prime_v...@basic-fence-read.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-bsw-kefka/igt@prime_v...@basic-fence-read.html - fi-gdg-551: [PASS][11] -> [FAIL][12] +4 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-gdg-551/igt@prime_v...@basic-fence-read.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-gdg-551/igt@prime_v...@basic-fence-read.html - fi-bsw-n3050: [PASS][13] -> [INCOMPLETE][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-bsw-n3050/igt@prime_v...@basic-fence-read.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-bsw-n3050/igt@prime_v...@basic-fence-read.html * igt@prime_vgem@basic-gtt: - fi-bwr-2160:[PASS][15] -> [FAIL][16] +4 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-bwr-2160/igt@prime_v...@basic-gtt.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-bwr-2160/igt@prime_v...@basic-gtt.html - fi-ilk-650: [PASS][17] -> [FAIL][18] +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-ilk-650/igt@prime_v...@basic-gtt.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-ilk-650/igt@prime_v...@basic-gtt.html * igt@prime_vgem@basic-read: - fi-elk-e7500: [PASS][19] -> [FAIL][20] +2 similar issues [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-elk-e7500/igt@prime_v...@basic-read.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-elk-e7500/igt@prime_v...@basic-read.html * igt@prime_vgem@basic-sync-default: - fi-byt-n2820: [PASS][21] -> [FAIL][22] +7 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-byt-n2820/igt@prime_v...@basic-sync-default.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-byt-n2820/igt@prime_v...@basic-sync-default.html - fi-bxt-j4205: [PASS][23] -> [FAIL][24] +3 similar issues [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-bxt-j4205/igt@prime_v...@basic-sync-default.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-bxt-j4205/igt@prime_v...@basic-sync-default.html - fi-byt-j1900: [PASS][25] -> [FAIL][26] +7 similar issues [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-byt-j1900/igt@prime_v...@basic-sync-default.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13666/fi-byt-j1900/igt@prime_v...@basic-sync-default.html - fi-bxt-dsi: [PASS][27] -> [FAIL][28]
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/oa: Reconfigure contexts on the fly (rev3)
== Series Details ==
Series: drm/i915/oa: Reconfigure contexts on the fly (rev3)
URL : https://patchwork.freedesktop.org/series/63276/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6495 -> Patchwork_13665
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/
Known issues
Here are the changes found in Patchwork_13665 that come from known issues:
### IGT changes ###
Issues hit
* igt@kms_busy@basic-flip-c:
- fi-skl-6770hq: [PASS][1] -> [SKIP][2] ([fdo#109271] / [fdo#109278])
+2 similar issues
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-skl-6770hq/igt@kms_b...@basic-flip-c.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-skl-6770hq/igt@kms_b...@basic-flip-c.html
* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6770hq: [PASS][3] -> [SKIP][4] ([fdo#109271]) +23 similar
issues
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-skl-6770hq/igt@kms_f...@basic-flip-vs-dpms.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-skl-6770hq/igt@kms_f...@basic-flip-vs-dpms.html
* igt@kms_flip@basic-flip-vs-wf_vblank:
- fi-bsw-n3050: [PASS][5] -> [FAIL][6] ([fdo#100368])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-bsw-n3050/igt@kms_flip@basic-flip-vs-wf_vblank.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-bsw-n3050/igt@kms_flip@basic-flip-vs-wf_vblank.html
Possible fixes
* igt@i915_module_load@reload-with-fault-injection:
- {fi-icl-u4}:[DMESG-WARN][7] ([fdo#105602] / [fdo#106107] /
[fdo#106350]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-icl-u4/igt@i915_module_l...@reload-with-fault-injection.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-icl-u4/igt@i915_module_l...@reload-with-fault-injection.html
* igt@i915_selftest@live_hangcheck:
- fi-icl-u2: [INCOMPLETE][9] ([fdo#107713] / [fdo#108569]) ->
[PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][11] -> [PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-kbl-7500u/igt@kms_chamel...@hdmi-hpd-fast.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-kbl-7500u/igt@kms_chamel...@hdmi-hpd-fast.html
* igt@kms_psr@sprite_plane_onoff:
- {fi-icl-u4}:[DMESG-WARN][13] ([fdo#105602]) -> [PASS][14] +30
similar issues
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-icl-u4/igt@kms_psr@sprite_plane_onoff.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-icl-u4/igt@kms_psr@sprite_plane_onoff.html
* igt@prime_vgem@basic-fence-flip:
- fi-ilk-650: [DMESG-WARN][15] ([fdo#106387]) -> [PASS][16] +1
similar issue
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6495/fi-ilk-650/igt@prime_v...@basic-fence-flip.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13665/fi-ilk-650/igt@prime_v...@basic-fence-flip.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
[fdo#106350]: https://bugs.freedesktop.org/show_bug.cgi?id=106350
[fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
[fdo#110503]: https://bugs.freedesktop.org/show_bug.cgi?id=110503
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046
[fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
Participating hosts (54 -> 47)
--
Additional (1): fi-icl-dsi
Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-
* Linux: CI_DRM_6495 -> Patchwork_13665
CI_DRM_6495:
Re: [Intel-gfx] [PATCH 03/22] drm/i915/tgl: update ddi/tc clock_off bits
>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lucas De Marchi
>Sent: Friday, July 12, 2019 6:09 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Mahesh Kumar
>Subject: [Intel-gfx] [PATCH 03/22] drm/i915/tgl: update ddi/tc clock_off bits
>
>From: Mahesh Kumar
>
>In GEN 12 PORT_C DDI clk_off bit is not equally distanced to A/B, it's at
>offset 24.
>Similarly TC port (5/6) clk off bits are at offset 22/23. Extend the macros to
>cover
>the additional ports.
>
>Cc: Matt Roper
>Signed-off-by: Mahesh Kumar
>Signed-off-by: Lucas De Marchi
Checked with spec, looks good.
Reviewed-by: Anusha Srivatsa
>---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index def71fd2e4d1..d873d9fbbf0e 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -9749,8 +9749,9 @@ enum skl_power_gate {
>
> #define ICL_DPCLKA_CFGCR0 _MMIO(0x164280)
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) (1 << _PICK(phy, 10, 11,
>24))
>-#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) ==
>PORT_TC4 ? \
>-21 : (tc_port) + 12))
>+#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port)(1 << ((tc_port) <
>PORT_TC4 ? \
>+ (tc_port) + 12 : \
>+ (tc_port) - PORT_TC4 +
>21))
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2)
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 <<
>ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> #define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) <<
>ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
>--
>2.21.0
>
>___
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/gem: don't force writecombine mmap'ing
== Series Details == Series: series starting with [v3,1/3] drm/gem: don't force writecombine mmap'ing URL : https://patchwork.freedesktop.org/series/63770/ State : warning == Summary == $ dim checkpatch origin/drm-tip b6ec1673874e drm/gem: don't force writecombine mmap'ing 3e4afe6255ab drm: plumb attaching dev thru to prime_pin/unpin -:145: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files #145: FILE: drivers/gpu/drm/nouveau/nouveau_gem.h:35: +extern int nouveau_gem_prime_pin(struct drm_gem_object *, struct device *); -:145: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct drm_gem_object *' should also have an identifier name #145: FILE: drivers/gpu/drm/nouveau/nouveau_gem.h:35: +extern int nouveau_gem_prime_pin(struct drm_gem_object *, struct device *); -:145: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name #145: FILE: drivers/gpu/drm/nouveau/nouveau_gem.h:35: +extern int nouveau_gem_prime_pin(struct drm_gem_object *, struct device *); -:148: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files #148: FILE: drivers/gpu/drm/nouveau/nouveau_gem.h:37: +extern void nouveau_gem_prime_unpin(struct drm_gem_object *, struct device *); -:148: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct drm_gem_object *' should also have an identifier name #148: FILE: drivers/gpu/drm/nouveau/nouveau_gem.h:37: +extern void nouveau_gem_prime_unpin(struct drm_gem_object *, struct device *); -:148: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name #148: FILE: drivers/gpu/drm/nouveau/nouveau_gem.h:37: +extern void nouveau_gem_prime_unpin(struct drm_gem_object *, struct device *); total: 0 errors, 4 warnings, 2 checks, 191 lines checked 1eacd563d4f1 drm/vgem: use normal cached mmap'ings ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vbt: Fix VBT parsing for the PSR section
On Tue, 2019-07-16 at 15:03 -0700, Dhinakaran Pandiyan wrote:
> A single 32-bit PSR2 training pattern field follows the sixteen element
> array of PSR table entries in the VBT spec. But, we incorrectly define
> this PSR2 field for each of the PSR table entries. As a result, the PSR1
> training pattern duration for any panel_type != 0 will be parsed
> incorrectly. Secondly, PSR2 training pattern durations for VBTs with bdb
> version >= 226 will also be wrong.
This was reported and bisected by
Aliaksei Urbanski here - https://bugzilla.kernel.org/show_bug.cgi?id=204183
I'll add Bugzilla after the fix is confirmed.
Cc: Rodrigo Vivi
Cc: José Roberto de Souza
Fixes: 88a0d9606aff ("drm/i915/vbt: Parse and use the new field with PSR2 TP2/3
wakeup time")z
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/display/intel_bios.c | 2 +-
drivers/gpu/drm/i915/display/intel_vbt_defs.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
b/drivers/gpu/drm/i915/display/intel_bios.c
index 21501d565327..b416b394b641 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -766,7 +766,7 @@ parse_psr(struct drm_i915_private *dev_priv, const struct
bdb_header *bdb)
}
if (bdb->version >= 226) {
- u32 wakeup_time = psr_table->psr2_tp2_tp3_wakeup_time;
+ u32 wakeup_time = psr->psr2_tp2_tp3_wakeup_time;
wakeup_time = (wakeup_time >> (2 * panel_type)) & 0x3;
switch (wakeup_time) {
diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index 93f5c9d204d6..09cd37fb0b1c 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -481,13 +481,13 @@ struct psr_table {
/* TP wake up time in multiple of 100 */
u16 tp1_wakeup_time;
u16 tp2_tp3_wakeup_time;
-
- /* PSR2 TP2/TP3 wakeup time for 16 panels */
- u32 psr2_tp2_tp3_wakeup_time;
} __packed;
struct bdb_psr {
struct psr_table psr_table[16];
+
+ /* PSR2 TP2/TP3 wakeup time for 16 panels */
+ u32 psr2_tp2_tp3_wakeup_time;
} __packed;
/*
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/vbt: Fix VBT parsing for the PSR section
A single 32-bit PSR2 training pattern field follows the sixteen element
array of PSR table entries in the VBT spec. But, we incorrectly define
this PSR2 field for each of the PSR table entries. As a result, the PSR1
training pattern duration for any panel_type != 0 will be parsed
incorrectly. Secondly, PSR2 training pattern durations for VBTs with bdb
version >= 226 will also be wrong.
Cc: Rodrigo Vivi
Cc: José Roberto de Souza
Fixes: 88a0d9606aff ("drm/i915/vbt: Parse and use the new field with PSR2 TP2/3
wakeup time")
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/display/intel_bios.c | 2 +-
drivers/gpu/drm/i915/display/intel_vbt_defs.h | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
b/drivers/gpu/drm/i915/display/intel_bios.c
index 21501d565327..b416b394b641 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -766,7 +766,7 @@ parse_psr(struct drm_i915_private *dev_priv, const struct
bdb_header *bdb)
}
if (bdb->version >= 226) {
- u32 wakeup_time = psr_table->psr2_tp2_tp3_wakeup_time;
+ u32 wakeup_time = psr->psr2_tp2_tp3_wakeup_time;
wakeup_time = (wakeup_time >> (2 * panel_type)) & 0x3;
switch (wakeup_time) {
diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index 93f5c9d204d6..09cd37fb0b1c 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -481,13 +481,13 @@ struct psr_table {
/* TP wake up time in multiple of 100 */
u16 tp1_wakeup_time;
u16 tp2_tp3_wakeup_time;
-
- /* PSR2 TP2/TP3 wakeup time for 16 panels */
- u32 psr2_tp2_tp3_wakeup_time;
} __packed;
struct bdb_psr {
struct psr_table psr_table[16];
+
+ /* PSR2 TP2/TP3 wakeup time for 16 panels */
+ u32 psr2_tp2_tp3_wakeup_time;
} __packed;
/*
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 3/3] drm/vgem: use normal cached mmap'ings
From: Rob Clark
Since there is no real device associated with VGEM, it is impossible to
end up with appropriate dev->dma_ops, meaning that we have no way to
invalidate the shmem pages allocated by VGEM. So, at least on platforms
without drm_cflush_pages(), we end up with corruption when cache lines
from previous usage of VGEM bo pages get evicted to memory.
The only sane option is to use cached mappings.
Signed-off-by: Rob Clark
---
v3: rebased on drm-tip
drivers/gpu/drm/vgem/vgem_drv.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index e7d12e93b1f0..84262e2bd7f7 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -259,9 +259,6 @@ static int vgem_mmap(struct file *filp, struct
vm_area_struct *vma)
if (ret)
return ret;
- /* Keep the WC mmaping set by drm_gem_mmap() but our pages
-* are ordinary and not special.
-*/
vma->vm_flags = flags | VM_DONTEXPAND | VM_DONTDUMP;
return 0;
}
@@ -310,17 +307,17 @@ static void vgem_unpin_pages(struct drm_vgem_gem_object
*bo)
static int vgem_prime_pin(struct drm_gem_object *obj, struct device *dev)
{
struct drm_vgem_gem_object *bo = to_vgem_bo(obj);
- long n_pages = obj->size >> PAGE_SHIFT;
+ long i, n_pages = obj->size >> PAGE_SHIFT;
struct page **pages;
pages = vgem_pin_pages(bo);
if (IS_ERR(pages))
return PTR_ERR(pages);
- /* Flush the object from the CPU cache so that importers can rely
-* on coherent indirect access via the exported dma-address.
-*/
- drm_clflush_pages(pages, n_pages);
+ for (i = 0; i < n_pages; i++) {
+ dma_sync_single_for_device(dev, page_to_phys(pages[i]),
+ PAGE_SIZE, DMA_BIDIRECTIONAL);
+ }
return 0;
}
@@ -328,6 +325,13 @@ static int vgem_prime_pin(struct drm_gem_object *obj,
struct device *dev)
static void vgem_prime_unpin(struct drm_gem_object *obj, struct device *dev)
{
struct drm_vgem_gem_object *bo = to_vgem_bo(obj);
+ long i, n_pages = obj->size >> PAGE_SHIFT;
+ struct page **pages = bo->pages;
+
+ for (i = 0; i < n_pages; i++) {
+ dma_sync_single_for_cpu(dev, page_to_phys(pages[i]),
+ PAGE_SIZE, DMA_BIDIRECTIONAL);
+ }
vgem_unpin_pages(bo);
}
@@ -382,7 +386,7 @@ static void *vgem_prime_vmap(struct drm_gem_object *obj)
if (IS_ERR(pages))
return NULL;
- return vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL));
+ return vmap(pages, n_pages, 0, PAGE_KERNEL);
}
static void vgem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
@@ -411,7 +415,7 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
fput(vma->vm_file);
vma->vm_file = get_file(obj->filp);
vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
- vma->vm_page_prot =
pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
return 0;
}
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/3] drm: plumb attaching dev thru to prime_pin/unpin
From: Rob Clark
Needed in the following patch for cache operations.
Signed-off-by: Rob Clark
---
v3: rebased on drm-tip
drivers/gpu/drm/drm_gem.c | 8
drivers/gpu/drm/drm_internal.h | 4 ++--
drivers/gpu/drm/drm_prime.c | 4 ++--
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 ++--
drivers/gpu/drm/msm/msm_drv.h | 4 ++--
drivers/gpu/drm/msm/msm_gem_prime.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_gem.h | 4 ++--
drivers/gpu/drm/nouveau/nouveau_prime.c | 4 ++--
drivers/gpu/drm/qxl/qxl_prime.c | 4 ++--
drivers/gpu/drm/radeon/radeon_prime.c | 4 ++--
drivers/gpu/drm/vgem/vgem_drv.c | 4 ++--
include/drm/drm_drv.h | 5 ++---
12 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 84689ccae885..af2549c45027 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1215,22 +1215,22 @@ void drm_gem_print_info(struct drm_printer *p, unsigned
int indent,
obj->dev->driver->gem_print_info(p, indent, obj);
}
-int drm_gem_pin(struct drm_gem_object *obj)
+int drm_gem_pin(struct drm_gem_object *obj, struct device *dev)
{
if (obj->funcs && obj->funcs->pin)
return obj->funcs->pin(obj);
else if (obj->dev->driver->gem_prime_pin)
- return obj->dev->driver->gem_prime_pin(obj);
+ return obj->dev->driver->gem_prime_pin(obj, dev);
else
return 0;
}
-void drm_gem_unpin(struct drm_gem_object *obj)
+void drm_gem_unpin(struct drm_gem_object *obj, struct device *dev)
{
if (obj->funcs && obj->funcs->unpin)
obj->funcs->unpin(obj);
else if (obj->dev->driver->gem_prime_unpin)
- obj->dev->driver->gem_prime_unpin(obj);
+ obj->dev->driver->gem_prime_unpin(obj, dev);
}
void *drm_gem_vmap(struct drm_gem_object *obj)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 51a2055c8f18..e64090373e3a 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -133,8 +133,8 @@ void drm_gem_release(struct drm_device *dev, struct
drm_file *file_private);
void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
const struct drm_gem_object *obj);
-int drm_gem_pin(struct drm_gem_object *obj);
-void drm_gem_unpin(struct drm_gem_object *obj);
+int drm_gem_pin(struct drm_gem_object *obj, struct device *dev);
+void drm_gem_unpin(struct drm_gem_object *obj, struct device *dev);
void *drm_gem_vmap(struct drm_gem_object *obj);
void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 189d980402ad..126860432ff9 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -575,7 +575,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
{
struct drm_gem_object *obj = dma_buf->priv;
- return drm_gem_pin(obj);
+ return drm_gem_pin(obj, attach->dev);
}
EXPORT_SYMBOL(drm_gem_map_attach);
@@ -593,7 +593,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
{
struct drm_gem_object *obj = dma_buf->priv;
- drm_gem_unpin(obj);
+ drm_gem_unpin(obj, attach->dev);
}
EXPORT_SYMBOL(drm_gem_map_detach);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index a05292e8ed6f..67e69a5f00f2 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -43,7 +43,7 @@ int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
}
-int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
+int etnaviv_gem_prime_pin(struct drm_gem_object *obj, struct device *dev)
{
if (!obj->import_attach) {
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
@@ -55,7 +55,7 @@ int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
return 0;
}
-void etnaviv_gem_prime_unpin(struct drm_gem_object *obj)
+void etnaviv_gem_prime_unpin(struct drm_gem_object *obj, struct device *dev)
{
if (!obj->import_attach) {
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index ee7b512dc158..0eea68618b68 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -288,8 +288,8 @@ void msm_gem_prime_vunmap(struct drm_gem_object *obj, void
*vaddr);
int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
struct drm_gem_object *msm_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sg);
-int msm_gem_prime_pin(struct drm_gem_object *obj);
-void
[Intel-gfx] [PATCH v3 1/3] drm/gem: don't force writecombine mmap'ing
From: Rob Clark The driver should be in control of this. Signed-off-by: Rob Clark --- It is possible that this was masking bugs (ie. not setting appropriate pgprot) in drivers. I don't have a particularly good idea for tracking those down (since I don't have the hw for most drivers). Unless someone has a better idea, maybe land this and let driver maintainers fix any potential fallout in their drivers? This is necessary for the last patch to fix VGEM brokenness on arm. v3: rebased on drm-tip drivers/gpu/drm/drm_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index e6c12c6ec728..84689ccae885 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1109,7 +1109,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); /* Take a ref for this mapping of the object, so that the fault -- 2.21.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/22] drm/i915/tgl: select correct bit for port select
>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lucas De Marchi
>Sent: Friday, July 12, 2019 6:09 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Mahesh Kumar
>Subject: [Intel-gfx] [PATCH 02/22] drm/i915/tgl: select correct bit for port
>select
>
>From: Mahesh Kumar
>
>Bit definitions for port-select got changed for TRANS_CLK_SEL &
>TRANS_DDI_FUNC_CTL registers in TGL.
>
>v2 (Lucas):
> - Nuke TRANS_DDI_PORT_NONE since it's 0: we are already clearing
>{TGL_,}TRANS_DDI_PORT_MASK (suggested by Ville)
> - Also cover haswell_get_ddi_port_state() in intel_display.c that was
>missing
> - Define macros using the _SHIFT macros so we don't lose other users
Looks good.
Reviewed-by: Anusha Srivatsa
>Cc: Ville Syrjälä
>Signed-off-by: Mahesh Kumar
>Signed-off-by: Lucas De Marchi
>---
> drivers/gpu/drm/i915/display/intel_ddi.c | 47 +++-
> drivers/gpu/drm/i915/display/intel_display.c | 6 ++-
> drivers/gpu/drm/i915/i915_reg.h | 11 +++--
> 3 files changed, 50 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 8445244aa593..339c01e567ab 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -1773,7 +1773,10 @@ void intel_ddi_enable_transcoder_func(const struct
>intel_crtc_state *crtc_state)
>
> /* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
> temp = TRANS_DDI_FUNC_ENABLE;
>- temp |= TRANS_DDI_SELECT_PORT(port);
>+ if (INTEL_GEN(dev_priv) >= 12)
>+ temp |= TGL_TRANS_DDI_SELECT_PORT(port);
>+ else
>+ temp |= TRANS_DDI_SELECT_PORT(port);
>
> switch (crtc_state->pipe_bpp) {
> case 18:
>@@ -1853,8 +1856,13 @@ void intel_ddi_disable_transcoder_func(const struct
>intel_crtc_state *crtc_state
> i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> u32 val = I915_READ(reg);
>
>- val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
>TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
>- val |= TRANS_DDI_PORT_NONE;
>+ if (INTEL_GEN(dev_priv) >= 12) {
>+ val &= ~(TRANS_DDI_FUNC_ENABLE |
>TGL_TRANS_DDI_PORT_MASK |
>+ TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
>+ } else {
>+ val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK
>|
>+ TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
>+ }
> I915_WRITE(reg, val);
>
> if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME && @@ -
>2006,10 +2014,19 @@ static void intel_ddi_get_encoder_pipes(struct
>intel_encoder *encoder,
> mst_pipe_mask = 0;
> for_each_pipe(dev_priv, p) {
> enum transcoder cpu_transcoder = (enum transcoder)p;
>+ unsigned int port_mask, ddi_select;
>+
>+ if (INTEL_GEN(dev_priv) >= 12) {
>+ port_mask = TGL_TRANS_DDI_PORT_MASK;
>+ ddi_select = TGL_TRANS_DDI_SELECT_PORT(port);
>+ } else {
>+ port_mask = TRANS_DDI_PORT_MASK;
>+ ddi_select = TRANS_DDI_SELECT_PORT(port);
>+ }
>
> tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>
>- if ((tmp & TRANS_DDI_PORT_MASK) !=
>TRANS_DDI_SELECT_PORT(port))
>+ if ((tmp & port_mask) != ddi_select)
> continue;
>
> if ((tmp & TRANS_DDI_MODE_SELECT_MASK) == @@ -2126,9
>+2143,14 @@ void intel_ddi_enable_pipe_clock(const struct intel_crtc_state
>*crtc_state)
> enum port port = encoder->port;
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
>- if (cpu_transcoder != TRANSCODER_EDP)
>- I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
>- TRANS_CLK_SEL_PORT(port));
>+ if (cpu_transcoder != TRANSCODER_EDP) {
>+ if (INTEL_GEN(dev_priv) >= 12)
>+ I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
>+ TGL_TRANS_CLK_SEL_PORT(port));
>+ else
>+ I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
>+ TRANS_CLK_SEL_PORT(port));
>+ }
> }
>
> void intel_ddi_disable_pipe_clock(const struct intel_crtc_state *crtc_state)
> @@
>-2136,9 +2158,14 @@ void intel_ddi_disable_pipe_clock(const struct
>intel_crtc_state *crtc_state)
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
>- if (cpu_transcoder != TRANSCODER_EDP)
>- I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
>- TRANS_CLK_SEL_DISABLED);
>+ if (cpu_transcoder != TRANSCODER_EDP) {
>+ if (INTEL_GEN(dev_priv) >= 12)
>+ I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
>+
[Intel-gfx] [CI] drm/i915/oa: Reconfigure contexts on the fly
Avoid a global idle barrier by reconfiguring each context by rewriting
them with MI_STORE_DWORD from the kernel context.
v2: We only need to determine the desired register values once, they are
the same for all contexts.
v3: Don't remove the kernel context from the list of known GEM contexts;
the world is not ready for that yet.
Signed-off-by: Chris Wilson
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Reviewed-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 23 +-
drivers/gpu/drm/i915/gt/intel_context.c | 25 ++
drivers/gpu/drm/i915/gt/intel_context.h | 3 +
drivers/gpu/drm/i915/gt/intel_lrc.c | 7 +-
drivers/gpu/drm/i915/i915_perf.c| 243 +++-
5 files changed, 221 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c5f8bfa3f7b0..ffb59d96d4d8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1173,26 +1173,11 @@ gen8_modify_rpcs(struct intel_context *ce, struct
intel_sseu sseu)
if (IS_ERR(rq))
return PTR_ERR(rq);
- /* Queue this switch after all other activity by this context. */
- ret = i915_active_request_set(>ring->timeline->last_request, rq);
- if (ret)
- goto out_add;
-
- /*
-* Guarantee context image and the timeline remains pinned until the
-* modifying request is retired by setting the ce activity tracker.
-*
-* But we only need to take one pin on the account of it. Or in other
-* words transfer the pinned ce object to tracked active request.
-*/
- GEM_BUG_ON(i915_active_is_idle(>active));
- ret = i915_active_ref(>active, rq->fence.context, rq);
- if (ret)
- goto out_add;
-
- ret = gen8_emit_rpcs_config(rq, ce, sseu);
+ /* Serialise with the remote context */
+ ret = intel_context_prepare_remote_request(ce, rq);
+ if (ret == 0)
+ ret = gen8_emit_rpcs_config(rq, ce, sseu);
-out_add:
i915_request_add(rq);
return ret;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index 1110fc8f657a..b667e2b35804 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -239,6 +239,31 @@ void intel_context_exit_engine(struct intel_context *ce)
intel_engine_pm_put(ce->engine);
}
+int intel_context_prepare_remote_request(struct intel_context *ce,
+struct i915_request *rq)
+{
+ struct intel_timeline *tl = ce->ring->timeline;
+ int err;
+
+ /* Only suitable for use in remotely modifying this context */
+ GEM_BUG_ON(rq->hw_context == ce);
+
+ /* Queue this switch after all other activity by this context. */
+ err = i915_active_request_set(>last_request, rq);
+ if (err)
+ return err;
+
+ /*
+* Guarantee context image and the timeline remains pinned until the
+* modifying request is retired by setting the ce activity tracker.
+*
+* But we only need to take one pin on the account of it. Or in other
+* words transfer the pinned ce object to tracked active request.
+*/
+ GEM_BUG_ON(i915_active_is_idle(>active));
+ return i915_active_ref(>active, rq->fence.context, rq);
+}
+
struct i915_request *intel_context_create_request(struct intel_context *ce)
{
struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h
b/drivers/gpu/drm/i915/gt/intel_context.h
index 40cd8320fcc3..b41c610c2ce6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -139,6 +139,9 @@ static inline void intel_context_timeline_unlock(struct
intel_context *ce)
mutex_unlock(>ring->timeline->mutex);
}
+int intel_context_prepare_remote_request(struct intel_context *ce,
+struct i915_request *rq);
+
struct i915_request *intel_context_create_request(struct intel_context *ce);
#endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a220575a69bc..f35a57d6d34a 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1576,9 +1576,12 @@ __execlists_update_reg_state(struct intel_context *ce,
regs[CTX_RING_TAIL + 1] = ring->tail;
/* RPCS */
- if (engine->class == RENDER_CLASS)
+ if (engine->class == RENDER_CLASS) {
regs[CTX_R_PWR_CLK_STATE + 1] =
intel_sseu_make_rpcs(engine->i915, >sseu);
+
+ i915_oa_init_reg_state(engine, ce, regs);
+ }
}
static int
@@ -3001,8 +3004,6 @@ static void execlists_init_reg_state(u32 *regs,
if (rcs) {
Re: [Intel-gfx] [PATCH] drm/dp/dsc: Add Support for all BPCs supported by TGL
On Tue, Jul 16, 2019 at 01:26:19PM -0700, Srivatsa, Anusha wrote:
>
>
> >-Original Message-
> >From: Navare, Manasi D
> >Sent: Tuesday, July 16, 2019 11:15 AM
> >To: Srivatsa, Anusha
> >Cc: intel-gfx@lists.freedesktop.org; Ville Syrjälä
> >
> >Subject: Re: [PATCH] drm/dp/dsc: Add Support for all BPCs supported by TGL
> >
> >On Mon, Jul 15, 2019 at 04:40:56PM -0700, Anusha Srivatsa wrote:
> >> DSC engine on ICL supports only 8 and 10 BPC as the input BPC. But DSC
> >> engine in TGL supports 8, 10 and 12 BPC.
> >> Add 12 BPC support for DSC while calculating compression
> >> configuration.
> >>
> >> v2: Remove the separate define TGL_DP_DSC_MAX_SUPPORTED_BPC and use
> >> the value directly.(More such defines can be removed as part of future
> >> patches). (Ville)
> >
> >IMO what Ville asked to do in his comment was to remove all the #defines for
> >the
> >max DSC BPC so remove the ones for 8 and 10 also to mak eit more readable and
> >that user does not have to hunt for the #defines for either of these.
>
> Yes those changes can be part of a following patch. This is TGL specific.
IMO, you could just make them in the same patch since they are minor changes
happening because of diferent max limits for TGL, so just say in the commit
message
while at it also remove the #defines and use the values directly for ICL limits
as well.
Manasi
>
> Anusha
> >>
> >> Cc: Ville Syrjälä
> >> Cc: Manasi Navare
> >> Signed-off-by: Anusha Srivatsa
> >> ---
> >> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 0eb5d66f87a7..aa8bfb754fc1 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -1914,8 +1914,12 @@ static int intel_dp_dsc_compute_config(struct
> >intel_dp *intel_dp,
> >>if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> >>return -EINVAL;
> >>
> >> - dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
> >> - conn_state->max_requested_bpc);
> >> + /* Max DSC Input BPC for TGL+ is 12 */
> >> + if (INTEL_GEN(dev_priv) >= 12)
> >> + dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
> >> + else
> >> + dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
> >> + conn_state->max_requested_bpc);
> >
> >Use 10 here directly
> >
> >>
> >>pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, dsc_max_bpc);
> >>if (pipe_bpp < DP_DSC_MIN_SUPPORTED_BPC * 3) {
> >
> >Use 8 here directly
> >
> >Manasi
> >> --
> >> 2.21.0
> >>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 1/6] drm: Add Content protection type property
On Fri, Jul 12, 2019 at 02:39:05PM +0300, Pekka Paalanen wrote:
> On Thu, 11 Jul 2019 10:18:22 -0400
> Sean Paul wrote:
>
> > On Mon, Jul 08, 2019 at 04:51:11PM +0530, Ramalingam C wrote:
> > > This patch adds a DRM ENUM property to the selected connectors.
> > > This property is used for mentioning the protected content's type
> > > from userspace to kernel HDCP authentication.
> > >
> > > Type of the stream is decided by the protected content providers.
> > > Type 0 content can be rendered on any HDCP protected display wires.
> > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > >
> > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > for type 1. Else HDCP enable will be failed.
> > >
> > > Need ACK for this new conenctor property from userspace consumer.
>
> ...
>
> > > diff --git a/drivers/gpu/drm/drm_connector.c
> > > b/drivers/gpu/drm/drm_connector.c
> > > index 068d4b05f1be..732f6645643d 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list
> > > hdmi_colorspaces[] = {
> > > * is no longer protected and userspace should take appropriate
> > > action
> > > * (whatever that might be).
> > > *
> > > + * HDCP Content Type:
> > > + * This Enum property is used by the userspace to declare the
> > > content type
> > > + * of the display stream, to kernel. Here display stream stands
> > > for any
> > > + * display content that userspace intended to render with HDCP
> > > encryption.
> > > + *
> > > + * Content Type of a stream is decided by the owner of the stream,
> > > as
> > > + * "HDCP Type0" or "HDCP Type1".
> > > + *
> > > + * The value of the property can be one the below:
> > > + * - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > + * - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > + *
> > > + * When kernel starts the HDCP authentication upon the "DESIRED"
> > > state of
> > > + * the "Content Protection", it refers the "HDCP Content Type"
> > > property
> > > + * state. And perform the HDCP authentication with the display
> > > sink for
> > > + * the content type mentioned by "HDCP Content Type".
> > > + *
> > > + * Stream classified as HDCP Type0 can be transmitted on a link
> > > which is
> > > + * encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > + * and more).
> > > + *
> > > + * Streams classified as HDCP Type1 can be transmitted on a link
> > > which is
> > > + * encrypted only with HDCP 2.2. In future, HDCP versions >2.2
> > > also might
> > > + * support Type1 based on their spec.
> > > + *
> > > + * HDCP2.2 authentication protocol itself takes the "Content Type"
> > > as a
> > > + * parameter, which is a input for the DP HDCP2.2 encryption algo.
> > > + *
> > > + * Note that the HDCP Content Type property is introduced at HDCP
> > > 2.2, and
> > > + * defaults to type 0. It is only exposed by drivers supporting
> > > HDCP 2.2.
> > > + * Based on how next versions of HDCP specs are defined content
> > > Type could
> > > + * be used for higher versions too.
> > > + *
> > > + * If content type is changed when "Content Protection" is not
> > > UNDESIRED,
> > > + * then kernel will disable the HDCP and re-enable with new type
> > > in the
> > > + * same atomic commit. And when "Content Protection" is ENABLED,
> > > it means
> > > + * that link is HDCP authenticated and encrypted, for the
> > > transmission of
> > > + * the Type of stream mentioned at "HDCP Content Type".
> > > + *
> > > * HDR_OUTPUT_METADATA:
> > > * Connector property to enable userspace to send HDR Metadata to
> > > * driver. This metadata is based on the composition and blending
> >
> > Do we actually need an entirely new property? Can't we just add a new
> > entry to the existing Content Protection property which is "Desired Type1"
> > or
> > similar? The kernel will then either attempt 2.2 auth or it will ignore it
> > the
> > request if it's not supported.
>
> Hi,
>
> IMHO the existing "Content Protection" property is already complicated
> enough that one should not add anything new to it.
>
> If you added "desired-type-1", the readback of it would become
> ambiguous if it was "ENABLED", userspace would not know if the value
> written was "DESIRED" or "desired-type-1". Sure, it's not a problem
> when a display server knows what it just wrote into it, but shouldn't
> we try to keep KMS state readable as well, if for nothing but debugging?
Yeah, that's a fair point, it would also create a problem if the HDCP version
was somehow downgraded between u/s polling the property.
>
> I think using the same property for
Re: [Intel-gfx] [PATCH] drm/dp/dsc: Add Support for all BPCs supported by TGL
>-Original Message-
>From: Navare, Manasi D
>Sent: Tuesday, July 16, 2019 11:15 AM
>To: Srivatsa, Anusha
>Cc: intel-gfx@lists.freedesktop.org; Ville Syrjälä
>
>Subject: Re: [PATCH] drm/dp/dsc: Add Support for all BPCs supported by TGL
>
>On Mon, Jul 15, 2019 at 04:40:56PM -0700, Anusha Srivatsa wrote:
>> DSC engine on ICL supports only 8 and 10 BPC as the input BPC. But DSC
>> engine in TGL supports 8, 10 and 12 BPC.
>> Add 12 BPC support for DSC while calculating compression
>> configuration.
>>
>> v2: Remove the separate define TGL_DP_DSC_MAX_SUPPORTED_BPC and use
>> the value directly.(More such defines can be removed as part of future
>> patches). (Ville)
>
>IMO what Ville asked to do in his comment was to remove all the #defines for
>the
>max DSC BPC so remove the ones for 8 and 10 also to mak eit more readable and
>that user does not have to hunt for the #defines for either of these.
Yes those changes can be part of a following patch. This is TGL specific.
Anusha
>>
>> Cc: Ville Syrjälä
>> Cc: Manasi Navare
>> Signed-off-by: Anusha Srivatsa
>> ---
>> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 0eb5d66f87a7..aa8bfb754fc1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1914,8 +1914,12 @@ static int intel_dp_dsc_compute_config(struct
>intel_dp *intel_dp,
>> if (!intel_dp_supports_dsc(intel_dp, pipe_config))
>> return -EINVAL;
>>
>> -dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
>> -conn_state->max_requested_bpc);
>> +/* Max DSC Input BPC for TGL+ is 12 */
>> +if (INTEL_GEN(dev_priv) >= 12)
>> +dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
>> +else
>> +dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
>> +conn_state->max_requested_bpc);
>
>Use 10 here directly
>
>>
>> pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, dsc_max_bpc);
>> if (pipe_bpp < DP_DSC_MIN_SUPPORTED_BPC * 3) {
>
>Use 8 here directly
>
>Manasi
>> --
>> 2.21.0
>>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] dma-buf: Relax the write-seqlock for reallocating the shared fence list
Quoting Daniel Vetter (2019-07-16 10:21:54) > On Fri, Jul 12, 2019 at 09:03:14AM +0100, Chris Wilson wrote: > > As the set of shared fences is not being changed during reallocation of > > the reservation list, we can skip updating the write_seqlock. > > > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: Christian König > > sounds legit. > > Reviewed-by: Daniel Vetter > > More seriously, I think I convinced myself that we cant see a mess of old > and new fence arrays anywhere, even without the seqlock retry, so I think > we should be all good. Aye, the view remains consistent which is key. Thanks for the review, pushed to drm-misc-next. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/3] drm: plumb attaching dev thru to prime_pin/unpin
Quoting Rob Clark (2019-07-16 18:43:22) > From: Rob Clark > > Needed in the following patch for cache operations. What's the base for this patch? (I'm missing the ancestor for drm_gem.c) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/dp/dsc: Add Support for all BPCs supported by TGL
On Mon, Jul 15, 2019 at 04:40:56PM -0700, Anusha Srivatsa wrote:
> DSC engine on ICL supports only 8 and 10 BPC as the input
> BPC. But DSC engine in TGL supports 8, 10 and 12 BPC.
> Add 12 BPC support for DSC while calculating compression
> configuration.
>
> v2: Remove the separate define TGL_DP_DSC_MAX_SUPPORTED_BPC
> and use the value directly.(More such defines can be removed
> as part of future patches). (Ville)
IMO what Ville asked to do in his comment was to remove all the #defines
for the max DSC BPC so remove the ones for 8 and 10 also to mak eit more
readable
and that user does not have to hunt for the #defines for either of these.
>
> Cc: Ville Syrjälä
> Cc: Manasi Navare
> Signed-off-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0eb5d66f87a7..aa8bfb754fc1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1914,8 +1914,12 @@ static int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
> if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> return -EINVAL;
>
> - dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
> - conn_state->max_requested_bpc);
> + /* Max DSC Input BPC for TGL+ is 12 */
> + if (INTEL_GEN(dev_priv) >= 12)
> + dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
> + else
> + dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
> + conn_state->max_requested_bpc);
Use 10 here directly
>
> pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, dsc_max_bpc);
> if (pipe_bpp < DP_DSC_MIN_SUPPORTED_BPC * 3) {
Use 8 here directly
Manasi
> --
> 2.21.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] drm/vgem: use normal cached mmap'ings
From: Rob Clark
Since there is no real device associated with VGEM, it is impossible to
end up with appropriate dev->dma_ops, meaning that we have no way to
invalidate the shmem pages allocated by VGEM. So, at least on platforms
without drm_cflush_pages(), we end up with corruption when cache lines
from previous usage of VGEM bo pages get evicted to memory.
The only sane option is to use cached mappings.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/vgem/vgem_drv.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index a179e962b79e..b6071a466b92 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -259,9 +259,6 @@ static int vgem_mmap(struct file *filp, struct
vm_area_struct *vma)
if (ret)
return ret;
- /* Keep the WC mmaping set by drm_gem_mmap() but our pages
-* are ordinary and not special.
-*/
vma->vm_flags = flags | VM_DONTEXPAND | VM_DONTDUMP;
return 0;
}
@@ -310,17 +307,17 @@ static void vgem_unpin_pages(struct drm_vgem_gem_object
*bo)
static int vgem_prime_pin(struct drm_gem_object *obj, struct device *dev)
{
struct drm_vgem_gem_object *bo = to_vgem_bo(obj);
- long n_pages = obj->size >> PAGE_SHIFT;
+ long i, n_pages = obj->size >> PAGE_SHIFT;
struct page **pages;
pages = vgem_pin_pages(bo);
if (IS_ERR(pages))
return PTR_ERR(pages);
- /* Flush the object from the CPU cache so that importers can rely
-* on coherent indirect access via the exported dma-address.
-*/
- drm_clflush_pages(pages, n_pages);
+ for (i = 0; i < n_pages; i++) {
+ dma_sync_single_for_device(dev, page_to_phys(pages[i]),
+ PAGE_SIZE, DMA_BIDIRECTIONAL);
+ }
return 0;
}
@@ -328,6 +325,13 @@ static int vgem_prime_pin(struct drm_gem_object *obj,
struct device *dev)
static void vgem_prime_unpin(struct drm_gem_object *obj, struct device *dev)
{
struct drm_vgem_gem_object *bo = to_vgem_bo(obj);
+ long i, n_pages = obj->size >> PAGE_SHIFT;
+ struct page **pages = bo->pages;
+
+ for (i = 0; i < n_pages; i++) {
+ dma_sync_single_for_cpu(dev, page_to_phys(pages[i]),
+ PAGE_SIZE, DMA_BIDIRECTIONAL);
+ }
vgem_unpin_pages(bo);
}
@@ -382,7 +386,7 @@ static void *vgem_prime_vmap(struct drm_gem_object *obj)
if (IS_ERR(pages))
return NULL;
- return vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL));
+ return vmap(pages, n_pages, 0, PAGE_KERNEL);
}
static void vgem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
@@ -411,7 +415,7 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
fput(vma->vm_file);
vma->vm_file = get_file(obj->filp);
vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
- vma->vm_page_prot =
pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
return 0;
}
--
2.21.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: plumb attaching dev thru to prime_pin/unpin
From: Rob Clark
Needed in the following patch for cache operations.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/drm_gem.c | 10 ++
drivers/gpu/drm/drm_gem_vram_helper.c | 6 --
drivers/gpu/drm/drm_prime.c | 4 ++--
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 ++--
drivers/gpu/drm/msm/msm_drv.h | 4 ++--
drivers/gpu/drm/msm/msm_gem_prime.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_gem.h | 4 ++--
drivers/gpu/drm/nouveau/nouveau_prime.c | 4 ++--
drivers/gpu/drm/qxl/qxl_prime.c | 4 ++--
drivers/gpu/drm/radeon/radeon_prime.c | 4 ++--
drivers/gpu/drm/vboxvideo/vbox_prime.c | 4 ++--
drivers/gpu/drm/vgem/vgem_drv.c | 4 ++--
include/drm/drm_drv.h | 4 ++--
include/drm/drm_gem.h | 4 ++--
include/drm/drm_gem_vram_helper.h | 4 ++--
15 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 7d6242cc69f2..0a2645769624 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1219,18 +1219,19 @@ void drm_gem_print_info(struct drm_printer *p, unsigned
int indent,
/**
* drm_gem_pin - Pin backing buffer in memory
* @obj: GEM object
+ * @dev: the device the buffer is being pinned for
*
* Make sure the backing buffer is pinned in memory.
*
* Returns:
* 0 on success or a negative error code on failure.
*/
-int drm_gem_pin(struct drm_gem_object *obj)
+int drm_gem_pin(struct drm_gem_object *obj, struct device *dev)
{
if (obj->funcs && obj->funcs->pin)
return obj->funcs->pin(obj);
else if (obj->dev->driver->gem_prime_pin)
- return obj->dev->driver->gem_prime_pin(obj);
+ return obj->dev->driver->gem_prime_pin(obj, dev);
else
return 0;
}
@@ -1239,15 +1240,16 @@ EXPORT_SYMBOL(drm_gem_pin);
/**
* drm_gem_unpin - Unpin backing buffer from memory
* @obj: GEM object
+ * @dev: the device the buffer is being pinned for
*
* Relax the requirement that the backing buffer is pinned in memory.
*/
-void drm_gem_unpin(struct drm_gem_object *obj)
+void drm_gem_unpin(struct drm_gem_object *obj, struct device *dev)
{
if (obj->funcs && obj->funcs->unpin)
obj->funcs->unpin(obj);
else if (obj->dev->driver->gem_prime_unpin)
- obj->dev->driver->gem_prime_unpin(obj);
+ obj->dev->driver->gem_prime_unpin(obj, dev);
}
EXPORT_SYMBOL(drm_gem_unpin);
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 4de782ca26b2..62fafec93948 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -548,7 +548,8 @@ EXPORT_SYMBOL(drm_gem_vram_driver_dumb_mmap_offset);
* 0 on success, or
* a negative errno code otherwise.
*/
-int drm_gem_vram_driver_gem_prime_pin(struct drm_gem_object *gem)
+int drm_gem_vram_driver_gem_prime_pin(struct drm_gem_object *gem,
+ struct device *dev)
{
struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
@@ -569,7 +570,8 @@ EXPORT_SYMBOL(drm_gem_vram_driver_gem_prime_pin);
Implements drm_driver.gem_prime_unpin
* @gem: The GEM object to unpin
*/
-void drm_gem_vram_driver_gem_prime_unpin(struct drm_gem_object *gem)
+void drm_gem_vram_driver_gem_prime_unpin(struct drm_gem_object *gem,
+struct device *dev)
{
struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index d0c01318076b..505893cfac8e 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -196,7 +196,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
{
struct drm_gem_object *obj = dma_buf->priv;
- return drm_gem_pin(obj);
+ return drm_gem_pin(obj, attach->dev);
}
EXPORT_SYMBOL(drm_gem_map_attach);
@@ -213,7 +213,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
{
struct drm_gem_object *obj = dma_buf->priv;
- drm_gem_unpin(obj);
+ drm_gem_unpin(obj, attach->dev);
}
EXPORT_SYMBOL(drm_gem_map_detach);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 00e8b6a817e3..44385d590aa7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -43,7 +43,7 @@ int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
}
-int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
+int etnaviv_gem_prime_pin(struct drm_gem_object *obj, struct device *dev)
{
if (!obj->import_attach) {
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
@@ -55,7 +55,7 @@ int etnaviv_gem_prime_pin(struct
[Intel-gfx] [PATCH v2 1/3] drm/gem: don't force writecombine mmap'ing
From: Rob Clark The driver should be in control of this. Signed-off-by: Rob Clark --- It is possible that this was masking bugs (ie. not setting appropriate pgprot) in drivers. I don't have a particularly good idea for tracking those down (since I don't have the hw for most drivers). Unless someone has a better idea, maybe land this and let driver maintainers fix any potential fallout in their drivers? This is necessary for the last patch to fix VGEM brokenness on arm. drivers/gpu/drm/drm_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 8a55f71325b1..7d6242cc69f2 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1110,7 +1110,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); /* Take a ref for this mapping of the object, so that the fault -- 2.21.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] screen freeze with 5.2-rc6 Dell XPS-13 skylake i915
Thanks Paul
Paul and James could you test this final solution(at least for 5.2)?
Please revert the hack patch and apply this one.
Thanks
On Mon, 2019-07-15 at 23:34 +0200, Paul Bolle wrote:
> Hi Jose,
>
> Souza, Jose schreef op ma 15-07-2019 om 21:03 [+]:
> > So the issue did not happened again with the patch applied?
>
> Not in the three days that I've been running 5.2 kernels with the
> hack applied
> (so that should be about twelve hours of proper uptime).
>
> > If you still have the kernel 5.1 installed could you share your
> > /sys/kernel/debug/dri/0/i915_edp_psr_status with the older kernel?
> > We want to check if training values changed between kernel
> > versions.
>
> Sure. On 5.1.17 I get:
> Sink support: yes [0x01]
> PSR mode: PSR1 enabled
> Source PSR ctl: enabled [0x81f00626]
> Source PSR status: IDLE [0x040b0001]
> Busy frontbuffer bits: 0x
>
> And, in case you need it, on 5.2.1+hack I get:
> Sink support: yes [0x01]
> PSR mode: PSR1 enabled
> Source PSR ctl: enabled [0x81f00626]
> Source PSR status: IDLE [0x04030006]
> Busy frontbuffer bits: 0x
>
> Hope this helps,
>
>
> Paul
>
From 5d4fce9889e25828ee35481a09929e8ee616c933 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Roberto=20de=20Souza?=
Date: Tue, 16 Jul 2019 09:26:08 -0700
Subject: [PATCH] Revert "drm/i915/vbt: Parse and use the new field with PSR2
TP2/3 wakeup time"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch is causing PSR_CTL_TP2_TP3 to be set to
PSR_TP2_TP3_TIME_0us while VBT have a different value causing screen
freezing after exiting PSR.
For now lets just revert it and later I will bring it back fixed.
This reverts commit 88a0d9606aff09d2b1c5dbe95a9df9dac44e79b6.
Signed-off-by: José Roberto de Souza
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_bios.c | 25 -
drivers/gpu/drm/i915/intel_psr.c | 8
drivers/gpu/drm/i915/intel_vbt_defs.h | 3 ---
4 files changed, 4 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 066fd2a12851..d37262aa16ca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1013,7 +1013,6 @@ struct intel_vbt_data {
enum psr_lines_to_wait lines_to_wait;
int tp1_wakeup_time_us;
int tp2_tp3_wakeup_time_us;
- int psr2_tp2_tp3_wakeup_time_us;
} psr;
struct {
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1dc8d03ff127..455cc07392af 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -760,31 +760,6 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
dev_priv->vbt.psr.tp1_wakeup_time_us = psr_table->tp1_wakeup_time * 100;
dev_priv->vbt.psr.tp2_tp3_wakeup_time_us = psr_table->tp2_tp3_wakeup_time * 100;
}
-
- if (bdb->version >= 226) {
- u32 wakeup_time = psr_table->psr2_tp2_tp3_wakeup_time;
-
- wakeup_time = (wakeup_time >> (2 * panel_type)) & 0x3;
- switch (wakeup_time) {
- case 0:
- wakeup_time = 500;
- break;
- case 1:
- wakeup_time = 100;
- break;
- case 3:
- wakeup_time = 50;
- break;
- default:
- case 2:
- wakeup_time = 2500;
- break;
- }
- dev_priv->vbt.psr.psr2_tp2_tp3_wakeup_time_us = wakeup_time;
- } else {
- /* Reusing PSR1 wakeup time for PSR2 in older VBTs */
- dev_priv->vbt.psr.psr2_tp2_tp3_wakeup_time_us = dev_priv->vbt.psr.tp2_tp3_wakeup_time_us;
- }
}
static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 963663ba0edf..3926f4bf05f6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -523,12 +523,12 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
- if (dev_priv->vbt.psr.psr2_tp2_tp3_wakeup_time_us >= 0 &&
- dev_priv->vbt.psr.psr2_tp2_tp3_wakeup_time_us <= 50)
+ if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
+ dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
val |= EDP_PSR2_TP2_TIME_50us;
- else if (dev_priv->vbt.psr.psr2_tp2_tp3_wakeup_time_us <= 100)
+ else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 100)
val |= EDP_PSR2_TP2_TIME_100us;
- else if (dev_priv->vbt.psr.psr2_tp2_tp3_wakeup_time_us <= 500)
+ else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 500)
val |= EDP_PSR2_TP2_TIME_500us;
else
val |= EDP_PSR2_TP2_TIME_2500us;
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index fdbbb9a53804..bf3662ad5fed 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -772,9 +772,6 @@ struct psr_table {
/* TP wake up time in multiple of 100 */
u16 tp1_wakeup_time;
u16 tp2_tp3_wakeup_time;
-
-
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/5] drm/i915/userptr: Beware recursive lock_page()
== Series Details == Series: series starting with [1/5] drm/i915/userptr: Beware recursive lock_page() URL : https://patchwork.freedesktop.org/series/63752/ State : success == Summary == CI Bug Log - changes from CI_DRM_6492_full -> Patchwork_13664_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13664_full that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_rc6_residency@rc6-accuracy: - shard-snb: [PASS][1] -> [SKIP][2] ([fdo#109271]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-snb7/igt@i915_pm_rc6_reside...@rc6-accuracy.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-snb6/igt@i915_pm_rc6_reside...@rc6-accuracy.html * igt@kms_cursor_crc@pipe-a-cursor-128x42-sliding: - shard-skl: [PASS][3] -> [FAIL][4] ([fdo#103232]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-skl8/igt@kms_cursor_...@pipe-a-cursor-128x42-sliding.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-skl8/igt@kms_cursor_...@pipe-a-cursor-128x42-sliding.html * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic: - shard-skl: [PASS][5] -> [DMESG-WARN][6] ([fdo#105541]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-skl7/igt@kms_cursor_leg...@long-nonblocking-modeset-vs-cursor-atomic.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-skl7/igt@kms_cursor_leg...@long-nonblocking-modeset-vs-cursor-atomic.html * igt@kms_flip@2x-plain-flip-ts-check: - shard-glk: [PASS][7] -> [FAIL][8] ([fdo#100368]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-glk2/igt@kms_f...@2x-plain-flip-ts-check.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-glk9/igt@kms_f...@2x-plain-flip-ts-check.html * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw: - shard-iclb: [PASS][9] -> [FAIL][10] ([fdo#103167]) +6 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-iclb8/igt@kms_frontbuffer_track...@fbc-1p-pri-indfb-multidraw.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-iclb4/igt@kms_frontbuffer_track...@fbc-1p-pri-indfb-multidraw.html * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite: - shard-glk: [PASS][11] -> [FAIL][12] ([fdo#103167]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-glk7/igt@kms_frontbuffer_track...@fbc-1p-primscrn-spr-indfb-draw-pwrite.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-glk8/igt@kms_frontbuffer_track...@fbc-1p-primscrn-spr-indfb-draw-pwrite.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc: - shard-skl: [PASS][13] -> [FAIL][14] ([fdo#103167]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-skl7/igt@kms_frontbuffer_track...@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-skl8/igt@kms_frontbuffer_track...@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc.html * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min: - shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145]) +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-skl8/igt@kms_plane_alpha_bl...@pipe-a-constant-alpha-min.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-skl8/igt@kms_plane_alpha_bl...@pipe-a-constant-alpha-min.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [PASS][17] -> [FAIL][18] ([fdo#108145] / [fdo#110403]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-skl1/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-skl3/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html * igt@kms_psr2_su@frontbuffer: - shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109642] / [fdo#111068]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-iclb2/igt@kms_psr2...@frontbuffer.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-iclb8/igt@kms_psr2...@frontbuffer.html Possible fixes * igt@kms_cursor_crc@pipe-c-cursor-suspend: - shard-apl: [DMESG-WARN][21] ([fdo#108566]) -> [PASS][22] +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/shard-apl5/igt@kms_cursor_...@pipe-c-cursor-suspend.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/shard-apl7/igt@kms_cursor_...@pipe-c-cursor-suspend.html * igt@kms_cursor_legacy@cursor-vs-flip-toggle: - shard-hsw: [FAIL][23] ([fdo#103355])
Re: [Intel-gfx] [PATCH 4/4] drm/i915/gtt: Tidy up ppgtt insertion for gen8
Quoting Abdiel Janulgue (2019-07-16 16:30:09) > > > On 12/07/2019 14.27, Chris Wilson wrote: > > Apply the new radix shift helpers to extract the multi-level indices > > cleanly when inserting pte into the gtt tree. > > > Reviewed-by: Abdiel Janulgue Thanks for the reviews, -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915/userptr: Beware recursive lock_page()
Quoting Tvrtko Ursulin (2019-07-16 16:25:22) > > On 16/07/2019 13:49, Chris Wilson wrote: > > Following a try_to_unmap() we may want to remove the userptr and so call > > put_pages(). However, try_to_unmap() acquires the page lock and so we > > must avoid recursively locking the pages ourselves -- which means that > > we cannot safely acquire the lock around set_page_dirty(). Since we > > can't be sure of the lock, we have to risk skip dirtying the page, or > > else risk calling set_page_dirty() without a lock and so risk fs > > corruption. > > So if trylock randomly fail we get data corruption in whatever data set > application is working on, which is what the original patch was trying > to avoid? Are we able to detect the backing store type so at least we > don't risk skipping set_page_dirty with anonymous/shmemfs? page->mapping??? We still have the issue that if there is a mapping we should be taking the lock, and we may have both a mapping and be inside try_to_unmap(). I think it's lose-lose. The only way to win is not to userptr :) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/gtt: Tidy up ppgtt insertion for gen8
On 12/07/2019 14.27, Chris Wilson wrote:
> Apply the new radix shift helpers to extract the multi-level indices
> cleanly when inserting pte into the gtt tree.
>
Reviewed-by: Abdiel Janulgue
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 115 +++-
> drivers/gpu/drm/i915/i915_gem_gtt.h | 90 ++
> 2 files changed, 48 insertions(+), 157 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 72e0f9799a46..de78dc8c425c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1131,47 +1131,28 @@ static inline struct sgt_dma {
> return (struct sgt_dma) { sg, addr, addr + sg->length };
> }
>
> -struct gen8_insert_pte {
> - u16 pml4e;
> - u16 pdpe;
> - u16 pde;
> - u16 pte;
> -};
> -
> -static __always_inline struct gen8_insert_pte gen8_insert_pte(u64 start)
> -{
> - return (struct gen8_insert_pte) {
> - gen8_pml4e_index(start),
> - gen8_pdpe_index(start),
> - gen8_pde_index(start),
> - gen8_pte_index(start),
> - };
> -}
> -
> -static __always_inline bool
> +static __always_inline u64
> gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
> struct i915_page_directory *pdp,
> struct sgt_dma *iter,
> - struct gen8_insert_pte *idx,
> + u64 idx,
> enum i915_cache_level cache_level,
> u32 flags)
> {
> struct i915_page_directory *pd;
> const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> gen8_pte_t *vaddr;
> - bool ret;
>
> - GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(>vm));
> - pd = i915_pd_entry(pdp, idx->pdpe);
> - vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
> + pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2));
> + vaddr = kmap_atomic_px(i915_pt_entry(pd, gen8_pd_index(idx, 1)));
> do {
> - vaddr[idx->pte] = pte_encode | iter->dma;
> + vaddr[gen8_pd_index(idx, 0)] = pte_encode | iter->dma;
>
> iter->dma += I915_GTT_PAGE_SIZE;
> if (iter->dma >= iter->max) {
> iter->sg = __sg_next(iter->sg);
> if (!iter->sg) {
> - ret = false;
> + idx = 0;
> break;
> }
>
> @@ -1179,30 +1160,22 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt
> *ppgtt,
> iter->max = iter->dma + iter->sg->length;
> }
>
> - if (++idx->pte == GEN8_PTES) {
> - idx->pte = 0;
> -
> - if (++idx->pde == I915_PDES) {
> - idx->pde = 0;
> -
> + if (gen8_pd_index(++idx, 0) == 0) {
> + if (gen8_pd_index(idx, 1) == 0) {
> /* Limited by sg length for 3lvl */
> - if (++idx->pdpe == GEN8_PML4ES_PER_PML4) {
> - idx->pdpe = 0;
> - ret = true;
> + if (gen8_pd_index(idx, 2) == 0)
> break;
> - }
>
> - GEM_BUG_ON(idx->pdpe >=
> i915_pdpes_per_pdp(>vm));
> - pd = pdp->entry[idx->pdpe];
> + pd = pdp->entry[gen8_pd_index(idx, 2)];
> }
>
> kunmap_atomic(vaddr);
> - vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
> + vaddr = kmap_atomic_px(i915_pt_entry(pd,
> gen8_pd_index(idx, 1)));
> }
> } while (1);
> kunmap_atomic(vaddr);
>
> - return ret;
> + return idx;
> }
>
> static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
> @@ -1212,9 +1185,9 @@ static void gen8_ppgtt_insert_3lvl(struct
> i915_address_space *vm,
> {
> struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> struct sgt_dma iter = sgt_dma(vma);
> - struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
>
> - gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, , ,
> + gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, ,
> + vma->node.start >> GEN8_PTE_SHIFT,
> cache_level, flags);
>
> vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
> @@ -1231,39 +1204,38 @@ static void gen8_ppgtt_insert_huge_entries(struct
> i915_vma *vma,
> dma_addr_t rem = iter->sg->length;
>
> do {
> - struct gen8_insert_pte idx = gen8_insert_pte(start);
> struct i915_page_directory *pdp =
> -
Re: [Intel-gfx] [PATCH 3/4] drm/i915/gtt: Recursive ppgtt alloc for gen8
On 12/07/2019 14.27, Chris Wilson wrote:
> Refactor the separate allocation routines into a single recursive
> function.
>
Reviewed-by: Abdiel Janulgue
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 272 ++--
> 1 file changed, 97 insertions(+), 175 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7b2f3188d435..72e0f9799a46 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1007,199 +1007,119 @@ static void gen8_ppgtt_clear(struct
> i915_address_space *vm,
> start, start + length, vm->top);
> }
>
> -static void gen8_ppgtt_clear_pd(struct i915_address_space *vm,
> - struct i915_page_directory *pd,
> - u64 start, u64 length)
> -{
> - GEM_BUG_ON(!IS_ALIGNED(start, BIT_ULL(GEN8_PTE_SHIFT)));
> - GEM_BUG_ON(!IS_ALIGNED(length, BIT_ULL(GEN8_PTE_SHIFT)));
> -
> - start >>= GEN8_PTE_SHIFT;
> - length >>= GEN8_PTE_SHIFT;
> -
> - __gen8_ppgtt_clear(vm, pd, start, start + length, 1);
> -}
> -
> -static void gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
> - struct i915_page_directory * const pdp,
> - u64 start, u64 length)
> -{
> - GEM_BUG_ON(!IS_ALIGNED(start, BIT_ULL(GEN8_PTE_SHIFT)));
> - GEM_BUG_ON(!IS_ALIGNED(length, BIT_ULL(GEN8_PTE_SHIFT)));
> -
> - start >>= GEN8_PTE_SHIFT;
> - length >>= GEN8_PTE_SHIFT;
> -
> - __gen8_ppgtt_clear(vm, pdp, start, start + length, 2);
> -}
> -
> -static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
> -struct i915_page_directory *pd,
> -u64 start, u64 length)
> +static int __gen8_ppgtt_alloc(struct i915_address_space * const vm,
> + struct i915_page_directory * const pd,
> + u64 * const start, u64 end, int lvl)
> {
> - struct i915_page_table *pt, *alloc = NULL;
> - u64 from = start;
> - unsigned int pde;
> + const struct i915_page_scratch * const scratch = >scratch[lvl];
> + struct i915_page_table *alloc = NULL;
> + unsigned int idx, len;
> int ret = 0;
>
> + len = gen8_pd_range(*start, end, lvl--, );
> + DBG("%s(%p):{lvl:%d, start:%llx, end:%llx, idx:%d, len:%d, used:%d}\n",
> + __func__, vm, lvl + 1, *start, end,
> + idx, len, atomic_read(px_used(pd)));
> + GEM_BUG_ON(!len || (idx + len - 1) >> gen8_pd_shift(1));
> +
> spin_lock(>lock);
> - gen8_for_each_pde(pt, pd, start, length, pde) {
> - const int count = gen8_pte_count(start, length);
> + GEM_BUG_ON(!atomic_read(px_used(pd))); /* Must be pinned! */
> + do {
> + struct i915_page_table *pt = pd->entry[idx];
>
> if (!pt) {
> spin_unlock(>lock);
>
> - pt = fetch_and_zero();
> - if (!pt)
> - pt = alloc_pt(vm);
> - if (IS_ERR(pt)) {
> - ret = PTR_ERR(pt);
> - goto unwind;
> - }
> + DBG("%s(%p):{ lvl:%d, idx:%d } allocating new tree\n",
> + __func__, vm, lvl + 1, idx);
>
> - if (count < GEN8_PTES || intel_vgpu_active(vm->i915))
> - fill_px(pt, vm->scratch[0].encode);
> + pt = fetch_and_zero();
> + if (lvl) {
> + if (!pt) {
> + pt = _pd(vm)->pt;
> + if (IS_ERR(pt)) {
> + ret = PTR_ERR(pt);
> + goto out;
> + }
> + }
>
> - spin_lock(>lock);
> - if (!pd->entry[pde]) {
> - set_pd_entry(pd, pde, pt);
> + fill_px(pt, vm->scratch[lvl].encode);
> } else {
> - alloc = pt;
> - pt = pd->entry[pde];
> - }
> - }
> -
> - atomic_add(count, >used);
> - }
> - spin_unlock(>lock);
> - goto out;
> -
> -unwind:
> - gen8_ppgtt_clear_pd(vm, pd, from, start - from);
> -out:
> - if (alloc)
> - free_px(vm, alloc);
> - return ret;
> -}
> -
> -static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
> - struct i915_page_directory *pdp,
> - u64 start, u64 length)
> -{
> - struct i915_page_directory *pd, *alloc = NULL;
> - u64 from = start;
> - unsigned int pdpe;
> - int ret =
Re: [Intel-gfx] [PATCH 3/4] drm/i915/gtt: Recursive ppgtt alloc for gen8
On 12/07/2019 14.27, Chris Wilson wrote:
> Refactor the separate allocation routines into a single recursive
> function.
>
Reviewed-by: Abdiel Janulgue
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 272 ++--
> 1 file changed, 97 insertions(+), 175 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7b2f3188d435..72e0f9799a46 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1007,199 +1007,119 @@ static void gen8_ppgtt_clear(struct
> i915_address_space *vm,
> start, start + length, vm->top);
> }
>
> -static void gen8_ppgtt_clear_pd(struct i915_address_space *vm,
> - struct i915_page_directory *pd,
> - u64 start, u64 length)
> -{
> - GEM_BUG_ON(!IS_ALIGNED(start, BIT_ULL(GEN8_PTE_SHIFT)));
> - GEM_BUG_ON(!IS_ALIGNED(length, BIT_ULL(GEN8_PTE_SHIFT)));
> -
> - start >>= GEN8_PTE_SHIFT;
> - length >>= GEN8_PTE_SHIFT;
> -
> - __gen8_ppgtt_clear(vm, pd, start, start + length, 1);
> -}
> -
> -static void gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
> - struct i915_page_directory * const pdp,
> - u64 start, u64 length)
> -{
> - GEM_BUG_ON(!IS_ALIGNED(start, BIT_ULL(GEN8_PTE_SHIFT)));
> - GEM_BUG_ON(!IS_ALIGNED(length, BIT_ULL(GEN8_PTE_SHIFT)));
> -
> - start >>= GEN8_PTE_SHIFT;
> - length >>= GEN8_PTE_SHIFT;
> -
> - __gen8_ppgtt_clear(vm, pdp, start, start + length, 2);
> -}
> -
> -static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
> -struct i915_page_directory *pd,
> -u64 start, u64 length)
> +static int __gen8_ppgtt_alloc(struct i915_address_space * const vm,
> + struct i915_page_directory * const pd,
> + u64 * const start, u64 end, int lvl)
> {
> - struct i915_page_table *pt, *alloc = NULL;
> - u64 from = start;
> - unsigned int pde;
> + const struct i915_page_scratch * const scratch = >scratch[lvl];
> + struct i915_page_table *alloc = NULL;
> + unsigned int idx, len;
> int ret = 0;
>
> + len = gen8_pd_range(*start, end, lvl--, );
> + DBG("%s(%p):{lvl:%d, start:%llx, end:%llx, idx:%d, len:%d, used:%d}\n",
> + __func__, vm, lvl + 1, *start, end,
> + idx, len, atomic_read(px_used(pd)));
> + GEM_BUG_ON(!len || (idx + len - 1) >> gen8_pd_shift(1));
> +
> spin_lock(>lock);
> - gen8_for_each_pde(pt, pd, start, length, pde) {
> - const int count = gen8_pte_count(start, length);
> + GEM_BUG_ON(!atomic_read(px_used(pd))); /* Must be pinned! */
> + do {
> + struct i915_page_table *pt = pd->entry[idx];
>
> if (!pt) {
> spin_unlock(>lock);
>
> - pt = fetch_and_zero();
> - if (!pt)
> - pt = alloc_pt(vm);
> - if (IS_ERR(pt)) {
> - ret = PTR_ERR(pt);
> - goto unwind;
> - }
> + DBG("%s(%p):{ lvl:%d, idx:%d } allocating new tree\n",
> + __func__, vm, lvl + 1, idx);
>
> - if (count < GEN8_PTES || intel_vgpu_active(vm->i915))
> - fill_px(pt, vm->scratch[0].encode);
> + pt = fetch_and_zero();
> + if (lvl) {
> + if (!pt) {
> + pt = _pd(vm)->pt;
> + if (IS_ERR(pt)) {
> + ret = PTR_ERR(pt);
> + goto out;
> + }
> + }
>
> - spin_lock(>lock);
> - if (!pd->entry[pde]) {
> - set_pd_entry(pd, pde, pt);
> + fill_px(pt, vm->scratch[lvl].encode);
> } else {
> - alloc = pt;
> - pt = pd->entry[pde];
> - }
> - }
> -
> - atomic_add(count, >used);
> - }
> - spin_unlock(>lock);
> - goto out;
> -
> -unwind:
> - gen8_ppgtt_clear_pd(vm, pd, from, start - from);
> -out:
> - if (alloc)
> - free_px(vm, alloc);
> - return ret;
> -}
> -
> -static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
> - struct i915_page_directory *pdp,
> - u64 start, u64 length)
> -{
> - struct i915_page_directory *pd, *alloc = NULL;
> - u64 from = start;
> - unsigned int pdpe;
> - int ret = 0;
Re: [Intel-gfx] [PATCH 1/5] drm/i915/userptr: Beware recursive lock_page()
On 16/07/2019 13:49, Chris Wilson wrote:
Following a try_to_unmap() we may want to remove the userptr and so call
put_pages(). However, try_to_unmap() acquires the page lock and so we
must avoid recursively locking the pages ourselves -- which means that
we cannot safely acquire the lock around set_page_dirty(). Since we
can't be sure of the lock, we have to risk skip dirtying the page, or
else risk calling set_page_dirty() without a lock and so risk fs
corruption.
So if trylock randomly fail we get data corruption in whatever data set
application is working on, which is what the original patch was trying
to avoid? Are we able to detect the backing store type so at least we
don't risk skipping set_page_dirty with anonymous/shmemfs?
Regards,
Tvrtko
Reported-by: Lionel Landwerlin
Fixes: cb6d7c7dc7ff ("drm/i915/userptr: Acquire the page lock around
set_page_dirty()")
Signed-off-by: Chris Wilson
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index b9d2bb15e4a6..1ad2047a6dbd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -672,7 +672,7 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
obj->mm.dirty = false;
for_each_sgt_page(page, sgt_iter, pages) {
- if (obj->mm.dirty)
+ if (obj->mm.dirty && trylock_page(page)) {
/*
* As this may not be anonymous memory (e.g. shmem)
* but exist on a real mapping, we have to lock
@@ -680,8 +680,20 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
* the page reference is not sufficient to
* prevent the inode from being truncated.
* Play safe and take the lock.
+*
+* However...!
+*
+* The mmu-notifier can be invalidated for a
+* migrate_page, that is alreadying holding the lock
+* on the page. Such a try_to_unmap() will result
+* in us calling put_pages() and so recursively try
+* to lock the page. We avoid that deadlock with
+* a trylock_page() and in exchange we risk missing
+* some page dirtying.
*/
- set_page_dirty_lock(page);
+ set_page_dirty(page);
+ unlock_page(page);
+ }
mark_page_accessed(page);
put_page(page);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH V6 i-g-t 2/6] kms_writeback: Add initial writeback tests
Hi Rodrigo,
On Thu, Jul 11, 2019 at 11:44:35PM -0300, Rodrigo Siqueira wrote:
> On 07/10, Ser, Simon wrote:
> > Hi,
> >
> > Thanks for the patch! Here are a few comments.
> >
> > For bonus points, it would be nice to add igt_describe descriptions of
> > each sub-test.
>
> Hi Simon,
>
> First of all, thanks for your feedback; I already applied most of your
> suggestions. I just have some inline comments/questions.
>
> > On Wed, 2019-06-12 at 23:16 -0300, Brian Starkey wrote:
> > > Add tests for the WRITEBACK_PIXEL_FORMATS, WRITEBACK_OUT_FENCE_PTR and
> > > WRITEBACK_FB_ID properties on writeback connectors, ensuring their
> > > behaviour is correct.
> > >
> > > Signed-off-by: Brian Starkey
> > > [rebased and updated do_writeback_test() function to address feedback]
> > > Signed-off-by: Liviu Dudau
> > > ---
> > > tests/Makefile.sources | 1 +
> > > tests/kms_writeback.c | 314 +
> > > tests/meson.build | 1 +
> > > 3 files changed, 316 insertions(+)
> > > create mode 100644 tests/kms_writeback.c
> > >
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > index 027ed82f..03cc8efa 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -77,6 +77,7 @@ TESTS_progs = \
> > > kms_universal_plane \
> > > kms_vblank \
> > > kms_vrr \
> > > + kms_writeback \
> > > meta_test \
> > > perf \
> > > perf_pmu \
> > > diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
> > > new file mode 100644
> > > index ..66ef48a6
> > > --- /dev/null
> > > +++ b/tests/kms_writeback.c
> > > @@ -0,0 +1,314 @@
> > > +/*
> > > + * (C) COPYRIGHT 2017 ARM Limited. All rights reserved.
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining
> > > a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the
> > > next
> > > + * paragraph) shall be included in all copies or substantial portions of
> > > the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#include "igt.h"
> > > +#include "igt_core.h"
> > > +#include "igt_fb.h"
> > > +
> > > +static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t
> > > *output)
> > > +{
> > > + drmModePropertyBlobRes *blob = NULL;
> > > + uint64_t blob_id;
> > > + int ret;
> > > +
> > > + ret = kmstest_get_property(output->display->drm_fd,
> > > +output->config.connector->connector_id,
> > > +DRM_MODE_OBJECT_CONNECTOR,
> > > +
> > > igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
> > > +NULL, _id, NULL);
> > > + if (ret)
> > > + blob = drmModeGetPropertyBlob(output->display->drm_fd, blob_id);
> > > +
> > > + igt_assert(blob);
> > > +
> > > + return blob;
> > > +}
> > > +
> > > +static bool check_writeback_config(igt_display_t *display, igt_output_t
> > > *output)
> > > +{
> > > + igt_fb_t input_fb, output_fb;
> > > + igt_plane_t *plane;
> > > + uint32_t writeback_format = DRM_FORMAT_XRGB;
> > > + uint64_t tiling = igt_fb_mod_to_tiling(0);
> > > + int width, height, ret;
> > > + drmModeModeInfo override_mode = {
> > > + .clock = 25175,
> > > + .hdisplay = 640,
> > > + .hsync_start = 656,
> > > + .hsync_end = 752,
> > > + .htotal = 800,
> > > + .hskew = 0,
> > > + .vdisplay = 480,
> > > + .vsync_start = 490,
> > > + .vsync_end = 492,
> > > + .vtotal = 525,
> > > + .vscan = 0,
> > > + .vrefresh = 60,
> > > + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > > + .name = {"640x480-60"},
> > > + };
> > > + igt_output_override_mode(output, _mode);
> > > +
> > > + width = override_mode.hdisplay;
> > > + height = override_mode.vdisplay;
> > > +
> > > + ret =
Re: [Intel-gfx] [RFC][PATCH 0/2] drm: PATH prop for all connectors?
On 2019-07-11 3:29 a.m., Pekka Paalanen wrote: > Wait, one can write udev rules for connectors and stuff? > How? What can they do? I was using it to generate user-friendly device names for the mst aux implementation: https://patchwork.freedesktop.org/patch/315900/?series=63237=2 Leo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v13
Am 26.06.19 um 14:29 schrieb Daniel Vetter:
On Wed, Jun 26, 2019 at 02:23:05PM +0200, Christian König wrote:
On the exporter side we add optional explicit pinning callbacks. If those
callbacks are implemented the framework no longer caches sg tables and the
map/unmap callbacks are always called with the lock of the reservation object
held.
On the importer side we add an optional invalidate callback. This callback is
used by the exporter to inform the importers that their mappings should be
destroyed as soon as possible.
This allows the exporter to provide the mappings without the need to pin
the backing store.
v2: don't try to invalidate mappings when the callback is NULL,
lock the reservation obj while using the attachments,
add helper to set the callback
v3: move flag for invalidation support into the DMA-buf,
use new attach_info structure to set the callback
v4: use importer_priv field instead of mangling exporter priv.
v5: drop invalidation_supported flag
v6: squash together with pin/unpin changes
v7: pin/unpin takes an attachment now
v8: nuke dma_buf_attachment_(map|unmap)_locked,
everything is now handled backward compatible
v9: always cache when export/importer don't agree on dynamic handling
v10: minimal style cleanup
v11: drop automatically re-entry avoidance
v12: rename callback to move_notify
v13: add might_lock in appropriate places
v14 with dmabuf->lock gone?
So just back from vacation and double checked that. It almost works but
not quite yet because we still need the lock to protected against
concurrent vmap operations.
And I'm not sure yet if we can use the reservation object there as well.
Going to do try in a separate patch after this one landed.
Also I looked at CI results and stuff, I guess you indeed didn't break the
world because Chris Wilson has faught back struct_mutex far enough by now.
Other issue is that since 2 weeks or so our CI started filtering all
lockdep splats, so you need to dig into results yourself.
btw on that, I think in the end the reservation_obj locking will at best
be consistent between amdgpu and i915: I just remembered that all other
ttm drivers have the mmap_sem vs resv_obj locking the wrong way round, and
the trylock in ttm_bo_fault. So that part alone is hopeless, but I guess
since radeon.ko is abandonware no one will ever fix that.
So I think in the end it boils down to whether that's good enough to land
it, or not.
Well can you give me an rb then? I mean at some point I have to push it
to drm-misc-next.
Christian.
-Daniel
Signed-off-by: Christian König
---
drivers/dma-buf/dma-buf.c | 183 --
include/linux/dma-buf.h | 108 --
2 files changed, 277 insertions(+), 14 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 6c15deb5d4ad..bd8611fa2cfa 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -531,6 +531,9 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
return ERR_PTR(-EINVAL);
}
+ if (WARN_ON(exp_info->ops->cache_sgt_mapping && exp_info->ops->pin))
+ return ERR_PTR(-EINVAL);
+
if (!try_module_get(exp_info->owner))
return ERR_PTR(-ENOENT);
@@ -651,10 +654,12 @@ void dma_buf_put(struct dma_buf *dmabuf)
EXPORT_SYMBOL_GPL(dma_buf_put);
/**
- * dma_buf_attach - Add the device to dma_buf's attachments list; optionally,
+ * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list;
optionally,
* calls attach() of dma_buf_ops to allow device-specific attach functionality
- * @dmabuf:[in]buffer to attach device to.
- * @dev: [in]device to be attached.
+ * @dmabuf:[in]buffer to attach device to.
+ * @dev: [in]device to be attached.
+ * @importer_ops [in]importer operations for the attachment
+ * @importer_priv [in]importer private pointer for the attachment
*
* Returns struct dma_buf_attachment pointer for this attachment. Attachments
* must be cleaned up by calling dma_buf_detach().
@@ -668,8 +673,10 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
* accessible to @dev, and cannot be moved to a more suitable place. This is
* indicated with the error code -EBUSY.
*/
-struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
- struct device *dev)
+struct dma_buf_attachment *
+dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
+ const struct dma_buf_attach_ops *importer_ops,
+ void *importer_priv)
{
struct dma_buf_attachment *attach;
int ret;
@@ -683,6 +690,8 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf
*dmabuf,
attach->dev = dev;
attach->dmabuf = dmabuf;
+ attach->importer_ops = importer_ops;
+ attach->importer_priv = importer_priv;
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/userptr: Beware recursive lock_page()
== Series Details ==
Series: series starting with [1/5] drm/i915/userptr: Beware recursive
lock_page()
URL : https://patchwork.freedesktop.org/series/63752/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6492 -> Patchwork_13664
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/
Known issues
Here are the changes found in Patchwork_13664 that come from known issues:
### IGT changes ###
Issues hit
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [PASS][1] -> [FAIL][2] ([fdo#109485])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/fi-kbl-7500u/igt@kms_chamel...@hdmi-hpd-fast.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/fi-kbl-7500u/igt@kms_chamel...@hdmi-hpd-fast.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [PASS][3] -> [DMESG-WARN][4] ([fdo#102614])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/fi-hsw-peppy/igt@kms_frontbuffer_track...@basic.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/fi-hsw-peppy/igt@kms_frontbuffer_track...@basic.html
Possible fixes
* igt@gem_ctx_create@basic-files:
- fi-icl-u2: [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) ->
[PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/fi-icl-u2/igt@gem_ctx_cre...@basic-files.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/fi-icl-u2/igt@gem_ctx_cre...@basic-files.html
* igt@i915_module_load@reload-no-display:
- {fi-icl-u4}:[DMESG-WARN][7] ([fdo#105602]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6492/fi-icl-u4/igt@i915_module_l...@reload-no-display.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/fi-icl-u4/igt@i915_module_l...@reload-no-display.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046
Participating hosts (55 -> 47)
--
Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-
* Linux: CI_DRM_6492 -> Patchwork_13664
CI_DRM_6492: cb320040f8fea17bf02644f3536ebb34cf9cb5e1 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_5100: 0ea68a1efbfcc4961f2f816ab59e4ad8136c6250 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13664: a6bfd92acc43666e1f9cdbc5c5ac27eeb310a717 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a6bfd92acc43 drm/i915: Hide unshrinkable context objects from the shrinker
53455fa1905e drm/i915/execlists: Cancel breadcrumb on preempting the virtual
engine
2f6edb4997f0 drm/i915/execlists: Process interrupted context on reset
f2e625797017 drm/i915/gt: Push engine stopping into reset-prepare
578455bedd5b drm/i915/userptr: Beware recursive lock_page()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13664/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915/execlists: Cancel breadcrumb on preempting the virtual engine
As we unwind the requests for a preemption event, we return a virtual
request back to its original virtual engine (so that it is available for
execution on any of its siblings). In the process, this means that its
breadcrumb should no longer be associated with the original physical
engine, and so we are forced to decouple it. Previously, as the request
could not complete without our awareness, we would move it to the next
real engine without any danger. However, preempt-to-busy allowed for
requests to continue on the HW and complete in the background as we
unwound, which meant that we could end up retiring the request before
fixing up the breadcrumb link.
[51679.517943] INFO: trying to register non-static key.
[51679.517956] the code is fine but needs lockdep annotation.
[51679.517960] turning off the locking correctness validator.
[51679.517966] CPU: 0 PID: 3270 Comm: kworker/u8:0 Tainted: G U
5.2.0+ #717
[51679.517971] Hardware name: Intel Corporation NUC7i5BNK/NUC7i5BNB, BIOS
BNKBL357.86A.0052.2017.0918.1346 09/18/2017
[51679.518012] Workqueue: i915 retire_work_handler [i915]
[51679.518017] Call Trace:
[51679.518026] dump_stack+0x67/0x90
[51679.518031] register_lock_class+0x52c/0x540
[51679.518038] ? find_held_lock+0x2d/0x90
[51679.518042] __lock_acquire+0x68/0x1800
[51679.518047] ? find_held_lock+0x2d/0x90
[51679.518073] ? __i915_sw_fence_complete+0xff/0x1c0 [i915]
[51679.518079] lock_acquire+0x90/0x170
[51679.518105] ? i915_request_cancel_breadcrumb+0x29/0x160 [i915]
[51679.518112] _raw_spin_lock+0x27/0x40
[51679.518138] ? i915_request_cancel_breadcrumb+0x29/0x160 [i915]
[51679.518165] i915_request_cancel_breadcrumb+0x29/0x160 [i915]
[51679.518199] i915_request_retire+0x43f/0x530 [i915]
[51679.518232] retire_requests+0x4d/0x60 [i915]
[51679.518263] i915_retire_requests+0xdf/0x1f0 [i915]
[51679.518294] retire_work_handler+0x4c/0x60 [i915]
[51679.518301] process_one_work+0x22c/0x5c0
[51679.518307] worker_thread+0x37/0x390
[51679.518311] ? process_one_work+0x5c0/0x5c0
[51679.518316] kthread+0x116/0x130
[51679.518320] ? kthread_create_on_node+0x40/0x40
[51679.518325] ret_from_fork+0x24/0x30
[51679.520177] [ cut here ]
[51679.520189] list_del corruption, 3675e2f0->next is LIST_POISON1
(dead0100)
Fixes: 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 7570a9256001..2ae6695f342b 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -492,6 +492,19 @@ __unwind_incomplete_requests(struct intel_engine_cs
*engine)
list_move(>sched.link, pl);
active = rq;
} else {
+ /*
+* Decouple the virtual breadcrumb before moving it
+* back to the virtual engine -- we don't want the
+* request to complete in the background and try
+* and cancel the breadcrumb on the virtual engine
+* (instead of the old engine where it is linked)!
+*/
+ if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+>fence.flags)) {
+ spin_lock(>lock);
+ i915_request_cancel_breadcrumb(rq);
+ spin_unlock(>lock);
+ }
rq->engine = owner;
owner->submit_request(rq);
active = NULL;
--
2.22.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: Hide unshrinkable context objects from the shrinker
The shrinker cannot touch objects used by the contexts (logical state
and ring). Currently we mark those as "pin_global" to let the shrinker
skip over them, however, if we remove them from the shrinker lists
entirely, we don't event have to include them in our shrink accounting.
By keeping the unshrinkable objects in our shrinker tracking, we report
a large number of objects available to be shrunk, and leave the shrinker
deeply unsatisfied when we fail to reclaim those. The shrinker will
persist in trying to reclaim the unavailable objects, forcing the system
into a livelock (not even hitting the dread oomkiller).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 11 ++--
drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 ++
drivers/gpu/drm/i915/gem/i915_gem_pages.c| 13 +
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 57
drivers/gpu/drm/i915/gt/intel_context.c | 4 +-
drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 17 +++---
drivers/gpu/drm/i915/i915_debugfs.c | 3 +-
drivers/gpu/drm/i915/i915_vma.c | 15 ++
drivers/gpu/drm/i915/i915_vma.h | 4 ++
9 files changed, 97 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index d5197a2a106f..4ea97fca9c35 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -63,6 +63,8 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
spin_lock_init(>vma.lock);
INIT_LIST_HEAD(>vma.list);
+ INIT_LIST_HEAD(>mm.link);
+
INIT_LIST_HEAD(>lut_list);
INIT_LIST_HEAD(>batch_pool_link);
@@ -273,14 +275,7 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
* or else we may oom whilst there are plenty of deferred
* freed objects.
*/
- if (i915_gem_object_has_pages(obj) &&
- i915_gem_object_is_shrinkable(obj)) {
- unsigned long flags;
-
- spin_lock_irqsave(>mm.obj_lock, flags);
- list_del_init(>mm.link);
- spin_unlock_irqrestore(>mm.obj_lock, flags);
- }
+ i915_gem_object_make_unshrinkable(obj);
/*
* Since we require blocking on struct_mutex to unbind the freed
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 67aea07ea019..3714cf234d64 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -394,6 +394,10 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
unsigned int flags);
void i915_gem_object_unpin_from_display_plane(struct i915_vma *vma);
+void i915_gem_object_make_unshrinkable(struct drm_i915_gem_object *obj);
+void i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj);
+void i915_gem_object_make_purgeable(struct drm_i915_gem_object *obj);
+
static inline bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
{
if (obj->cache_dirty)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index b36ad269f4ea..92ad3cc220e3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -153,24 +153,13 @@ static void __i915_gem_object_reset_page_iter(struct
drm_i915_gem_object *obj)
struct sg_table *
__i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
{
- struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct sg_table *pages;
pages = fetch_and_zero(>mm.pages);
if (IS_ERR_OR_NULL(pages))
return pages;
- if (i915_gem_object_is_shrinkable(obj)) {
- unsigned long flags;
-
- spin_lock_irqsave(>mm.obj_lock, flags);
-
- list_del(>mm.link);
- i915->mm.shrink_count--;
- i915->mm.shrink_memory -= obj->base.size;
-
- spin_unlock_irqrestore(>mm.obj_lock, flags);
- }
+ i915_gem_object_make_unshrinkable(obj);
if (obj->mm.mapping) {
void *ptr;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 3f4c6bdcc3c3..14abfd77365f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -530,3 +530,60 @@ void i915_gem_shrinker_taints_mutex(struct
drm_i915_private *i915,
if (unlock)
mutex_release(>drm.struct_mutex.dep_map, 0, _RET_IP_);
}
+
+void i915_gem_object_make_unshrinkable(struct drm_i915_gem_object *obj)
+{
+ if (!list_empty(>mm.link)) {
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ unsigned long flags;
+
+ spin_lock_irqsave(>mm.obj_lock, flags);
+
[Intel-gfx] [PATCH 1/5] drm/i915/userptr: Beware recursive lock_page()
Following a try_to_unmap() we may want to remove the userptr and so call
put_pages(). However, try_to_unmap() acquires the page lock and so we
must avoid recursively locking the pages ourselves -- which means that
we cannot safely acquire the lock around set_page_dirty(). Since we
can't be sure of the lock, we have to risk skip dirtying the page, or
else risk calling set_page_dirty() without a lock and so risk fs
corruption.
Reported-by: Lionel Landwerlin
Fixes: cb6d7c7dc7ff ("drm/i915/userptr: Acquire the page lock around
set_page_dirty()")
Signed-off-by: Chris Wilson
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index b9d2bb15e4a6..1ad2047a6dbd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -672,7 +672,7 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
obj->mm.dirty = false;
for_each_sgt_page(page, sgt_iter, pages) {
- if (obj->mm.dirty)
+ if (obj->mm.dirty && trylock_page(page)) {
/*
* As this may not be anonymous memory (e.g. shmem)
* but exist on a real mapping, we have to lock
@@ -680,8 +680,20 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
* the page reference is not sufficient to
* prevent the inode from being truncated.
* Play safe and take the lock.
+*
+* However...!
+*
+* The mmu-notifier can be invalidated for a
+* migrate_page, that is alreadying holding the lock
+* on the page. Such a try_to_unmap() will result
+* in us calling put_pages() and so recursively try
+* to lock the page. We avoid that deadlock with
+* a trylock_page() and in exchange we risk missing
+* some page dirtying.
*/
- set_page_dirty_lock(page);
+ set_page_dirty(page);
+ unlock_page(page);
+ }
mark_page_accessed(page);
put_page(page);
--
2.22.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915/gt: Push engine stopping into reset-prepare
Push the engine stop into the back reset_prepare (where it already was!)
This allows us to avoid dangerously setting the RING registers to 0 for
logical contexts. If we clear the register on a live context, those
invalid register values are recorded in the logical context state and
replayed (with hilarious results).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gt/intel_lrc.c| 16 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 58 --
drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 40 ++-
3 files changed, 53 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 9e0992498087..9b87a2fc186c 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2173,11 +2173,23 @@ static void execlists_reset_prepare(struct
intel_engine_cs *engine)
__tasklet_disable_sync_once(>tasklet);
GEM_BUG_ON(!reset_in_progress(execlists));
- intel_engine_stop_cs(engine);
-
/* And flush any current direct submission. */
spin_lock_irqsave(>active.lock, flags);
spin_unlock_irqrestore(>active.lock, flags);
+
+ /*
+* We stop engines, otherwise we might get failed reset and a
+* dead gpu (on elk). Also as modern gpu as kbl can suffer
+* from system hang if batchbuffer is progressing when
+* the reset is issued, regardless of READY_TO_RESET ack.
+* Thus assume it is best to stop engines on all gens
+* where we have a gpu reset.
+*
+* WaKBLVECSSemaphoreWaitPoll:kbl (on ALL_ENGINES)
+*
+* FIXME: Wa for more modern gens needs to be validated
+*/
+ intel_engine_stop_cs(engine);
}
static void reset_csb_pointers(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c
b/drivers/gpu/drm/i915/gt/intel_reset.c
index 7ddedfb16aa2..55e2ddcbd215 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -135,47 +135,6 @@ void __i915_request_reset(struct i915_request *rq, bool
guilty)
}
}
-static void gen3_stop_engine(struct intel_engine_cs *engine)
-{
- struct intel_uncore *uncore = engine->uncore;
- const u32 base = engine->mmio_base;
-
- GEM_TRACE("%s\n", engine->name);
-
- if (intel_engine_stop_cs(engine))
- GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
-
- intel_uncore_write_fw(uncore,
- RING_HEAD(base),
- intel_uncore_read_fw(uncore, RING_TAIL(base)));
- intel_uncore_posting_read_fw(uncore, RING_HEAD(base)); /* paranoia */
-
- intel_uncore_write_fw(uncore, RING_HEAD(base), 0);
- intel_uncore_write_fw(uncore, RING_TAIL(base), 0);
- intel_uncore_posting_read_fw(uncore, RING_TAIL(base));
-
- /* The ring must be empty before it is disabled */
- intel_uncore_write_fw(uncore, RING_CTL(base), 0);
-
- /* Check acts as a post */
- if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
- GEM_TRACE("%s: ring head [%x] not parked\n",
- engine->name,
- intel_uncore_read_fw(uncore, RING_HEAD(base)));
-}
-
-static void stop_engines(struct intel_gt *gt, intel_engine_mask_t engine_mask)
-{
- struct intel_engine_cs *engine;
- intel_engine_mask_t tmp;
-
- if (INTEL_GEN(gt->i915) < 3)
- return;
-
- for_each_engine_masked(engine, gt->i915, engine_mask, tmp)
- gen3_stop_engine(engine);
-}
-
static bool i915_in_reset(struct pci_dev *pdev)
{
u8 gdrst;
@@ -607,23 +566,6 @@ int __intel_gt_reset(struct intel_gt *gt,
intel_engine_mask_t engine_mask)
*/
intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
- /*
-* We stop engines, otherwise we might get failed reset and a
-* dead gpu (on elk). Also as modern gpu as kbl can suffer
-* from system hang if batchbuffer is progressing when
-* the reset is issued, regardless of READY_TO_RESET ack.
-* Thus assume it is best to stop engines on all gens
-* where we have a gpu reset.
-*
-* WaKBLVECSSemaphoreWaitPoll:kbl (on ALL_ENGINES)
-*
-* WaMediaResetMainRingCleanup:ctg,elk (presumably)
-*
-* FIXME: Wa for more modern gens needs to be validated
-*/
- if (retry)
- stop_engines(gt, engine_mask);
-
GEM_TRACE("engine_mask=%x\n", engine_mask);
preempt_disable();
ret = reset(gt, engine_mask, retry);
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
[Intel-gfx] [PATCH 3/5] drm/i915/execlists: Process interrupted context on reset
By stopping the rings, we may trigger an arbitration point resulting in a premature context-switch (i.e. a completion event before the request is actually complete). This clears the active context before the reset, but we must remember to rewind the incomplete context for replay upon resume. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_lrc.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 9b87a2fc186c..7570a9256001 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1419,7 +1419,8 @@ static void process_csb(struct intel_engine_cs *engine) * coherent (visible from the CPU) before the * user interrupt and CSB is processed. */ - GEM_BUG_ON(!i915_request_completed(*execlists->active)); + GEM_BUG_ON(!i915_request_completed(*execlists->active) && + !reset_in_progress(execlists)); execlists_schedule_out(*execlists->active++); GEM_BUG_ON(execlists->active - execlists->inflight > @@ -2254,7 +2255,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) */ rq = execlists_active(execlists); if (!rq) - return; + goto unwind; ce = rq->hw_context; GEM_BUG_ON(i915_active_is_idle(>active)); @@ -2331,6 +2332,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) intel_ring_update_space(ce->ring); __execlists_update_reg_state(ce, engine); +unwind: /* Push back any incomplete requests for replay after the reset. */ __unwind_incomplete_requests(engine); } -- 2.22.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/24] drm/i915: Rely on spinlock protection for GPU error capture
Quoting Tvrtko Ursulin (2019-07-16 11:48:28)
>
> On 15/07/2019 09:09, Chris Wilson wrote:
> > +/* single threaded page allocator with a reserved stash for emergencies */
> > +struct pool {
> > + void *stash[31];
>
> Why 31?
Random power-of-two. I considered just using struct pagevec.
>
> > + unsigned int count;
> > +};
> > static bool compress_init(struct compress *c)
> > {
> > - struct z_stream_s *zstream = memset(>zstream, 0,
> > sizeof(c->zstream));
> > + struct z_stream_s *zstream = >zstream;
> >
> > - zstream->workspace =
> > - kmalloc(zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL),
> > - GFP_ATOMIC | __GFP_NOWARN);
> > - if (!zstream->workspace)
> > + if (pool_init(>pool, ALLOW_FAIL))
> > return false;
> >
> > - if (zlib_deflateInit(zstream, Z_DEFAULT_COMPRESSION) != Z_OK) {
> > - kfree(zstream->workspace);
> > + zstream->workspace =
> > + kmalloc(zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL),
> > + ALLOW_FAIL);
> > + if (!zstream->workspace) {
> > + pool_fini(>pool);
> > return false;
> > }
> >
> > c->tmp = NULL;
> > if (i915_has_memcpy_from_wc())
> > - c->tmp = (void *)__get_free_page(GFP_ATOMIC | __GFP_NOWARN);
> > + c->tmp = pool_alloc(>pool, ALLOW_FAIL);
>
> So 31 stashed pages will be enough to not need atomic allocations, or I
> missed a point?
No. It's just a backup in case we run out of atomic allocations. It's
modeled on mempool, except that mempool doesn't allow you to refill in
between atomic sections and instead relies on those being freed. Since
we continually allocate and keep the pages essentially forever, mempool
doesn't help.
> What determines 31 is enough?
It's just a backup, so we have a bit more leeway than current.
Currently, we have no non-atomic phase in which to wait for allocations,
so we should fare much better and have fewer blank error states.
Most compressed error states are less than 128k, so I plucked that as
being sufficiently large enough to capture a single compressed buffer in
case we ran out of atomic.
> > -static void print_error_buffers(struct drm_i915_error_state_buf *m,
> > - const char *name,
> > - struct drm_i915_error_buffer *err,
> > - int count)
> > -{
> > - err_printf(m, "%s [%d]:\n", name, count);
> > -
> > - while (count--) {
> > - err_printf(m, "%08x_%08x %8u %02x %02x",
> > -upper_32_bits(err->gtt_offset),
> > -lower_32_bits(err->gtt_offset),
> > -err->size,
> > -err->read_domains,
> > -err->write_domain);
> > - err_puts(m, tiling_flag(err->tiling));
> > - err_puts(m, dirty_flag(err->dirty));
> > - err_puts(m, purgeable_flag(err->purgeable));
> > - err_puts(m, err->userptr ? " userptr" : "");
> > - err_puts(m, i915_cache_level_str(m->i915, err->cache_level));
> > -
> > - if (err->name)
> > - err_printf(m, " (name: %d)", err->name);
> > - if (err->fence_reg != I915_FENCE_REG_NONE)
> > - err_printf(m, " (fence: %d)", err->fence_reg);
> > -
> > - err_puts(m, "\n");
> > - err++;
> > - }
> > -}
>
> So no active and pinned bos any more.
>
> Ca you put in the commit message what data is gone and what remains? And
> some notes on how does that affect the usefulness of error state.
My purpose for including them was for cross-checking relocations in the
batch. It's been a long time since I've had to do that, and now mesa
likes to tag everything important with EXEC_CAPTURE so the buffers are
included in their entirety.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/24] drm/i915: Rely on spinlock protection for GPU error capture
On 15/07/2019 09:09, Chris Wilson wrote:
Trust that we now have adequate protection over the low level structures
via the engine->active.lock to allow ourselves to capture the GPU error
state without the heavy hammer of stop_machine(). Sadly this does mean
that we have to forgo some of the lesser used information (not derived
from the active state) that is not controlled by the active locks.
A useful side-effect is that this allows us to restore error capturing
for Braswell and Broxton.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 -
drivers/gpu/drm/i915/i915_gpu_error.c | 503 +++---
drivers/gpu/drm/i915/i915_gpu_error.h | 17 -
3 files changed, 207 insertions(+), 318 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 220aba5a94d2..5c9c7cae4817 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2966,11 +2966,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
ggtt->vm.insert_page= bxt_vtd_ggtt_insert_page__BKL;
if (ggtt->vm.clear_range != nop_clear_range)
ggtt->vm.clear_range = bxt_vtd_ggtt_clear_range__BKL;
-
- /* Prevent recursively calling stop_machine() and deadlocks. */
- dev_info(dev_priv->drm.dev,
-"Disabling error capture for VT-d workaround\n");
- i915_disable_error_state(dev_priv, -ENODEV);
}
ggtt->invalidate = gen6_ggtt_invalidate;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index c5b89bf4d616..026ee1f5536d 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -30,7 +30,6 @@
#include
#include
#include
-#include
#include
#include
@@ -46,6 +45,9 @@
#include "i915_scatterlist.h"
#include "intel_csr.h"
+#define ALLOW_FAIL (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
+#define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
+
static inline const struct intel_engine_cs *
engine_lookup(const struct drm_i915_private *i915, unsigned int id)
{
@@ -67,26 +69,6 @@ engine_name(const struct drm_i915_private *i915, unsigned
int id)
return __engine_name(engine_lookup(i915, id));
}
-static const char *tiling_flag(int tiling)
-{
- switch (tiling) {
- default:
- case I915_TILING_NONE: return "";
- case I915_TILING_X: return " X";
- case I915_TILING_Y: return " Y";
- }
-}
-
-static const char *dirty_flag(int dirty)
-{
- return dirty ? " dirty" : "";
-}
-
-static const char *purgeable_flag(int purgeable)
-{
- return purgeable ? " purgeable" : "";
-}
-
static void __sg_set_buf(struct scatterlist *sg,
void *addr, unsigned int len, loff_t it)
{
@@ -114,7 +96,7 @@ static bool __i915_error_grow(struct
drm_i915_error_state_buf *e, size_t len)
if (e->cur == e->end) {
struct scatterlist *sgl;
- sgl = (typeof(sgl))__get_free_page(GFP_KERNEL);
+ sgl = (typeof(sgl))__get_free_page(ALLOW_FAIL);
if (!sgl) {
e->err = -ENOMEM;
return false;
@@ -134,7 +116,7 @@ static bool __i915_error_grow(struct
drm_i915_error_state_buf *e, size_t len)
}
e->size = ALIGN(len + 1, SZ_64K);
- e->buf = kmalloc(e->size, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
+ e->buf = kmalloc(e->size, ALLOW_FAIL);
if (!e->buf) {
e->size = PAGE_ALIGN(len + 1);
e->buf = kmalloc(e->size, GFP_KERNEL);
@@ -211,47 +193,127 @@ i915_error_printer(struct drm_i915_error_state_buf *e)
return p;
}
+/* single threaded page allocator with a reserved stash for emergencies */
+struct pool {
+ void *stash[31];
Why 31?
+ unsigned int count;
+};
+
+static void *__alloc_page(gfp_t gfp)
+{
+ return (void *)__get_free_page(gfp);
+}
+
+static void pool_fini(struct pool *p)
+{
+ while (p->count--)
+ free_page((unsigned long)p->stash[p->count]);
+}
+
+static int pool_refill(struct pool *p, gfp_t gfp)
+{
+ while (p->count < ARRAY_SIZE(p->stash)) {
+ p->stash[p->count] = __alloc_page(gfp);
+ if (!p->stash[p->count])
+ return -ENOMEM;
+
+ p->count++;
+ }
+
+ return 0;
+}
+
+static int pool_init(struct pool *p, gfp_t gfp)
+{
+ int err;
+
+ p->count = 0;
+
+ err = pool_refill(p, gfp);
+ if (err)
+ pool_fini(p);
+
+ return err;
+}
+
+static void *pool_alloc(struct pool *p, gfp_t gfp)
+{
+ void *ptr;
+
+ ptr = __alloc_page(gfp);
+ if (ptr)
+ return ptr;
+
+ if (p->count)
+ return p->stash[--p->count];
+
+ return NULL;
+}
+
+static void pool_free(struct pool *p, void
Re: [Intel-gfx] [PATCH 06/24] drm/i915: Lock the engine while dumping the active request
On 15/07/2019 09:09, Chris Wilson wrote:
We cannot let the request be retired and freed while we are trying to
dump it during error capture. It is not sufficient just to grab a
reference to the request, as during retirement we may free the ring
which we are also dumping. So take the engine lock to prevent retiring
and freeing of the request.
Reported-by: Alex Shumsky
Fixes: 83c317832eb1 ("drm/i915: Dump the ringbuffer of the active request for
debugging")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
Cc: Alex Shumsky
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 ---
drivers/gpu/drm/i915/i915_gpu_error.c | 6 --
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 75099500d1ed..5f8909546d36 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1472,6 +1472,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct i915_gpu_error * const error = >i915->gpu_error;
struct i915_request *rq;
intel_wakeref_t wakeref;
+ unsigned long flags;
if (header) {
va_list ap;
@@ -1491,10 +1492,9 @@ void intel_engine_dump(struct intel_engine_cs *engine,
i915_reset_engine_count(error, engine),
i915_reset_count(error));
- rcu_read_lock();
-
drm_printf(m, "\tRequests:\n");
+ spin_lock_irqsave(>active.lock, flags);
rq = intel_engine_find_active_request(engine);
if (rq) {
print_request(m, rq, "\t\tactive ");
@@ -1514,8 +1514,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
print_request_ring(m, rq);
}
-
- rcu_read_unlock();
+ spin_unlock_irqrestore(>active.lock, flags);
wakeref = intel_runtime_pm_get_if_in_use(>i915->runtime_pm);
if (wakeref) {
@@ -1654,7 +1653,6 @@ struct i915_request *
intel_engine_find_active_request(struct intel_engine_cs *engine)
{
struct i915_request *request, *active = NULL;
- unsigned long flags;
/*
* We are called by the error capture, reset and to dump engine
@@ -1667,7 +1665,7 @@ intel_engine_find_active_request(struct intel_engine_cs
*engine)
* At all other times, we must assume the GPU is still running, but
* we only care about the snapshot of this moment.
*/
- spin_lock_irqsave(>active.lock, flags);
+ lockdep_assert_held(>active.lock);
list_for_each_entry(request, >active.requests, sched.link) {
if (i915_request_completed(request))
continue;
@@ -1682,7 +1680,6 @@ intel_engine_find_active_request(struct intel_engine_cs
*engine)
active = request;
break;
}
- spin_unlock_irqrestore(>active.lock, flags);
return active;
}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 78e388fa059c..c5b89bf4d616 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1411,6 +1411,7 @@ static void gem_record_rings(struct i915_gpu_state *error)
struct intel_engine_cs *engine = i915->engine[i];
struct drm_i915_error_engine *ee = >engine[i];
struct i915_request *request;
+ unsigned long flags;
ee->engine_id = -1;
@@ -1422,10 +1423,11 @@ static void gem_record_rings(struct i915_gpu_state *error)
error_record_engine_registers(error, engine, ee);
error_record_engine_execlists(engine, ee);
+ spin_lock_irqsave(>active.lock, flags);
request = intel_engine_find_active_request(engine);
if (request) {
struct i915_gem_context *ctx = request->gem_context;
- struct intel_ring *ring;
+ struct intel_ring *ring = request->ring;
ee->vm = ctx->vm ?: >gt->ggtt->vm;
@@ -1455,7 +1457,6 @@ static void gem_record_rings(struct i915_gpu_state *error)
ee->rq_post = request->postfix;
ee->rq_tail = request->tail;
- ring = request->ring;
ee->cpu_ring_head = ring->head;
ee->cpu_ring_tail = ring->tail;
ee->ringbuffer =
@@ -1463,6 +1464,7 @@ static void gem_record_rings(struct i915_gpu_state *error)
engine_record_requests(engine, request, ee);
}
+ spin_unlock_irqrestore(>active.lock, flags);
ee->hws_page =
i915_error_object_create(i915,
I missed in the initial read the fact underlying allocations are already
atomic.
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH 4/4] drm/i915/tgl/dsi: Gate the ddi clocks after pll mapping
>-Original Message-
>From: Kulkarni, Vandita
>Sent: Tuesday, July 2, 2019 9:49 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: ville.syrj...@linux.intel.com; Nikula, Jani ;
>Shankar, Uma
>; Kulkarni, Vandita
>Subject: [PATCH 4/4] drm/i915/tgl/dsi: Gate the ddi clocks after pll mapping
>
>No need to keep it on till IO enabling.
Minor nit: You can replace "it" by "ddi clock". Also add that when (at what
stage) they
get enabled to give a relative picture.
With this fixed.
Reviewed-by: Uma Shankar
>Signed-off-by: Vandita Kulkarni
>---
> drivers/gpu/drm/i915/display/icl_dsi.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>b/drivers/gpu/drm/i915/display/icl_dsi.c
>index d1c50a4186f0..99ce8c708353 100644
>--- a/drivers/gpu/drm/i915/display/icl_dsi.c
>+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>@@ -609,8 +609,12 @@ static void gen11_dsi_map_pll(struct intel_encoder
>*encoder,
> I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>
> for_each_dsi_port(port, intel_dsi->ports) {
>- val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port);
>+ if (INTEL_GEN(dev_priv) >= 12)
>+ val |= DPCLKA_CFGCR0_DDI_CLK_OFF(port);
>+ else
>+ val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> }
>+
> I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>
> POSTING_READ(DPCLKA_CFGCR0_ICL);
>@@ -955,6 +959,8 @@ static void
> gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config) {
>+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>+
> /* step 4a: power up all lanes of the DDI used by DSI */
> gen11_dsi_power_up_lanes(encoder);
>
>@@ -977,7 +983,8 @@ gen11_dsi_enable_port_and_phy(struct intel_encoder
>*encoder,
> gen11_dsi_configure_transcoder(encoder, pipe_config);
>
> /* Step 4l: Gate DDI clocks */
>- gen11_dsi_gate_clocks(encoder);
>+ if (IS_GEN(dev_priv, 11))
>+ gen11_dsi_gate_clocks(encoder);
> }
>
> static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
>--
>2.21.0.5.gaeb582a
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915/tgl/dsi: Do not override TA_SURE
>-Original Message-
>From: Kulkarni, Vandita
>Sent: Tuesday, July 2, 2019 9:49 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: ville.syrj...@linux.intel.com; Nikula, Jani ;
>Shankar, Uma
>; Kulkarni, Vandita
>Subject: [PATCH 3/4] drm/i915/tgl/dsi: Do not override TA_SURE
>
>Do not override TA_SURE timing parameter to zero for DSI 8X frequency 800MHz or
>below on TGL.
Looks good to me.
Reviewed-by: Uma Shankar
>Signed-off-by: Vandita Kulkarni
>---
> drivers/gpu/drm/i915/display/icl_dsi.c | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>b/drivers/gpu/drm/i915/display/icl_dsi.c
>index e3980676bcef..d1c50a4186f0 100644
>--- a/drivers/gpu/drm/i915/display/icl_dsi.c
>+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>@@ -530,18 +530,20 @@ static void gen11_dsi_setup_dphy_timings(struct
>intel_encoder *encoder)
>* a value '0' inside TA_PARAM_REGISTERS otherwise
>* leave all fields at HW default values.
>*/
>- if (intel_dsi_bitrate(intel_dsi) <= 80) {
>- for_each_dsi_port(port, intel_dsi->ports) {
>- tmp = I915_READ(DPHY_TA_TIMING_PARAM(port));
>- tmp &= ~TA_SURE_MASK;
>- tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
>- I915_WRITE(DPHY_TA_TIMING_PARAM(port), tmp);
>-
>- /* shadow register inside display core */
>- tmp = I915_READ(DSI_TA_TIMING_PARAM(port));
>- tmp &= ~TA_SURE_MASK;
>- tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
>- I915_WRITE(DSI_TA_TIMING_PARAM(port), tmp);
>+ if (IS_GEN(dev_priv, 11)) {
>+ if (intel_dsi_bitrate(intel_dsi) <= 80) {
>+ for_each_dsi_port(port, intel_dsi->ports) {
>+ tmp = I915_READ(DPHY_TA_TIMING_PARAM(port));
>+ tmp &= ~TA_SURE_MASK;
>+ tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
>+ I915_WRITE(DPHY_TA_TIMING_PARAM(port), tmp);
>+
>+ /* shadow register inside display core */
>+ tmp = I915_READ(DSI_TA_TIMING_PARAM(port));
>+ tmp &= ~TA_SURE_MASK;
>+ tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
>+ I915_WRITE(DSI_TA_TIMING_PARAM(port), tmp);
>+ }
> }
> }
>
>--
>2.21.0.5.gaeb582a
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915/tgl/dsi: Set latency PCS_DW1 for tgl
>-Original Message-
>From: Kulkarni, Vandita
>Sent: Tuesday, July 2, 2019 9:49 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: ville.syrj...@linux.intel.com; Nikula, Jani ;
>Shankar, Uma
>; Kulkarni, Vandita
>Subject: [PATCH 2/4] drm/i915/tgl/dsi: Set latency PCS_DW1 for tgl
>
>Rest of the latency programming remains same as that of ICL.
You can put this as "latency programming for TGL remains same as that of ICL
and EHL.
Extended the same for TGL"
With this minor nit fixed.
Reviewed-by: Uma Shankar
>Signed-off-by: Vandita Kulkarni
>---
> drivers/gpu/drm/i915/display/icl_dsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>b/drivers/gpu/drm/i915/display/icl_dsi.c
>index 556eba2636fe..e3980676bcef 100644
>--- a/drivers/gpu/drm/i915/display/icl_dsi.c
>+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>@@ -404,8 +404,8 @@ static void gen11_dsi_config_phy_lanes_sequence(struct
>intel_encoder *encoder)
> tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
> I915_WRITE(ICL_PORT_TX_DW2_GRP(port), tmp);
>
>- /* For EHL set latency optimization for PCS_DW1 lanes */
>- if (IS_ELKHARTLAKE(dev_priv)) {
>+ /* EHL and TGL, set latency optimization for PCS_DW1 lanes */
>+ if (IS_ELKHARTLAKE(dev_priv) || (INTEL_GEN(dev_priv) >= 12)) {
> tmp = I915_READ(ICL_PORT_PCS_DW1_AUX(port));
> tmp &= ~LATENCY_OPTIM_MASK;
> tmp |= LATENCY_OPTIM_VAL(0);
>--
>2.21.0.5.gaeb582a
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/tgl/dsi: Program TRANS_VBLANK register
>-Original Message-
>From: Kulkarni, Vandita
>Sent: Tuesday, July 2, 2019 9:49 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: ville.syrj...@linux.intel.com; Nikula, Jani ;
>Shankar, Uma
>; Kulkarni, Vandita
>Subject: [PATCH 1/4] drm/i915/tgl/dsi: Program TRANS_VBLANK register
>
>Program vblank register for mipi dsi in video mode on TGL.
>
>Signed-off-by: Vandita Kulkarni
>---
> drivers/gpu/drm/i915/display/icl_dsi.c | 9 +
> 1 file changed, 9 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>b/drivers/gpu/drm/i915/display/icl_dsi.c
>index b8673debf932..556eba2636fe 100644
>--- a/drivers/gpu/drm/i915/display/icl_dsi.c
>+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>@@ -866,6 +866,15 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
>*encoder,
> dsi_trans = dsi_port_to_transcoder(port);
> I915_WRITE(VSYNCSHIFT(dsi_trans), vsync_shift);
> }
>+
>+ /* program TRANS_VBLANK register, should be same as vtotal progammed */
Typo here in programmed.
>+ if (INTEL_GEN(dev_priv) >= 12) {
>+ for_each_dsi_port(port, intel_dsi->ports) {
>+ dsi_trans = dsi_port_to_transcoder(port);
>+ I915_WRITE(VBLANK(dsi_trans),
>+ (vactive - 1) | ((vtotal - 1) << 16));
We can put this line along with VTOTAL and get rid of this extra for loop.
>+ }
>+ }
> }
>
> static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
>--
>2.21.0.5.gaeb582a
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/execlists: Disable preemption under GVT
On 2019.07.09 10:12:33 +0100, Chris Wilson wrote:
> Preempt-to-busy uses a GPU semaphore to enforce an idle-barrier across
> preemption, but mediated gvt does not fully support semaphores.
>
> v2: Fiddle around with the flags and settle on using has-semaphores for
> the core bits so that we retain the ability to preempt our own
> semaphores.
>
> Signed-off-by: Chris Wilson
> Cc: Zhenyu Wang
> Cc: Xiaolin Zhang
> Cc: Tvrtko Ursulin
> Cc: Mika Kuoppala
> ---
I've tried to run guest with this one for several benchmarks on SKL
and didn't observe hang. I think the hang met by Xiaolin might be caused
by other things that can be double checked later.
So for semaphore disable in vGPU case,
Acked-by: Zhenyu Wang
thanks!
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 ++--
> drivers/gpu/drm/i915/gt/intel_lrc.c | 24 +--
> drivers/gpu/drm/i915/gt/selftest_lrc.c| 6 ++
> 3 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 56310812da21..614ed8c488ef 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -825,6 +825,8 @@ int intel_engine_init_common(struct intel_engine_cs
> *engine)
> struct drm_i915_private *i915 = engine->i915;
> int ret;
>
> + engine->set_default_submission(engine);
> +
> /* We may need to do things with the shrinker which
>* require us to immediately switch back to the default
>* context. This can cause a problem as pinning the
> @@ -852,8 +854,6 @@ int intel_engine_init_common(struct intel_engine_cs
> *engine)
>
> engine->emit_fini_breadcrumb_dw = ret;
>
> - engine->set_default_submission(engine);
> -
> return 0;
>
> err_unpin:
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 558a5850de3c..ef36f4b5e212 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -295,6 +295,9 @@ static inline bool need_preempt(const struct
> intel_engine_cs *engine,
> {
> int last_prio;
>
> + if (!intel_engine_has_semaphores(engine))
> + return false;
> +
> /*
>* Check if the current priority hint merits a preemption attempt.
>*
> @@ -893,6 +896,9 @@ need_timeslice(struct intel_engine_cs *engine, const
> struct i915_request *rq)
> {
> int hint;
>
> + if (!intel_engine_has_semaphores(engine))
> + return false;
> +
> if (list_is_last(>sched.link, >active.requests))
> return false;
>
> @@ -2634,7 +2640,8 @@ static u32 *gen8_emit_fini_breadcrumb(struct
> i915_request *request, u32 *cs)
> *cs++ = MI_USER_INTERRUPT;
>
> *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
> - cs = emit_preempt_busywait(request, cs);
> + if (intel_engine_has_semaphores(request->engine))
> + cs = emit_preempt_busywait(request, cs);
>
> request->tail = intel_ring_offset(request, cs);
> assert_ring_tail_valid(request->ring, request->tail);
> @@ -2658,7 +2665,8 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct
> i915_request *request, u32 *cs)
> *cs++ = MI_USER_INTERRUPT;
>
> *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
> - cs = emit_preempt_busywait(request, cs);
> + if (intel_engine_has_semaphores(request->engine))
> + cs = emit_preempt_busywait(request, cs);
>
> request->tail = intel_ring_offset(request, cs);
> assert_ring_tail_valid(request->ring, request->tail);
> @@ -2706,10 +2714,11 @@ void intel_execlists_set_default_submission(struct
> intel_engine_cs *engine)
> engine->unpark = NULL;
>
> engine->flags |= I915_ENGINE_SUPPORTS_STATS;
> - if (!intel_vgpu_active(engine->i915))
> + if (!intel_vgpu_active(engine->i915)) {
> engine->flags |= I915_ENGINE_HAS_SEMAPHORES;
> - if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> - engine->flags |= I915_ENGINE_HAS_PREEMPTION;
> + if (HAS_LOGICAL_RING_PREEMPTION(engine->i915))
> + engine->flags |= I915_ENGINE_HAS_PREEMPTION;
> + }
> }
>
> static void execlists_destroy(struct intel_engine_cs *engine)
> @@ -3399,7 +3408,6 @@ intel_execlists_create_virtual(struct i915_gem_context
> *ctx,
> ve->base.class = OTHER_CLASS;
> ve->base.uabi_class = I915_ENGINE_CLASS_INVALID;
> ve->base.instance = I915_ENGINE_CLASS_INVALID_VIRTUAL;
> - ve->base.flags = I915_ENGINE_IS_VIRTUAL;
>
> /*
>* The decision on whether to submit a request using semaphores
> @@ -3496,8 +3504,12 @@ intel_execlists_create_virtual(struct i915_gem_context
> *ctx,
> ve->base.emit_fini_breadcrumb = sibling->emit_fini_breadcrumb;
> ve->base.emit_fini_breadcrumb_dw =
> sibling->emit_fini_breadcrumb_dw;
> +
> +
Re: [Intel-gfx] [PATCH 2/2] dma-buf: Relax the write-seqlock for reallocating the shared fence list
On Fri, Jul 12, 2019 at 09:03:14AM +0100, Chris Wilson wrote: > As the set of shared fences is not being changed during reallocation of > the reservation list, we can skip updating the write_seqlock. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Christian König sounds legit. Reviewed-by: Daniel Vetter More seriously, I think I convinced myself that we cant see a mess of old and new fence arrays anywhere, even without the seqlock retry, so I think we should be all good. -Daniel > --- > drivers/dma-buf/reservation.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index 80ecc1283d15..c71b85c8c159 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -157,15 +157,15 @@ int reservation_object_reserve_shared(struct > reservation_object *obj, > (ksize(new) - offsetof(typeof(*new), shared)) / > sizeof(*new->shared); > > - preempt_disable(); > - write_seqcount_begin(>seq); > /* > - * RCU_INIT_POINTER can be used here, > - * seqcount provides the necessary barriers > + * We are not changing the effective set of fences here so can > + * merely update the pointer to the new array; both existing > + * readers and new readers will see exactly the same set of > + * active (unsignaled) shared fences. Individual fences and the > + * old array are protected by RCU and so will not vanish under > + * the gaze of the rcu_read_lock() readers. >*/ > - RCU_INIT_POINTER(obj->fence, new); > - write_seqcount_end(>seq); > - preempt_enable(); > + rcu_assign_pointer(obj->fence, new); > > if (!old) > return 0; > -- > 2.22.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 07/11] drm/i915: add syncobj timeline support
On 15/07/2019 14:30, Koenig, Christian wrote:
Hi Lionel,
sorry for the delayed response, I'm just back from vacation.
Am 03.07.19 um 11:17 schrieb Lionel Landwerlin:
On 03/07/2019 11:56, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-07-01 12:34:33)
+ syncobj = drm_syncobj_find(eb->file,
user_fence.handle);
+ if (!syncobj) {
+ DRM_DEBUG("Invalid syncobj handle provided\n");
+ err = -EINVAL;
+ goto err;
+ }
+
+ if (user_fence.flags & I915_EXEC_FENCE_WAIT) {
+ fence = drm_syncobj_fence_get(syncobj);
+ if (!fence) {
+ DRM_DEBUG("Syncobj handle has no
fence\n");
+ drm_syncobj_put(syncobj);
+ err = -EINVAL;
+ goto err;
+ }
+
+ err = dma_fence_chain_find_seqno(,
point);
I'm very dubious about chain_find_seqno().
It returns -EINVAL if the point is older than the first in the chain --
it is in an unknown state, but may be signaled since we remove signaled
links from the chain. If we are waiting for an already signaled syncpt,
we should not be erring out!
You're right, I got this wrong.
We can get fence = NULL if the point is already signaled.
The easiest would be to skip it from the list, or add the stub fence.
I guess the CTS got lucky that it always got the point needed before
it was garbage collected...
The topmost point is never garbage collected. So IIRC the check is
actually correct and you should never get NULL here.
Do we allow later requests to insert earlier syncpt into the chain? If
so, then the request we wait on here may be woefully inaccurate and
quite easily lead to cycles in the fence tree. We have no way of
resolving such deadlocks -- we would have to treat this fence as a
foreign fence and install a backup timer. Alternatively, we only allow
this to return the exact fence for a syncpt, and proxies for the rest.
Adding points < latest added point is forbidden.
I wish we enforced it a bit more than what's currently done in
drm_syncobj_add_point().
In my view we should :
- lock the syncobj in get_timeline_fence_array() do the sanity
check there.
- keep the lock until we add the point to the timeline
- unlock once added
That way we would ensure that the application cannot generate invalid
timelines and error out if it does.
We could do the same for host signaling in
drm_syncobj_timeline_signal_ioctl/drm_syncobj_transfer_to_timeline
(there the locking a lot shorter).
That requires holding the lock for longer than maybe other driver
would prefer.
Ccing Christian who can tell whether that's out of question for AMD.
Yeah, adding the lock was the only other option I could see as well, but
we intentionally decided against that.
Since we have multiple out sync objects we would need to use a ww_mutex
as lock here.
That in turn would result in a another rather complicated dance for
deadlock avoidance. Something which each driver would have to implement
correctly.
That doesn't sounds like a good idea to me just to improve error checking.
As long as it is only in the same process userspace could check that as
well before doing the submission.
Thanks Christian,
Would you be opposed to exposing an _locked() version of
drm_syncobj_add_point() and have a static inline do the locking?
I don't think it would be a difference for your driver and we could add
checking with a proxy fence Chris suggested on our side.
We could also allow do checks in drm_syncobj_timeline_signal_ioctl().
-Lionel
Regards,
Christian.
Cheers,
-Lionel
+ if (err || !fence) {
+ DRM_DEBUG("Syncobj handle missing
requested point\n");
+ drm_syncobj_put(syncobj);
+ err = err != 0 ? err : -EINVAL;
+ goto err;
+ }
+ }
+
+ /*
+ * For timeline syncobjs we need to preallocate
chains for
+ * later signaling.
+ */
+ if (point != 0 && user_fence.flags &
I915_EXEC_FENCE_SIGNAL) {
+ fences[n].chain_fence =
+ kmalloc(sizeof(*fences[n].chain_fence),
+ GFP_KERNEL);
+ if (!fences[n].chain_fence) {
+ dma_fence_put(fence);
+ drm_syncobj_put(syncobj);
+ err = -ENOMEM;
+ DRM_DEBUG("Unable to alloc
chain_fence\n");
+ goto err;
+ }
What happens if we later try to insert two fences for the same syncpt?
Should we not reserve the slot in the chain to reject duplicates?
