Re: [Intel-gfx] [PATCH] drm/i915: Truly bump ready tasks ahead of busywaits
Quoting Chris Wilson (2019-05-09 21:51:54)
> In commit b7404c7ecb38 ("drm/i915: Bump ready tasks ahead of
> busywaits"), I tried cutting a corner in order to not install a signal
> for each of our dependencies, and only listened to requests on which we
> were intending to busywait. The compromise that was made was that
> instead of then being able to promite the request with a full
> NOSEMAPHORE like its non-busywaiting brethren, as we had not ensured we
> had cleared the semaphore chain, we settled for only using the NEWCLIENT
> boost. With an over saturated system with multiple NEWCLIENTS in flight
> at any time, this was found to be an inadequate promotion and left us
> with a much poorer scheduling order than prior to using semaphores.
>
> The outcome of this patch, is that all requests have NOSEMAPHORE
> priority when they have no dependencies and are ready to run and not
> busywait, restoring the pre-semaphore ordering on saturated systems.
>
> Fixes: b7404c7ecb38 ("drm/i915: Bump ready tasks ahead of busywaits")
Fixes: f9e9e9de58c7 ("drm/i915: Prioritise non-busywait semaphore workloads")
> Signed-off-by: Chris Wilson
> Cc: Tvrtko Ursulin
> Cc: Dmitry Rogozhkin
> Cc: Dmitry Ermilov
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] kernel/locking/semaphore: use wake_q in up()
On Fri, May 10, 2019 at 7:50 AM Sergey Senozhatsky
wrote:
>
> On (05/09/19 22:06), Daniel Vetter wrote:
> [..]
> > +/* Functions for the contended case */
> > +
> > +struct semaphore_waiter {
> > + struct list_head list;
> > + struct task_struct *task;
> > + bool up;
> > +};
> > +
> > /**
> > * up - release the semaphore
> > * @sem: the semaphore to release
> > @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> > void up(struct semaphore *sem)
> > {
> > unsigned long flags;
> > + struct semaphore_waiter *waiter;
> > + DEFINE_WAKE_Q(wake_q);
> >
> > raw_spin_lock_irqsave(&sem->lock, flags);
> > - if (likely(list_empty(&sem->wait_list)))
> > + if (likely(list_empty(&sem->wait_list))) {
> > sem->count++;
> > - else
> > - __up(sem);
> > + } else {
> > + waiter = list_first_entry(&sem->wait_list,
> > +struct semaphore_waiter, list);
> > + list_del(&waiter->list);
> > + waiter->up = true;
> > + wake_q_add(&wake_q, waiter->task);
> > + }
> > raw_spin_unlock_irqrestore(&sem->lock, flags);
>
> So the new code still can printk/WARN under sem->lock in some buggy
> cases.
>
> E.g.
> wake_q_add()
> get_task_struct()
> refcount_inc_checked()
>WARN_ONCE()
>
> Are we fine with that?
Hm not great. It's not as bad as the one I'm trying to fix (or not the
same at least), because with the wake up chain we have a few locks in
there. Which allows lockdep to connect the loop and complain, even
when we never actually hit that specific recursion. I.e. once hitting
a WARN_ON from try_to_wake_up is enough, plus a totally separate
callchain can then close the semaphore.lock->scheduler locks part.
Your chain only goes boom if it happens from the console_lock's up.
wake_q_add_safe would be an option, but then we somehow need to
arrange for down to call get_task_struct(current) and releasing that,
but only if there's no waker who needs that task ref. Sounds tricky
... Also not sure we want to stuff that trickery into the generic
semaphore code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Truly bump ready tasks ahead of busywaits
== Series Details == Series: drm/i915: Truly bump ready tasks ahead of busywaits URL : https://patchwork.freedesktop.org/series/60486/ State : success == Summary == CI Bug Log - changes from CI_DRM_6072_full -> Patchwork_12999_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_12999_full that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_rpm@i2c: - shard-iclb: [PASS][1] -> [DMESG-WARN][2] ([fdo#109982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb3/igt@i915_pm_...@i2c.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-iclb2/igt@i915_pm_...@i2c.html * igt@i915_pm_rpm@system-suspend: - shard-skl: [PASS][3] -> [INCOMPLETE][4] ([fdo#104108] / [fdo#107807]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl10/igt@i915_pm_...@system-suspend.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-skl1/igt@i915_pm_...@system-suspend.html * igt@i915_suspend@fence-restore-tiled2untiled: - shard-apl: [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +2 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-apl3/igt@i915_susp...@fence-restore-tiled2untiled.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-apl6/igt@i915_susp...@fence-restore-tiled2untiled.html * igt@kms_color@pipe-a-ctm-0-5: - shard-skl: [PASS][7] -> [FAIL][8] ([fdo#108682]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl5/igt@kms_co...@pipe-a-ctm-0-5.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-skl10/igt@kms_co...@pipe-a-ctm-0-5.html * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic: - shard-glk: [PASS][9] -> [FAIL][10] ([fdo#106509] / [fdo#107409]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-glk6/igt@kms_cursor_leg...@2x-nonblocking-modeset-vs-cursor-atomic.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-glk1/igt@kms_cursor_leg...@2x-nonblocking-modeset-vs-cursor-atomic.html * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions: - shard-hsw: [PASS][11] -> [FAIL][12] ([fdo#103355]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-hsw2/igt@kms_cursor_leg...@cursor-vs-flip-atomic-transitions.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-hsw6/igt@kms_cursor_leg...@cursor-vs-flip-atomic-transitions.html * igt@kms_flip@flip-vs-expired-vblank-interruptible: - shard-skl: [PASS][13] -> [FAIL][14] ([fdo#105363]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl1/igt@kms_f...@flip-vs-expired-vblank-interruptible.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-skl7/igt@kms_f...@flip-vs-expired-vblank-interruptible.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_6072/shard-skl2/igt@kms_f...@flip-vs-suspend-interruptible.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-skl2/igt@kms_f...@flip-vs-suspend-interruptible.html * igt@kms_frontbuffer_tracking@fbc-suspend: - shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103167]) +9 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb3/igt@kms_frontbuffer_track...@fbc-suspend.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-iclb2/igt@kms_frontbuffer_track...@fbc-suspend.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl1/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-skl8/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html * igt@kms_plane_lowres@pipe-a-tiling-x: - shard-iclb: [PASS][21] -> [FAIL][22] ([fdo#103166]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb7/igt@kms_plane_low...@pipe-a-tiling-x.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-iclb7/igt@kms_plane_low...@pipe-a-tiling-x.html * igt@kms_psr@no_drrs: - shard-iclb: [PASS][23] -> [FAIL][24] ([fdo#108341]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb6/igt@kms_psr@no_drrs.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12999/shard-iclb1/igt@kms_psr@no_drrs.html * igt@kms_psr@psr2_sprite_plane_move: - shard-iclb: [PASS][25] -> [SKIP][26] ([fdo#109441]) +1 similar issue [25]: https://
Re: [Intel-gfx] IOMMU RMRR for Intel graphic device
On Fri, May 10, 2019 at 01:21:59PM +0800, Lu Baolu wrote: > Forward to i915 mailing list and post the question again so people knows > what are the key concerns. > > The background is that Linux community is going to collect and report > the reserved memory ranges of an assigned device to VFIO driver, and any > mapped address will be checked against the reserved ranges. If there's > any conflict, vifo will refuse the map request. > > Unfortunately, when it comes Intel graphic device, the conflict happens. > That means address range mapped through vfio conflicts with the rmrr for > graphic device. > > https://lkml.org/lkml/2018/6/5/760 > > The questions are 1) will the rmrr for graphic device still needs to be > reserved for BIOS or firmware use, when a device is going to be assigned > to user level? 2) if no, what's the check point, after which the rmrr is > unnecessary anymore? The gfx RMRR isn't for the bios, it's for the driver. It covers stolen memory, and we need it. There's various piles of hacks to disable use of stolen, but they're all somewhat fragile, and afaiui for huc/guc we need it, and huc is part of the uapi exposed by the driver/device combo on gen9+. Until our hw folks come up with a better design I think we're just stuck on this, iow you can't pass-thru modern intel igfx because of this. -Daniel > > Best regards, > Lu Baolu > > On 5/9/19 5:14 PM, Zhang, Tina wrote: > > Hi Baolu, > > > > +Xiong > > > > I think the root cause is that guest i915 needs to access RMRR. Xiong > > cooked a patch to disable the RMRR use in i915 guest, however that patch > > didn't get landed into the i915 upstream repo due to some concerns from > > i915 maintainers. Xiong can give us more backgrounds. > > > > So agreed, need to ask the i915 folk for this. > > > > BR, > > Tina > > > > > > > -Original Message- > > > From: Yuan, Hang > > > Sent: Thursday, May 9, 2019 4:07 PM > > > To: Lu Baolu ; Tian, Kevin > > > ; > > > Zhenyu Wang ; Zhang, Tina > > > ; Lu, Baolu ; Liu, Yi L > > > > > > Subject: RE: IOMMU RMRR for Intel graphic device > > > > > > Hi Baolu, as Kevin suggested, would you like to ask i915 people in their > > > mailing list intel-gfx@lists.freedesktop.org? > > > > > > Regards, > > > Henry > > > > > > > -Original Message- > > > > From: Lu Baolu [mailto:baolu...@linux.intel.com] > > > > Sent: Thursday, May 9, 2019 2:42 PM > > > > To: Tian, Kevin ; Zhenyu Wang > > > > ; Zhang, Tina ; Yuan, > > > > Hang ; Lu, Baolu ; Liu, Yi L > > > > > > > > Cc: baolu...@linux.intel.com > > > > Subject: Re: IOMMU RMRR for Intel graphic device > > > > > > > > Hi, > > > > > > > > +Tina and Henry and cc more people > > > > > > > > The background is that Linux community is going to collect and report > > > > the reserved memory ranges of an assigned device to VFIO driver, and > > > > any mapped address will be checked against the reserved ranges. If > > > > there's any conflict, vifo will refuse the map request. > > > > > > > > Unfortunately, when it comes Intel graphic device, the conflict happens. > > > > That means address range mapped through vfio conflicts with the rmrr > > > > for graphic device. > > > > > > > > https://lkml.org/lkml/2018/6/5/760 > > > > > > > > The questions are 1) will the rmrr for graphic device still needs to > > > > be reserved for BIOS or firmware use, when a device is going to be > > > > assigned to user level? 2) if no, what's the check point, after which > > > > the rmrr is unnecessary anymore? > > > > > > > > Best regards, > > > > Lu Baolu > > > > > > > > On 5/6/19 2:16 PM, Tian, Kevin wrote: > > > > > this should better be asked to i915 guys, since it's not > > > > > virtualization related. :-) > > > > > > > > > > One caveat, iirc, i915 driver tries to reuse stolen memory (covered > > > > > by > > > > > RMRR) even after boot time. take it as if another type of memory > > > > > resource. If true I'm afraid this might be a gap to your proposal. > > > > > > > > > > Since nothing confidential, possibly you can directly discuss in > > > community. > > > > > > > > > > > From: Lu Baolu [mailto:baolu...@linux.intel.com] > > > > > > Sent: Thursday, May 2, 2019 2:45 PM > > > > > > > > > > > > Ping... > > > > > > > > > > > > Any comments? This has been postponed in the community for a long > > > > time. > > > > > > We need to response this as soon as possible. > > > > > > > > > > > > Best regards, > > > > > > Lu Baolu > > > > > > > > > > > > On 4/29/19 1:19 PM, Lu Baolu wrote: > > > > > > > Hi Zhenyu, > > > > > > > > > > > > > > As we discussed, BIOS always exports IOMMU reserved memory > > > regions > > > > > > > for (a.k.a. RMRR in vt-d spec) Intel integrated graphic device. > > > > > > > This caused some problems when we pass-through such graphic > > > > > > > devices to > > > > user level. > > > > > > > > > > > > > > I am about to propose something to the community so that a RMRR > > > > > > > for graphic devices could be explicitly canceled a
Re: [Intel-gfx] [PATCH v4 4/8] drm/i915: Shuffle stride checking code around
On Thu, May 09, 2019 at 03:21:55PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Reorganize some fb stride checking code a bit to prepare for
> gtt remapping. And do a bit of s/pitch/stride/ renaming in the
> process for a bit more uniformity (apart from the whole
> fb->pitches[] thing).
>
> Cc: Daniel Vetter
> Signed-off-by: Ville Syrjälä
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_display.c | 64 ++--
> 1 file changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f4e6e9a8bbf9..321cb6d2bc76 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2509,6 +2509,33 @@ bool is_ccs_modifier(u64 modifier)
> modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
> }
>
> +static
> +u32 intel_fb_max_stride(struct drm_i915_private *dev_priv,
> + u32 pixel_format, u64 modifier)
> +{
> + struct intel_crtc *crtc;
> + struct intel_plane *plane;
> +
> + /*
> + * We assume the primary plane for pipe A has
> + * the highest stride limits of them all.
> + */
> + crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> + plane = to_intel_plane(crtc->base.primary);
> +
> + return plane->max_stride(plane, pixel_format, modifier,
> + DRM_MODE_ROTATE_0);
> +}
> +
> +static u32
> +intel_fb_stride_alignment(const struct drm_framebuffer *fb, int color_plane)
> +{
> + if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
> + return 64;
> + else
> + return intel_tile_width_bytes(fb, color_plane);
> +}
> +
> static int
> intel_fill_fb_info(struct drm_i915_private *dev_priv,
> struct drm_framebuffer *fb)
> @@ -3586,15 +3613,6 @@ static bool i9xx_plane_get_hw_state(struct intel_plane
> *plane,
> return ret;
> }
>
> -static u32
> -intel_fb_stride_alignment(const struct drm_framebuffer *fb, int color_plane)
> -{
> - if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
> - return 64;
> - else
> - return intel_tile_width_bytes(fb, color_plane);
> -}
> -
> static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
> {
> struct drm_device *dev = intel_crtc->base.dev;
> @@ -14921,31 +14939,13 @@ static const struct drm_framebuffer_funcs
> intel_fb_funcs = {
> .dirty = intel_user_framebuffer_dirty,
> };
>
> -static
> -u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
> - u32 pixel_format, u64 fb_modifier)
> -{
> - struct intel_crtc *crtc;
> - struct intel_plane *plane;
> -
> - /*
> - * We assume the primary plane for pipe A has
> - * the highest stride limits of them all.
> - */
> - crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> - plane = to_intel_plane(crtc->base.primary);
> -
> - return plane->max_stride(plane, pixel_format, fb_modifier,
> - DRM_MODE_ROTATE_0);
> -}
> -
> static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> struct drm_i915_gem_object *obj,
> struct drm_mode_fb_cmd2 *mode_cmd)
> {
> struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> struct drm_framebuffer *fb = &intel_fb->base;
> - u32 pitch_limit;
> + u32 max_stride;
> unsigned int tiling, stride;
> int ret = -EINVAL;
> int i;
> @@ -14997,13 +14997,13 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> goto err;
> }
>
> - pitch_limit = intel_fb_pitch_limit(dev_priv, mode_cmd->pixel_format,
> -mode_cmd->modifier[0]);
> - if (mode_cmd->pitches[0] > pitch_limit) {
> + max_stride = intel_fb_max_stride(dev_priv, mode_cmd->pixel_format,
> + mode_cmd->modifier[0]);
> + if (mode_cmd->pitches[0] > max_stride) {
> DRM_DEBUG_KMS("%s pitch (%u) must be at most %d\n",
> mode_cmd->modifier[0] != DRM_FORMAT_MOD_LINEAR ?
> "tiled" : "linear",
> - mode_cmd->pitches[0], pitch_limit);
> + mode_cmd->pitches[0], max_stride);
> goto err;
> }
>
> --
> 2.21.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 v4 6/8] drm/i915: Align dumb buffer stride to 4k to allow for gtt remapping
On Thu, May 09, 2019 at 03:21:57PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Align dumb buffer stride to 4k if the fb will be big enough to
> require gtt remapping.
>
> v2: Leave the stride alone for buffers that look to be for the cursor
> v3: Make it not a hack (Daniel)
>
> Cc: Daniel Vetter
> Signed-off-by: Ville Syrjälä
Yeah I think this is a reasonable heuristics.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem.c | 26 +-
> drivers/gpu/drm/i915/intel_display.c | 1 -
> drivers/gpu/drm/i915/intel_display.h | 2 ++
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 4e474bcf4c22..7cafd5612f71 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -52,6 +52,7 @@
> #include "i915_trace.h"
> #include "i915_vgpu.h"
>
> +#include "intel_display.h"
> #include "intel_drv.h"
> #include "intel_frontbuffer.h"
> #include "intel_pm.h"
> @@ -560,8 +561,31 @@ i915_gem_dumb_create(struct drm_file *file,
>struct drm_device *dev,
>struct drm_mode_create_dumb *args)
> {
> + int cpp = DIV_ROUND_UP(args->bpp, 8);
> + u32 format;
> +
> + switch (cpp) {
> + case 1:
> + format = DRM_FORMAT_C8;
> + break;
> + case 2:
> + format = DRM_FORMAT_RGB565;
> + break;
> + case 4:
> + format = DRM_FORMAT_XRGB;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> /* have to work out size/pitch and return them */
> - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64);
> + args->pitch = ALIGN(args->width * cpp, 64);
> +
> + /* align stride to page size so that we can remap */
> + if (args->pitch > intel_plane_fb_max_stride(to_i915(dev), format,
> + DRM_FORMAT_MOD_LINEAR))
> + args->pitch = ALIGN(args->pitch, 4096);
> +
> args->size = args->pitch * args->height;
> return i915_gem_create(file, to_i915(dev),
> &args->size, &args->handle);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 94faac9e3666..fa317c40d548 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2498,7 +2498,6 @@ bool is_ccs_modifier(u64 modifier)
> modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
> }
>
> -static
> u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> u32 pixel_format, u64 modifier)
> {
> diff --git a/drivers/gpu/drm/i915/intel_display.h
> b/drivers/gpu/drm/i915/intel_display.h
> index 500eec90928d..1e6533fbd061 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -436,6 +436,8 @@ void intel_link_compute_m_n(u16 bpp, int nlanes,
> bool constant_n);
> bool is_ccs_modifier(u64 modifier);
> void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
> +u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> + u32 pixel_format, u64 modifier);
> bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
>
> #endif
> --
> 2.21.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] RFC: console: hack up console_lock more v3
On Thu 2019-05-09 18:43:12, Daniel Vetter wrote:
> One thing to keep in mind is that the kernel is already dying, and
> things will come crashing down later on
This is important information. I havn't seen it mentioned earlier.
> (I've seen this only in dmesg
> tails capture in pstore in our CI, i.e. the box died for good). I just
> want to make sure that the useful information isn't overwritten by
> more dmesg splats that happen as a consequence of us somehow trying to
> run things on an offline cpu. Once console_unlock has completed in
> your above backtrace and the important stuff has gone out I'm totally
> fine with the kernel just dying. Pulling the wake_up_process out from
> under the semaphore.lock is enough to prevent lockdep from dumping
> more stuff while we're trying to print the important things,
With the more stuff you mean the lockdep splat? If yes, it might
make sense to call debug_locks_off() earlier in panic().
> and I think the untangling of the locking hiararchy is useful irrespective
> of this lockdep splat. Plus Peter doesn't sound like he likes to roll
> out more printk_deferred kind of things.
>
> But if you think I should do the printk_deferred thing too I can look
> into that. Just not quite sure what that's supposed to look like now.
Your patch might remove the particular lockdep splat. It might be
worth it (Peter mentioned also an optimization effect). Anyway
it will not prevent the deadlock.
The only way to avoid the deadlock is to use printk_deferred()
with the current printk() code.
Finally, I have recently worked on similar problem with dying system.
I sent the following patch for testing. I wonder if it might be
acceptable upstream:
From: Petr Mladek
Subject: sched/x86: Do not warn about offline CPUs when all are being stopped
Patch-mainline: No, just for testing
References: bsc#1104406
The warning about rescheduling offline CPUs cause dealock when
the CPUs need to get stopped using NMI. It might happen with
logbuf_lock, locks used by console drivers, especially tty.
But it might also be caused by a registered kernel message
dumper, for example, pstore.
The warning is pretty common when there is a high load and
CPUs are being stopped by native_stop_other_cpus(). But
they are not really useful in this context. And they scrolls
the really important messages off the screen.
We need to fix printk() in the long term. But disabling
the message looks reasonable at least in the meantime.
Signed-off-by: Petr Mladek
---
arch/x86/kernel/smp.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -124,7 +124,8 @@ static bool smp_no_nmi_ipi = false;
*/
static void native_smp_send_reschedule(int cpu)
{
- if (unlikely(cpu_is_offline(cpu))) {
+ if (unlikely(cpu_is_offline(cpu) &&
+atomic_read(&stopping_cpu) < 0)) {
WARN(1, "sched: Unexpected reschedule of offline CPU#%d!\n",
cpu);
return;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] kernel/locking/semaphore: use wake_q in up()
On Thu 2019-05-09 22:06:33, Daniel Vetter wrote:
> console_trylock, called from within printk, can be called from pretty
> much anywhere. Including try_to_wake_up. Note that this isn't common,
> usually the box is in pretty bad shape at that point already. But it
> really doesn't help when then lockdep jumps in and spams the logs,
> potentially obscuring the real backtrace we're really interested in.
> One case I've seen (slightly simplified backtrace):
>
> Fix this specific locking recursion by moving the wake_up_process out
> from under the semaphore.lock spinlock, using wake_q as recommended by
> Peter Zijlstra.
It might make sense to mention also the optimization effect mentioned
by Peter.
> diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> index 561acdd39960..7a6f33715688 100644
> --- a/kernel/locking/semaphore.c
> +++ b/kernel/locking/semaphore.c
> @@ -169,6 +169,14 @@ int down_timeout(struct semaphore *sem, long timeout)
> }
> EXPORT_SYMBOL(down_timeout);
>
> +/* Functions for the contended case */
> +
> +struct semaphore_waiter {
> + struct list_head list;
> + struct task_struct *task;
> + bool up;
> +};
> +
> /**
> * up - release the semaphore
> * @sem: the semaphore to release
> @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> void up(struct semaphore *sem)
> {
> unsigned long flags;
> + struct semaphore_waiter *waiter;
> + DEFINE_WAKE_Q(wake_q);
We need to call wake_q_init(&wake_q) to make sure that
it is empty.
Best Regards,
Petr
> raw_spin_lock_irqsave(&sem->lock, flags);
> - if (likely(list_empty(&sem->wait_list)))
> + if (likely(list_empty(&sem->wait_list))) {
> sem->count++;
> - else
> - __up(sem);
> + } else {
> + waiter = list_first_entry(&sem->wait_list,
> +struct semaphore_waiter, list);
> + list_del(&waiter->list);
> + waiter->up = true;
> + wake_q_add(&wake_q, waiter->task);
> + }
> raw_spin_unlock_irqrestore(&sem->lock, flags);
> +
> + wake_up_q(&wake_q);
> }
> EXPORT_SYMBOL(up);
>
> -/* Functions for the contended case */
> -
> -struct semaphore_waiter {
> - struct list_head list;
> - struct task_struct *task;
> - bool up;
> -};
> -
> /*
> * Because this function is inlined, the 'state' parameter will be
> * constant, and thus optimised away by the compiler. Likewise the
___
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/dp: Support for DP YCbCr4:2:0 outputs (rev2)
== Series Details == Series: drm/i915/dp: Support for DP YCbCr4:2:0 outputs (rev2) URL : https://patchwork.freedesktop.org/series/60404/ State : success == Summary == CI Bug Log - changes from CI_DRM_6072_full -> Patchwork_13000_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13000_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_eio@reset-stress: - shard-snb: [PASS][1] -> [FAIL][2] ([fdo#109661]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-snb6/igt@gem_...@reset-stress.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-snb7/igt@gem_...@reset-stress.html * igt@gem_softpin@noreloc-s3: - shard-skl: [PASS][3] -> [INCOMPLETE][4] ([fdo#104108] / [fdo#107773]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl8/igt@gem_soft...@noreloc-s3.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-skl1/igt@gem_soft...@noreloc-s3.html * igt@gem_tiled_wb: - shard-hsw: [PASS][5] -> [INCOMPLETE][6] ([fdo#103540]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-hsw6/igt@gem_tiled_wb.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-hsw6/igt@gem_tiled_wb.html * igt@kms_flip@flip-vs-expired-vblank: - shard-glk: [PASS][7] -> [FAIL][8] ([fdo#105363]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-glk9/igt@kms_f...@flip-vs-expired-vblank.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-glk6/igt@kms_f...@flip-vs-expired-vblank.html * igt@kms_flip_tiling@flip-x-tiled: - shard-skl: [PASS][9] -> [FAIL][10] ([fdo#108145] / [fdo#108303]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl5/igt@kms_flip_til...@flip-x-tiled.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-skl8/igt@kms_flip_til...@flip-x-tiled.html * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt: - shard-iclb: [PASS][11] -> [FAIL][12] ([fdo#103167]) +3 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb4/igt@kms_frontbuffer_track...@fbc-1p-primscrn-cur-indfb-draw-blt.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-iclb1/igt@kms_frontbuffer_track...@fbc-1p-primscrn-cur-indfb-draw-blt.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [PASS][13] -> [FAIL][14] ([fdo#108145] / [fdo#110403]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-skl1/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-skl8/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html * igt@kms_plane_lowres@pipe-a-tiling-x: - shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103166]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb7/igt@kms_plane_low...@pipe-a-tiling-x.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-iclb1/igt@kms_plane_low...@pipe-a-tiling-x.html * igt@kms_psr@psr2_cursor_plane_onoff: - shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-iclb3/igt@kms_psr@psr2_cursor_plane_onoff.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_6072/shard-apl1/igt@kms_setm...@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-apl2/igt@kms_setm...@basic.html * igt@kms_vblank@pipe-a-ts-continuation-suspend: - shard-apl: [PASS][21] -> [DMESG-WARN][22] ([fdo#108566]) +2 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-apl5/igt@kms_vbl...@pipe-a-ts-continuation-suspend.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-apl4/igt@kms_vbl...@pipe-a-ts-continuation-suspend.html Possible fixes * igt@i915_suspend@debugfs-reader: - shard-apl: [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +6 similar issues [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-apl6/igt@i915_susp...@debugfs-reader.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13000/shard-apl4/igt@i915_susp...@debugfs-reader.html * igt@kms_cursor_edge_walk@pipe-b-64x64-bottom-edge: - shard-snb: [SKIP][25] ([fdo#109271] / [fdo#109278]) -> [PASS][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6072/shard-snb7/igt@kms_cursor_edge_w...@pipe-b-64x64-bottom-e
Re: [Intel-gfx] [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro
On Fri, May 10, 2019 at 09:13:26AM +, Ardelean, Alexandru wrote:
> On Wed, 2019-05-08 at 16:26 +0300, Alexandru Ardelean wrote:
> > On Wed, 2019-05-08 at 15:20 +0300, Dan Carpenter wrote:
> > >
> > >
> > > On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote:
> > > > -static const char * const phy_types[] = {
> > > > - "emmc 5.0 phy",
> > > > - "emmc 5.1 phy"
> > > > -};
> > > > -
> > > > enum xenon_phy_type_enum {
> > > > EMMC_5_0_PHY,
> > > > EMMC_5_1_PHY,
> > > > NR_PHY_TYPES
> > >
> > > There is no need for NR_PHY_TYPES now so you could remove that as well.
> > >
> >
> > I thought the same.
> > The only reason to keep NR_PHY_TYPES, is for potential future patches,
> > where it would be just 1 addition
> >
> > enum xenon_phy_type_enum {
> > EMMC_5_0_PHY,
> > EMMC_5_1_PHY,
> > + EMMC_5_2_PHY,
> > NR_PHY_TYPES
> > }
> >
> > Depending on style/preference of how to do enums (allow comma on last
> > enum
> > or not allow comma on last enum value), adding new enum values woudl be 2
> > additions + 1 deletion lines.
> >
> > enum xenon_phy_type_enum {
> > EMMC_5_0_PHY,
> > - EMMC_5_1_PHY
> > + EMM
> > C_5_1_PHY,
> > + EMMC_5_2_PHY
> > }
> >
> > Either way (leave NR_PHY_TYPES or remove NR_PHY_TYPES) is fine from my
> > side.
> >
>
> Preference on this ?
> If no objection [nobody insists] I would keep.
>
> I don't feel strongly about it [dropping NR_PHY_TYPES or not].
If you end up resending the series could you remove it, but if not then
it's not worth it.
regards,
dan carpenter
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [drm-intel:drm-intel-next-queued 5/6] drivers/gpu/drm/i915/intel_hdcp.c:1406 hdcp2_authenticate_repeater_topology() warn: should this be a bitwise op?
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head: c16fd9be70faf3c49a61700efd16018dd910e390
commit: f26ae6a652f2e75a3a12ac22b7da5797978436c4 [5/6] drm/i915: SRM revocation
check for HDCP1.4 and 2.2
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/i915/intel_hdcp.c:1406 hdcp2_authenticate_repeater_topology()
warn: should this be a bitwise op?
git remote add drm-intel git://anongit.freedesktop.org/drm-intel
git remote update drm-intel
git checkout f26ae6a652f2e75a3a12ac22b7da5797978436c4
vim +1406 drivers/gpu/drm/i915/intel_hdcp.c
d849178e Ramalingam C 2019-02-16 1367 static
d849178e Ramalingam C 2019-02-16 1368 int
hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
d849178e Ramalingam C 2019-02-16 1369 {
d849178e Ramalingam C 2019-02-16 1370 struct intel_digital_port
*intel_dig_port = conn_to_dig_port(connector);
d849178e Ramalingam C 2019-02-16 1371 struct intel_hdcp *hdcp =
&connector->hdcp;
f26ae6a6 Ramalingam C 2019-05-07 1372 struct drm_device *dev =
connector->base.dev;
d849178e Ramalingam C 2019-02-16 1373 union {
d849178e Ramalingam C 2019-02-16 1374 struct
hdcp2_rep_send_receiverid_list recvid_list;
d849178e Ramalingam C 2019-02-16 1375 struct
hdcp2_rep_send_ack rep_ack;
d849178e Ramalingam C 2019-02-16 1376 } msgs;
d849178e Ramalingam C 2019-02-16 1377 const struct intel_hdcp_shim
*shim = hdcp->shim;
f26ae6a6 Ramalingam C 2019-05-07 1378 u32 seq_num_v, device_cnt;
d849178e Ramalingam C 2019-02-16 1379 u8 *rx_info;
d849178e Ramalingam C 2019-02-16 1380 int ret;
d849178e Ramalingam C 2019-02-16 1381
d849178e Ramalingam C 2019-02-16 1382 ret =
shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_SEND_RECVID_LIST,
d849178e Ramalingam C 2019-02-16 1383
&msgs.recvid_list, sizeof(msgs.recvid_list));
d849178e Ramalingam C 2019-02-16 1384 if (ret < 0)
d849178e Ramalingam C 2019-02-16 1385 return ret;
d849178e Ramalingam C 2019-02-16 1386
d849178e Ramalingam C 2019-02-16 1387 rx_info =
msgs.recvid_list.rx_info;
d849178e Ramalingam C 2019-02-16 1388
d849178e Ramalingam C 2019-02-16 1389 if
(HDCP_2_2_MAX_CASCADE_EXCEEDED(rx_info[1]) ||
d849178e Ramalingam C 2019-02-16 1390
HDCP_2_2_MAX_DEVS_EXCEEDED(rx_info[1])) {
d849178e Ramalingam C 2019-02-16 1391 DRM_DEBUG_KMS("Topology
Max Size Exceeded\n");
d849178e Ramalingam C 2019-02-16 1392 return -EINVAL;
d849178e Ramalingam C 2019-02-16 1393 }
d849178e Ramalingam C 2019-02-16 1394
d849178e Ramalingam C 2019-02-16 1395 /* Converting and Storing the
seq_num_v to local variable as DWORD */
0de655ca Ramalingam C 2019-05-07 1396 seq_num_v =
0de655ca Ramalingam C 2019-05-07 1397
drm_hdcp_be24_to_cpu((const u8 *)msgs.recvid_list.seq_num_v);
d849178e Ramalingam C 2019-02-16 1398
d849178e Ramalingam C 2019-02-16 1399 if (seq_num_v <
hdcp->seq_num_v) {
d849178e Ramalingam C 2019-02-16 1400 /* Roll over of the
seq_num_v from repeater. Reauthenticate. */
d849178e Ramalingam C 2019-02-16 1401
DRM_DEBUG_KMS("Seq_num_v roll over.\n");
d849178e Ramalingam C 2019-02-16 1402 return -EINVAL;
d849178e Ramalingam C 2019-02-16 1403 }
d849178e Ramalingam C 2019-02-16 1404
f26ae6a6 Ramalingam C 2019-05-07 1405 device_cnt =
HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 ||
^^
Bitwise OR | was probably intended.
f26ae6a6 Ramalingam C 2019-05-07 @1406
HDCP_2_2_DEV_COUNT_LO(rx_info[1]);
f26ae6a6 Ramalingam C 2019-05-07 1407 if
(drm_hdcp_check_ksvs_revoked(dev, msgs.recvid_list.receiver_ids,
f26ae6a6 Ramalingam C 2019-05-07 1408
device_cnt)) {
f26ae6a6 Ramalingam C 2019-05-07 1409 DRM_ERROR("Revoked
receiver ID(s) is in list\n");
f26ae6a6 Ramalingam C 2019-05-07 1410 return -EPERM;
f26ae6a6 Ramalingam C 2019-05-07 1411 }
f26ae6a6 Ramalingam C 2019-05-07 1412
d849178e Ramalingam C 2019-02-16 1413 ret =
hdcp2_verify_rep_topology_prepare_ack(connector,
d849178e Ramalingam C 2019-02-16 1414
&msgs.recvid_list,
d849178e Ramalingam C 2019-02-16 1415
&msgs.rep_ack);
d849178e Ramalingam C 2019-02-16 1416 if (ret < 0)
d849178e Ramalingam C 2019-02-16 1417 return ret;
d849178e Ramalingam C 2019-02-16 1418
d849178e Ramalingam C 2019-02-16 1419 hdcp->se
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for RFC: console: hack up console_lock more v3 (rev4)
== Series Details == Series: RFC: console: hack up console_lock more v3 (rev4) URL : https://patchwork.freedesktop.org/series/60467/ State : warning == Summary == $ dim checkpatch origin/drm-tip 7e7042b1b9a6 RFC: console: hack up console_lock more v3 -:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #25: > and I think the untangling of the locking hiararchy is useful irrespective total: 0 errors, 1 warnings, 0 checks, 9 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for RFC: console: hack up console_lock more v3 (rev4)
== Series Details ==
Series: RFC: console: hack up console_lock more v3 (rev4)
URL : https://patchwork.freedesktop.org/series/60467/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6073 -> Patchwork_13001
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_13001 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13001, 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_13001/
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_13001:
### IGT changes ###
Possible regressions
* igt@i915_selftest@live_sanitycheck:
- fi-skl-lmem:[PASS][1] -> [INCOMPLETE][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-skl-lmem/igt@i915_selftest@live_sanitycheck.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13001/fi-skl-lmem/igt@i915_selftest@live_sanitycheck.html
Known issues
Here are the changes found in Patchwork_13001 that come from known issues:
### IGT changes ###
Possible fixes
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [DMESG-FAIL][3] ([fdo#110235]) -> [PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13001/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@i915_selftest@live_hangcheck:
- {fi-icl-u2}:[INCOMPLETE][5] ([fdo#107713] / [fdo#108569]) ->
[PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13001/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (47 -> 43)
--
Additional (3): fi-icl-y fi-blb-e6850 fi-apl-guc
Missing(7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan
fi-kbl-7500u fi-pnv-d510 fi-byt-clapper
Build changes
-
* Linux: CI_DRM_6073 -> Patchwork_13001
CI_DRM_6073: c059ddabfe60a5072ace44a34a9de9b4202df6ec @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4978: b9b3646d4f04dd0204ead2a1a10f9e1806a0b622 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13001: 7e7042b1b9a60db79de8464000c3bfe5016e054f @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
7e7042b1b9a6 RFC: console: hack up console_lock more v3
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13001/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [drm-intel:drm-intel-next-queued 4/6] drivers/gpu/drm/drm_hdcp.c:190 drm_hdcp_parse_hdcp2_srm() warn: mask and shift to zero
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head: c16fd9be70faf3c49a61700efd16018dd910e390
commit: 6498bf5800a302ef69e7f4914e727893f278bb2f [4/6] drm: revocation check at
drm subsystem
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/drm_hdcp.c:190 drm_hdcp_parse_hdcp2_srm() warn: mask and shift
to zero
git remote add drm-intel git://anongit.freedesktop.org/drm-intel
git remote update drm-intel
git checkout 6498bf5800a302ef69e7f4914e727893f278bb2f
vim +190 drivers/gpu/drm/drm_hdcp.c
6498bf58 Ramalingam C 2019-05-07 151 static int
drm_hdcp_parse_hdcp2_srm(const u8 *buf, size_t count)
6498bf58 Ramalingam C 2019-05-07 152 {
6498bf58 Ramalingam C 2019-05-07 153 struct hdcp_srm_header *header;
6498bf58 Ramalingam C 2019-05-07 154 u32 vrl_length, ksv_count, ksv_sz;
6498bf58 Ramalingam C 2019-05-07 155
6498bf58 Ramalingam C 2019-05-07 156 if (count < (sizeof(struct
hdcp_srm_header) +
6498bf58 Ramalingam C 2019-05-07 157 DRM_HDCP_2_VRL_LENGTH_SIZE +
DRM_HDCP_2_DCP_SIG_SIZE)) {
6498bf58 Ramalingam C 2019-05-07 158 DRM_ERROR("Invalid blob
length\n");
6498bf58 Ramalingam C 2019-05-07 159 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 160 }
6498bf58 Ramalingam C 2019-05-07 161
6498bf58 Ramalingam C 2019-05-07 162 header = (struct hdcp_srm_header *)buf;
6498bf58 Ramalingam C 2019-05-07 163 DRM_DEBUG("SRM ID: 0x%x, SRM Ver: 0x%x,
SRM Gen No: 0x%x\n",
6498bf58 Ramalingam C 2019-05-07 164 header->srm_id &
DRM_HDCP_SRM_ID_MASK,
6498bf58 Ramalingam C 2019-05-07 165
be16_to_cpu(header->srm_version), header->srm_gen_no);
6498bf58 Ramalingam C 2019-05-07 166
6498bf58 Ramalingam C 2019-05-07 167 if (header->reserved)
6498bf58 Ramalingam C 2019-05-07 168 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 169
6498bf58 Ramalingam C 2019-05-07 170 buf = buf + sizeof(*header);
6498bf58 Ramalingam C 2019-05-07 171 vrl_length = get_vrl_length(buf);
6498bf58 Ramalingam C 2019-05-07 172
6498bf58 Ramalingam C 2019-05-07 173 if (count < (sizeof(struct
hdcp_srm_header) + vrl_length) ||
6498bf58 Ramalingam C 2019-05-07 174 vrl_length <
(DRM_HDCP_2_VRL_LENGTH_SIZE +
6498bf58 Ramalingam C 2019-05-07 175 DRM_HDCP_2_DCP_SIG_SIZE)) {
6498bf58 Ramalingam C 2019-05-07 176 DRM_ERROR("Invalid blob length
or vrl length\n");
6498bf58 Ramalingam C 2019-05-07 177 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 178 }
6498bf58 Ramalingam C 2019-05-07 179
6498bf58 Ramalingam C 2019-05-07 180 /* Length of the all vrls combined */
6498bf58 Ramalingam C 2019-05-07 181 vrl_length -=
(DRM_HDCP_2_VRL_LENGTH_SIZE +
6498bf58 Ramalingam C 2019-05-07 182 DRM_HDCP_2_DCP_SIG_SIZE);
6498bf58 Ramalingam C 2019-05-07 183
6498bf58 Ramalingam C 2019-05-07 184 if (!vrl_length) {
6498bf58 Ramalingam C 2019-05-07 185 DRM_ERROR("No vrl found\n");
6498bf58 Ramalingam C 2019-05-07 186 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 187 }
6498bf58 Ramalingam C 2019-05-07 188
6498bf58 Ramalingam C 2019-05-07 189 buf += DRM_HDCP_2_VRL_LENGTH_SIZE;
6498bf58 Ramalingam C 2019-05-07 @190 ksv_count = (*buf << 2) |
DRM_HDCP_2_KSV_COUNT_2_LSBITS(*(buf + 1));
#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte) (((byte) & 0xC) >> 6)
0xC >> 6 is always zero.
6498bf58 Ramalingam C 2019-05-07 191 if (!ksv_count) {
6498bf58 Ramalingam C 2019-05-07 192 DRM_DEBUG("Revoked KSV count is
0\n");
6498bf58 Ramalingam C 2019-05-07 193 return count;
6498bf58 Ramalingam C 2019-05-07 194 }
6498bf58 Ramalingam C 2019-05-07 195
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 01/21] scripts/trace.pl: Fix after intel_engine_notify removal
Quoting Tvrtko Ursulin (2019-05-08 13:10:38)
> From: Tvrtko Ursulin
>
> After the removal of engine global seqnos and the corresponding
> intel_engine_notify tracepoints the script needs to be adjusted to cope
> with the new state of things.
>
> To keep working it switches over using the dma_fence:dma_fence_signaled:
> tracepoint and keeps one extra internal map to connect the ctx-seqno pairs
> with engines.
>
> It also needs to key the completion events on the full engine/ctx/seqno
> tokens, and adjust correspondingly the timeline sorting logic.
>
> v2:
> * Do not use late notifications (received after context complete) when
>splitting up coalesced requests. They are now much more likely and can
>not be used.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> scripts/trace.pl | 82
> 1 file changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/scripts/trace.pl b/scripts/trace.pl
> index 18f9f3b18396..95dc3a645e8e 100755
> --- a/scripts/trace.pl
> +++ b/scripts/trace.pl
> @@ -27,7 +27,8 @@ use warnings;
> use 5.010;
>
> my $gid = 0;
> -my (%db, %queue, %submit, %notify, %rings, %ctxdb, %ringmap, %reqwait,
> %ctxtimelines);
> +my (%db, %queue, %submit, %notify, %rings, %ctxdb, %ringmap, %reqwait,
> +%ctxtimelines, %ctxengines);
> my @freqs;
So what's ctxengines? Or rings for that matter?
I take it ctxengines is really the last engine which we saw this context
execute on?
>
> my $max_items = 3000;
> @@ -66,7 +67,7 @@ Notes:
>i915:i915_request_submit, \
>i915:i915_request_in, \
>i915:i915_request_out, \
> - i915:intel_engine_notify, \
> + dma_fence:dma_fence_signaled, \
>i915:i915_request_wait_begin, \
>i915:i915_request_wait_end \
>[command-to-be-profiled]
> @@ -161,7 +162,7 @@ sub arg_trace
>'i915:i915_request_submit',
>'i915:i915_request_in',
>'i915:i915_request_out',
> - 'i915:intel_engine_notify',
> + 'dma_fence:dma_fence_signaled',
>'i915:i915_request_wait_begin',
>'i915:i915_request_wait_end' );
>
> @@ -312,13 +313,6 @@ sub db_key
> return $ring . '/' . $ctx . '/' . $seqno;
> }
>
> -sub global_key
> -{
> - my ($ring, $seqno) = @_;
> -
> - return $ring . '/' . $seqno;
> -}
> -
> sub sanitize_ctx
> {
> my ($ctx, $ring) = @_;
> @@ -419,6 +413,8 @@ while (<>) {
> $req{'ring'} = $ring;
> $req{'seqno'} = $seqno;
> $req{'ctx'} = $ctx;
> + die if exists $ctxengines{$ctx} and $ctxengines{$ctx} ne
> $ring;
> + $ctxengines{$ctx} = $ring;
> $ctxtimelines{$ctx . '/' . $ring} = 1;
> $req{'name'} = $ctx . '/' . $seqno;
> $req{'global'} = $tp{'global'};
> @@ -429,16 +425,29 @@ while (<>) {
> $ringmap{$rings{$ring}} = $ring;
> $db{$key} = \%req;
> } elsif ($tp_name eq 'i915:i915_request_out:') {
> - my $gkey = global_key($ring, $tp{'global'});
> + my $gkey;
> +
# Must be paired with a previous i915_request_in
> + die unless exists $ctxengines{$ctx};
I'd suggest next unless, because there's always a change the capture is
started part way though someone's workload.
> + $gkey = db_key($ctxengines{$ctx}, $ctx, $seqno);
> +
> + if ($tp{'completed?'}) {
> + die unless exists $db{$key};
> + die unless exists $db{$key}->{'start'};
> + die if exists $db{$key}->{'end'};
> +
> + $db{$key}->{'end'} = $time;
> + $db{$key}->{'notify'} = $notify{$gkey}
> + if exists $notify{$gkey};
Hmm. With preempt-to-busy, a request can complete when we are no longer
tracking it (it completes before we preempt it).
They will still get the schedule-out tracepoint, but marked as
incomplete, and there will be a signaled tp later before we try and
resubmit.
> + } else {
> + delete $db{$key};
> + }
> + } elsif ($tp_name eq 'dma_fence:dma_fence_signaled:') {
> + my $gkey;
>
> - die unless exists $db{$key};
> - die unless exists $db{$key}->{'start'};
> - die if exists $db{$key}->{'end'};
> + die unless exists $ctxengines{$tp{'context'}};
>
> - $db{$key}->{'end'} = $time;
> - $db{$key}->{'notify'} = $notify{$gkey} if exists
> $notify{$gkey};
> - } elsif ($tp_name eq 'i915:intel_engine_not
Re: [Intel-gfx] [PATCH i-g-t 03/21] trace.pl: Virtual engine support
Quoting Tvrtko Ursulin (2019-05-08 13:10:40)
> From: Tvrtko Ursulin
>
> Add virtual/queue timelines to both stdout and HTML output.
>
> A new timeline is created for each queue/virtual engine to display
> associated requests in queued and runnable states. Once requests are
> submitted to a real engine for executing they show up on the physical
> engine timeline.
How does it cope with preemption events that shift the request onto
another engine?
Queues. So why are virtual engines treated differently, from my pov it's
just a timeline like any other, the only difference is that it my
execute on a different engine? My expectation would have been that
tracking would have been timeline centric.
However, I think I am confusing my perspective of timelines in the
kernel with the visualisation timelines.
> +sub is_veng
> +{
> + my ($class, $instance) = split ':', shift;
> +
> + return $instance eq '254';
Ok. I thought I might have caught you out.
> + unless (exists $queue{$key}) {
> + # Virtual engine
> + my $vkey = db_key(VENG, $ctx, $seqno);
> + my %req;
> +
> + die unless exists $queues{$ctx};
> + die unless exists $queue{$vkey};
> + die unless exists $submit{$vkey};
> +
> + # Create separate request record on the queue timeline
> + $q = $queue{$vkey};
> + $s = $submit{$vkey};
> + $req{'queue'} = $q;
> + $req{'submit'} = $s;
> + $req{'start'} = $time;
> + $req{'end'} = $time;
> + $req{'ring'} = VENG;
> + $req{'seqno'} = $seqno;
> + $req{'ctx'} = $ctx;
> + $req{'name'} = $ctx . '/' . $seqno;
> + $req{'global'} = $tp{'global'};
> + $req{'port'} = $tp{'port'};
Just quietly thinking why not adopt this for each timeline; create a
on-engine event box for all.
> +
> + $vdb{$vkey} = \%req;
> + } else {
> + $q = $queue{$key};
> + $s = $submit{$key};
> + }
>
> $req{'start'} = $time;
> $req{'ring'} = $ring;
> sub stdio_stats
> {
> my ($stats, $group, $id) = @_;
> + my $veng = exists $stats->{'virtual'} ? 1 : 0;
> my $str;
>
> - $str = 'Ring' . $group . ': ';
> + $str = $veng ? 'Virtual' : 'Ring';
> + $str .= $group . ': ';
> $str .= $stats->{'count'} . ' batches, ';
> - $str .= sprintf('%.2f (%.2f) avg batch us, ', $stats->{'avg'},
> $stats->{'total-avg'});
> - $str .= sprintf('%.2f', $stats->{'idle'}) . '% idle, ';
> - $str .= sprintf('%.2f', $stats->{'busy'}) . '% busy, ';
> + unless ($veng) {
> + $str .= sprintf('%.2f (%.2f) avg batch us, ',
> + $stats->{'avg'}, $stats->{'total-avg'});
> + $str .= sprintf('%.2f', $stats->{'idle'}) . '% idle, ';
> + $str .= sprintf('%.2f', $stats->{'busy'}) . '% busy, ';
> + }
> +
> $str .= sprintf('%.2f', $stats->{'runnable'}) . '% runnable, ';
> $str .= sprintf('%.2f', $stats->{'queued'}) . '% queued, ';
> $str .= sprintf('%.2f', $stats->{'wait'}) . '% wait';
So I'm looking that the utilisation, trying to figure out why veng
matters? Do we not breakdown utilisation for the real engines, plus
utilisation on each client timeline?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 04/21] trace.pl: Virtual engine preemption support
Quoting Tvrtko Ursulin (2019-05-08 13:10:41)
> From: Tvrtko Ursulin
>
> Use the 'completed?' tracepoint field to detect more robustly when a
> request has been preempted and remove it from the engine database if so.
>
> Otherwise the script can hit a scenario where the same global seqno will
> be mentioned multiple times (on an engine seqno) which aborts processing.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> scripts/trace.pl | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/trace.pl b/scripts/trace.pl
> index 6cc332bb6e2a..cb7cc46df22e 100755
> --- a/scripts/trace.pl
> +++ b/scripts/trace.pl
> @@ -483,17 +483,17 @@ while (<>) {
> $ringmap{$rings{$ring}} = $ring;
> $db{$key} = \%req;
> } elsif ($tp_name eq 'i915:i915_request_out:') {
> - my $gkey;
> -
> die unless exists $ctxengines{$ctx};
>
> - $gkey = db_key($ctxengines{$ctx}, $ctx, $seqno);
> -
> if ($tp{'completed?'}) {
> + my $gkey;
> +
> die unless exists $db{$key};
> die unless exists $db{$key}->{'start'};
> die if exists $db{$key}->{'end'};
>
> + $gkey = db_key($ctxengines{$ctx}, $ctx, $seqno);
I'm lost, how does do the commit message? I thought db_key() just gave
the hash value and not alter the db?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
Hi,
On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote:
> DRM API for generating uevent for a status changes of connector's
> property.
>
> This uevent will have following details related to the status change:
>
> HOTPLUG=1, CONNECTOR= and PROPERTY=
>
> Need ACK from this uevent from userspace consumer.
So we just had some discussions over on IRC and at about the hotplug
issue and came up with similar ideas:
https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html
The conclusions of these discussions so far would be to have a more or
less fine grain of uevent reporting depending on what happened. The
point is that we need to cover different cases:
- one or more properties changed;
- the connector status changed;
- something else about the connector changed (e.g. EDID/modes)
For the first case, we can send out:
HOTPLUG=1
CONNECTOR=
PROPERTY=
and no reprobe is required.
For the second one, something like:
HOTPLUG=1
CONNECTOR=
STATUS=Connected/Disconnected
and a connector probe is needed for connected, but not for
disconnected;
For the third one, we can only indicate the connector:
HOTPLUG=1
CONNECTOR=
and a reprobe of the connector is always needed
Then we still have the legacy case:
HOTPLUG=1
where userspace is expected to reprobe all the connectors.
I think this would deserve to be a separate series on its own. So I am
proposing to take this one off your plate and come up with another
seres implementing this proposal. What do you think?
Cheers,
Paul
> v2:
> Minor fixes at KDoc comments [Daniel]
> v3:
> Check the property is really attached with connector [Daniel]
>
> Signed-off-by: Ramalingam C
> Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_sysfs.c | 35 +++
> include/drm/drm_sysfs.h | 5 -
> 2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 18b1ac442997..63fa951a20db 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include "drm_internal.h"
> +#include "drm_crtc_internal.h"
>
> #define to_drm_minor(d) dev_get_drvdata(d)
> #define to_drm_connector(d) dev_get_drvdata(d)
> @@ -320,6 +321,9 @@ void drm_sysfs_lease_event(struct drm_device *dev)
> * Send a uevent for the DRM device specified by @dev. Currently we only
> * set HOTPLUG=1 in the uevent environment, but this could be expanded to
> * deal with other types of events.
> + *
> + * Any new uapi should be using the drm_sysfs_connector_status_event()
> + * for uevents on connector status change.
> */
> void drm_sysfs_hotplug_event(struct drm_device *dev)
> {
> @@ -332,6 +336,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>
> +/**
> + * drm_sysfs_connector_status_event - generate a DRM uevent for connector
> + * property status change
> + * @connector: connector on which property status changed
> + * @property: connector property whoes status changed.
> + *
> + * Send a uevent for the DRM device specified by @dev. Currently we
> + * set HOTPLUG=1 and connector id along with the attached property id
> + * related to the status change.
> + */
> +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> + struct drm_property *property)
> +{
> + struct drm_device *dev = connector->dev;
> + char hotplug_str[] = "HOTPLUG=1", conn_id[30], prop_id[30];
> + char *envp[4] = { hotplug_str, conn_id, prop_id, NULL };
> +
> + WARN_ON(!drm_mode_obj_find_prop_id(&connector->base,
> +property->base.id));
> +
> + snprintf(conn_id, ARRAY_SIZE(conn_id),
> + "CONNECTOR=%u", connector->base.id);
> + snprintf(prop_id, ARRAY_SIZE(prop_id),
> + "PROPERTY=%u", property->base.id);
> +
> + DRM_DEBUG("generating connector status event\n");
> +
> + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +}
> +EXPORT_SYMBOL(drm_sysfs_connector_status_event);
> +
> static void drm_sysfs_release(struct device *dev)
> {
> kfree(dev);
> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> index 4f311e836cdc..d454ef617b2c 100644
> --- a/include/drm/drm_sysfs.h
> +++ b/include/drm/drm_sysfs.h
> @@ -4,10 +4,13 @@
>
> struct drm_device;
> struct device;
> +struct drm_connector;
> +struct drm_property;
>
> int drm_class_device_register(struct device *dev);
> void drm_class_device_unregister(struct device *dev);
>
> void drm_sysfs_hotplug_event(struct drm_device *dev);
> -
> +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> + struct drm_property *property);
> #endif
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
___
Intel-gfx mailing list
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 07/21] gem_wsim: Factor out common error handling
Quoting Tvrtko Ursulin (2019-05-08 13:10:44) > From: Tvrtko Ursulin > > There is a repeated pattern with error handling which can be moved to a > macro to for better readability in the command parsing loop. > > Signed-off-by: Tvrtko Ursulin Bah, at the cost of including control-flow buried inside the macro. Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 08/21] gem_wsim: More wsim_err
Quoting Tvrtko Ursulin (2019-05-08 13:10:45) > From: Tvrtko Ursulin > > A few more opportunities to compact the code by using the error logging > helper. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson I had to double check that wsim_err() wasn't the magic cf macro. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 06/21] gem_wsim: Use IGT uapi headers
Quoting Tvrtko Ursulin (2019-05-08 13:10:43) > From: Tvrtko Ursulin > > We are moving towards bumping the uAPI headers more often instead of using > too much local struct/ioctl/param definitions since the latter are more > challenging for rebase and maintenance. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 05/21] wsim/media-bench: i915 balancing
Quoting Tvrtko Ursulin (2019-05-08 13:10:42)
> From: Tvrtko Ursulin
>
> Support i915 virtual engine from gem_wsim (-b i915) and media-bench.pl
>
> Signed-off-by: Tvrtko Ursulin
> ---
> + /*
> +* Create and configure contexts.
> +*/
> + for (i = 0; i < wrk->nr_ctxs; i += 2) {
> + struct ctx *ctx = &wrk->ctx_list[i];
> + uint32_t ctx_id, share_vm = 0;
>
> - wrk->ctx_list[w->context].id = arg.ctx_id;
> + if (ctx->id)
> + continue;
>
> - if (flags & GLOBAL_BALANCE) {
> - wrk->ctx_list[w->context].static_vcs =
> context_vcs_rr;
> - context_vcs_rr ^= 1;
> - } else {
> - wrk->ctx_list[w->context].static_vcs =
> ctx_vcs;
> - ctx_vcs ^= 1;
> - }
> + if (flags & I915) {
vm sharing shouldn't be a i915-balancer only option. For single jobs split
across multiple contexts, I would expect they will want to share vm.
> + struct drm_i915_gem_context_create_ext_setparam ext =
> {
> + .base.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> + .param.param = I915_CONTEXT_PARAM_VM,
> + };
> + struct drm_i915_gem_context_create_ext args = { };
>
> - if (wrk->prio) {
> + /* Find existing context to share ppgtt with. */
> + for (j = 0; j < wrk->nr_ctxs; j++) {
> struct drm_i915_gem_context_param param = {
> - .ctx_id = arg.ctx_id,
> - .param = I915_CONTEXT_PARAM_PRIORITY,
> - .value = wrk->prio,
> + .param = I915_CONTEXT_PARAM_VM,
> };
> - gem_context_set_param(fd, ¶m);
> +
> + if (!wrk->ctx_list[j].id)
> + continue;
> +
> + param.ctx_id = wrk->ctx_list[j].id;
> +
> + gem_context_get_param(fd, ¶m);
> + igt_assert(param.value);
> +
> + share_vm = param.value;
> +
> + ext.param.value = share_vm;
> + args.flags =
> + I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS;
> + args.extensions = to_user_pointer(&ext);
> + break;
> }
> +
> + if (!ctx->targets_instance)
> + args.flags |=
> +
> I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE;
> +
> + drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT,
> +&args);
> +
> + ctx_id = args.ctx_id;
> + } else {
> + struct drm_i915_gem_context_create args = {};
> +
> + drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE,
> &args);
> + ctx_id = args.ctx_id;
> + }
> +
> + igt_assert(ctx_id);
> + ctx->id = ctx_id;
> +
> + if (flags & GLOBAL_BALANCE) {
> + ctx->static_vcs = context_vcs_rr;
> + context_vcs_rr ^= 1;
> + } else {
> + ctx->static_vcs = ctx_vcs;
> + ctx_vcs ^= 1;
> + }
> +
> + __ctx_set_prio(ctx_id, wrk->prio);
> +
> + /*
> +* Do we need a separate context to satisfy this workloads
> which
> +* both want to target specific engines and be balanced by
> i915?
> +*/
> + if ((flags & I915) && ctx->wants_balance &&
> + ctx->targets_instance) {
> + struct drm_i915_gem_context_create_ext_setparam ext =
> {
> + .base.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> + .param.param = I915_CONTEXT_PARAM_VM,
> + .param.value = share_vm,
> + };
> + struct drm_i915_gem_context_create_ext args = {
> + .extensions = to_user_pointer(&ext),
> + .flags =
> + I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS |
> + I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE,
> + };
> +
> + igt_assert(share_vm);
> +
> +
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 10/21] gem_wsim: Extract str to engine lookup
Quoting Tvrtko Ursulin (2019-05-08 13:10:47)
> From: Tvrtko Ursulin
>
> Signed-off-by: Tvrtko Ursulin
> ---
> benchmarks/gem_wsim.c | 34 +-
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index 5245692df6eb..f654decb24cc 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -318,6 +318,18 @@ wsim_err(const char *fmt, ...)
> } \
> }
>
> +static int str_to_engine(const char *str)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(ring_str_map); i++) {
> + if (!strcasecmp(str, ring_str_map[i]))
> + return i;
> + }
> +
> + return -1;
> +}
> +
> static struct workload *
> parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
> {
> @@ -480,22 +492,18 @@ parse_workload(struct w_arg *arg, unsigned int flags,
> struct workload *app_w)
> }
>
> if ((field = strtok_r(fstart, ".", &fctx)) != NULL) {
> - unsigned int old_valid = valid;
> -
> fstart = NULL;
>
> - for (i = 0; i < ARRAY_SIZE(ring_str_map); i++) {
> - if (!strcasecmp(field, ring_str_map[i])) {
> - step.engine = i;
> - if (step.engine == BCS)
> - bcs_used = true;
> - valid++;
> - break;
> - }
> - }
> -
> - check_arg(old_valid == valid,
> + i = str_to_engine(field);
> + check_arg(i < 0,
> "Invalid engine id at step %u!\n",
> nr_steps);
> + if (i >= 0)
> + valid++;
check_arg() returned already for all i < 0, no?
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 6/6] drm/i915: Add sysfs toggle to enable per-client engine stats
From: Tvrtko Ursulin
By default we are not collecting any per-engine and per-context
statistcs.
Add a new sysfs toggle to enable this facility:
$ echo 1 >/sys/class/drm/card0/clients/enable_stats
v2: Rebase.
v3: sysfs_attr_init.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++
drivers/gpu/drm/i915/i915_sysfs.c | 73 +++
2 files changed, 77 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e663924dfc78..490d863d1e1e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2082,6 +2082,10 @@ struct drm_i915_private {
struct i915_drm_clients {
struct kobject *root;
atomic_t serial;
+ struct {
+ bool enabled;
+ struct device_attribute attr;
+ } stats;
} clients;
struct i915_hdcp_comp_master *hdcp_master;
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 06a558436f54..b60c0775951a 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -571,9 +571,67 @@ static void i915_setup_error_capture(struct device *kdev)
{}
static void i915_teardown_error_capture(struct device *kdev) {}
#endif
+static ssize_t
+show_client_stats(struct device *kdev, struct device_attribute *attr, char
*buf)
+{
+ struct drm_i915_private *i915 =
+ container_of(attr, struct drm_i915_private, clients.stats.attr);
+
+ return snprintf(buf, PAGE_SIZE, "%u\n", i915->clients.stats.enabled);
+}
+
+static ssize_t
+store_client_stats(struct device *kdev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_i915_private *i915 =
+ container_of(attr, struct drm_i915_private, clients.stats.attr);
+ bool disable = false;
+ bool enable = false;
+ bool val = false;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ int ret;
+
+/* Use RCS as proxy for all engines. */
+ if (!intel_engine_supports_stats(i915->engine[RCS0]))
+ return -EINVAL;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ ret = i915_mutex_lock_interruptible(&i915->drm);
+ if (ret)
+ return ret;
+
+ if (val && !i915->clients.stats.enabled)
+ enable = true;
+ else if (!val && i915->clients.stats.enabled)
+ disable = true;
+
+ if (!enable && !disable)
+ goto out;
+
+ for_each_engine(engine, i915, id) {
+ if (enable)
+ WARN_ON_ONCE(intel_enable_engine_stats(engine));
+ else if (disable)
+ intel_disable_engine_stats(engine);
+ }
+
+ i915->clients.stats.enabled = val;
+
+out:
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ return count;
+}
+
void i915_setup_sysfs(struct drm_i915_private *dev_priv)
{
struct device *kdev = dev_priv->drm.primary->kdev;
+ struct device_attribute *attr;
int ret;
dev_priv->clients.root =
@@ -581,6 +639,18 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
if (!dev_priv->clients.root)
DRM_ERROR("Per-client sysfs setup failed\n");
+ attr = &dev_priv->clients.stats.attr;
+ sysfs_attr_init(&attr->attr);
+ attr->attr.name = "enable_stats";
+ attr->attr.mode = 0664;
+ attr->show = show_client_stats;
+ attr->store = store_client_stats;
+
+ ret = sysfs_create_file(dev_priv->clients.root,
+ (struct attribute *)attr);
+ if (ret)
+ DRM_ERROR("Per-client sysfs setup failed! (%d)\n", ret);
+
#ifdef CONFIG_PM
if (HAS_RC6(dev_priv)) {
ret = sysfs_merge_group(&kdev->kobj,
@@ -642,6 +712,9 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group);
#endif
+ sysfs_remove_file(dev_priv->clients.root,
+ (struct attribute *)&dev_priv->clients.stats.attr);
+
if (dev_priv->clients.root)
kobject_put(dev_priv->clients.root);
}
--
2.19.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 1/6] drm/i915: Move intel_engine_context_in/out into intel_lrc.c
From: Tvrtko Ursulin
Intel_lrc.c is the only caller and so to avoid some header file ordering
issues in future patches move these two over there.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/intel_engine.h | 55 -
drivers/gpu/drm/i915/gt/intel_lrc.c| 57 ++
2 files changed, 57 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
b/drivers/gpu/drm/i915/gt/intel_engine.h
index 9359b3a7ad9c..3236153b5058 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -463,61 +463,6 @@ void intel_engine_dump(struct intel_engine_cs *engine,
struct intel_engine_cs *
intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance);
-static inline void intel_engine_context_in(struct intel_engine_cs *engine)
-{
- unsigned long flags;
-
- if (READ_ONCE(engine->stats.enabled) == 0)
- return;
-
- write_seqlock_irqsave(&engine->stats.lock, flags);
-
- if (engine->stats.enabled > 0) {
- if (engine->stats.active++ == 0)
- engine->stats.start = ktime_get();
- GEM_BUG_ON(engine->stats.active == 0);
- }
-
- write_sequnlock_irqrestore(&engine->stats.lock, flags);
-}
-
-static inline void intel_engine_context_out(struct intel_engine_cs *engine)
-{
- unsigned long flags;
-
- if (READ_ONCE(engine->stats.enabled) == 0)
- return;
-
- write_seqlock_irqsave(&engine->stats.lock, flags);
-
- if (engine->stats.enabled > 0) {
- ktime_t last;
-
- if (engine->stats.active && --engine->stats.active == 0) {
- /*
-* Decrement the active context count and in case GPU
-* is now idle add up to the running total.
-*/
- last = ktime_sub(ktime_get(), engine->stats.start);
-
- engine->stats.total = ktime_add(engine->stats.total,
- last);
- } else if (engine->stats.active == 0) {
- /*
-* After turning on engine stats, context out might be
-* the first event in which case we account from the
-* time stats gathering was turned on.
-*/
- last = ktime_sub(ktime_get(), engine->stats.enabled_at);
-
- engine->stats.total = ktime_add(engine->stats.total,
- last);
- }
- }
-
- write_sequnlock_irqrestore(&engine->stats.lock, flags);
-}
-
int intel_enable_engine_stats(struct intel_engine_cs *engine);
void intel_disable_engine_stats(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e18623def282..170e394206ca 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -453,6 +453,63 @@ execlists_context_status_change(struct i915_request *rq,
unsigned long status)
status, rq);
}
+static inline void
+intel_engine_context_in(struct intel_engine_cs *engine)
+{
+ unsigned long flags;
+
+ if (READ_ONCE(engine->stats.enabled) == 0)
+ return;
+
+ write_seqlock_irqsave(&engine->stats.lock, flags);
+
+ if (engine->stats.enabled > 0) {
+ if (engine->stats.active++ == 0)
+ engine->stats.start = ktime_get();
+ GEM_BUG_ON(engine->stats.active == 0);
+ }
+
+ write_sequnlock_irqrestore(&engine->stats.lock, flags);
+}
+
+static inline void
+intel_engine_context_out(struct intel_engine_cs *engine)
+{
+ unsigned long flags;
+
+ if (READ_ONCE(engine->stats.enabled) == 0)
+ return;
+
+ write_seqlock_irqsave(&engine->stats.lock, flags);
+
+ if (engine->stats.enabled > 0) {
+ ktime_t last;
+
+ if (engine->stats.active && --engine->stats.active == 0) {
+ /*
+* Decrement the active context count and in case GPU
+* is now idle add up to the running total.
+*/
+ last = ktime_sub(ktime_get(), engine->stats.start);
+
+ engine->stats.total = ktime_add(engine->stats.total,
+ last);
+ } else if (engine->stats.active == 0) {
+ /*
+* After turning on engine stats, context out might be
+* the first event in which case we account from the
+* time stats gathering was turned on.
+*/
+ last =
[Intel-gfx] [RFC 2/6] drm/i915: Track per-context engine busyness
From: Tvrtko Ursulin
Some customers want to know how much of the GPU time are their clients
using in order to make dynamic load balancing decisions.
With the hooks already in place which track the overall engine busyness,
we can extend that slightly to split that time between contexts.
v2: Fix accounting for tail updates.
v3: Rebase.
v4: Mark currently running contexts as active on stats enable.
v5: Include some headers to fix the build.
v6: Added fine grained lock.
v7: Convert to seqlock. (Chris Wilson)
v8: Rebase and tidy with helpers.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/intel_context.c | 21 +++
drivers/gpu/drm/i915/gt/intel_context.h | 9 +++
drivers/gpu/drm/i915/gt/intel_context_types.h | 9 +++
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 +++
drivers/gpu/drm/i915/gt/intel_lrc.c | 62 ---
5 files changed, 99 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index 5b31e1e05ddd..9adf63ff02e0 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -125,6 +125,8 @@ intel_context_init(struct intel_context *ce,
i915_active_request_init(&ce->active_tracker,
NULL, intel_context_retire);
+
+ seqlock_init(&ce->stats.lock);
}
static void i915_global_context_shrink(void)
@@ -177,3 +179,22 @@ struct i915_request *intel_context_create_request(struct
intel_context *ce)
return rq;
}
+
+ktime_t intel_context_get_busy_time(struct intel_context *ce)
+{
+ unsigned int seq;
+ ktime_t total;
+
+ do {
+ seq = read_seqbegin(&ce->stats.lock);
+
+ total = ce->stats.total;
+
+ if (ce->stats.active)
+ total = ktime_add(total,
+ ktime_sub(ktime_get(),
+ ce->stats.start));
+ } while (read_seqretry(&ce->stats.lock, seq));
+
+ return total;
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h
b/drivers/gpu/drm/i915/gt/intel_context.h
index 63392c88cd98..657dcdce7152 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -127,4 +127,13 @@ static inline void intel_context_timeline_unlock(struct
intel_context *ce)
struct i915_request *intel_context_create_request(struct intel_context *ce);
+static inline void
+__intel_context_stats_start(struct intel_context_stats *stats, ktime_t now)
+{
+ stats->start = now;
+ stats->active = true;
+}
+
+ktime_t intel_context_get_busy_time(struct intel_context *ce);
+
#endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 963a312430e6..b33770f396e2 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "i915_active_types.h"
#include "intel_engine_types.h"
@@ -65,6 +66,14 @@ struct intel_context {
/** sseu: Control eu/slice partitioning */
struct intel_sseu sseu;
+
+ /** stats: Context GPU engine busyness tracking. */
+ struct intel_context_stats {
+ seqlock_t lock;
+ bool active;
+ ktime_t start;
+ ktime_t total;
+ } stats;
};
#endif /* __INTEL_CONTEXT_TYPES__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 4c3753c1b573..c97269e1beb3 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1559,6 +1559,14 @@ int intel_enable_engine_stats(struct intel_engine_cs
*engine)
engine->stats.enabled_at = ktime_get();
+ /* Mark currently running context as active. */
+ if (port_isset(port)) {
+ struct i915_request *rq = port_request(port);
+
+ __intel_context_stats_start(&rq->hw_context->stats,
+ engine->stats.enabled_at);
+ }
+
/* XXX submission method oblivious? */
while (num_ports-- && port_isset(port)) {
engine->stats.active++;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 170e394206ca..3a96bddf9474 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -454,18 +454,48 @@ execlists_context_status_change(struct i915_request *rq,
unsigned long status)
}
static inline void
-intel_engine_context_in(struct intel_engine_cs *engine)
+intel_context_stats_start(struct intel_context_stats *stats, ktime_t now)
{
+ write_seqlock(&stats->lock);
+ __intel_context_stats_start
[Intel-gfx] [RFC i-g-t 0/1] intel_gpu_top: Per-client engine busyness
From: Tvrtko Ursulin Extension to intel_gpu_top which uses the corresponding sysfs interface proposal to implement per client and per class engine utilization view. Example output: == intel-gpu-top - 935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s IMC reads: 1401 MiB/s IMC writes:4 MiB/s ENGINE BUSY MI_SEMA MI_WAIT Render/3D/0 63.73% |███ | 3% 0% Blitter/09.53% |██▊ | 6% 0% Video/0 39.32% |███▊ | 16% 0% Video/1 15.62% |▋ | 0% 0% VideoEnhance/00.00% | | 0% 0% PIDNAME RCS BCS VCS VECS 4084gem_wsim |█▌ ||█ || || | 4086gem_wsim |█▌ || ||███|| | == Tvrtko Ursulin (1): intel-gpu-top: Support for client stats tools/intel_gpu_top.c | 590 +- 1 file changed, 584 insertions(+), 6 deletions(-) -- 2.19.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 5/6] drm/i915: Expose per-engine client busyness
From: Tvrtko Ursulin
Expose per-client and per-engine busyness under the previously added sysfs
client root.
The new files are one per-engine instance and located under the 'busy'
directory. Each contains a monotonically increasing nano-second resolution
times each client's jobs were executing on the GPU.
This enables userspace to create a top-like tool for GPU utilization:
==
intel-gpu-top - 935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s
IMC reads: 1401 MiB/s
IMC writes:4 MiB/s
ENGINE BUSY MI_SEMA MI_WAIT
Render/3D/0 63.73% |███ | 3% 0%
Blitter/09.53% |██▊ | 6% 0%
Video/0 39.32% |███▊ | 16% 0%
Video/1 15.62% |▋ | 0% 0%
VideoEnhance/00.00% | | 0% 0%
PIDNAME RCS BCS VCS VECS
4084gem_wsim |█▌ ||█ || || |
4086gem_wsim |█▌ || ||███|| |
==
v2: Use intel_context_engine_get_busy_time.
v3: New directory structure.
v4: Rebase.
v5: sysfs_attr_init.
v6: Small tidy in i915_gem_add_client.
v7: Rebase to be engine class based.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_drv.h | 8 +++
drivers/gpu/drm/i915/i915_gem.c | 102 ++--
2 files changed, 106 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b6fa3d45ed22..e663924dfc78 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -204,6 +204,12 @@ struct drm_i915_private;
struct i915_mm_struct;
struct i915_mmu_object;
+struct i915_engine_busy_attribute {
+ struct device_attribute attr;
+ struct drm_i915_file_private *file_priv;
+ unsigned int engine_class;
+};
+
struct drm_i915_file_private {
struct drm_i915_private *dev_priv;
struct drm_file *file;
@@ -250,10 +256,12 @@ struct drm_i915_file_private {
char *name;
struct kobject *root;
+ struct kobject *busy_root;
struct {
struct device_attribute pid;
struct device_attribute name;
+ struct i915_engine_busy_attribute
busy[MAX_ENGINE_CLASS];
} attr;
} client;
};
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 03cac6ce0c31..adeba5c1d343 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4834,15 +4834,67 @@ show_client_pid(struct device *kdev, struct
device_attribute *attr, char *buf)
return snprintf(buf, PAGE_SIZE, "%u", file_priv->client.pid);
}
+struct busy_ctx {
+ unsigned int engine_class;
+ u64 total;
+};
+
+static int busy_add(int id, void *p, void *data)
+{
+ struct busy_ctx *bc = data;
+ struct i915_gem_context *ctx = p;
+ unsigned int engine_class = bc->engine_class;
+ struct i915_gem_engines_iter it;
+ struct intel_context *ce;
+ uint64_t total = bc->total;
+
+ for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
+ if (ce->engine->uabi_class == engine_class)
+ total += ktime_to_ns(intel_context_get_busy_time(ce));
+ }
+ i915_gem_context_unlock_engines(ctx);
+
+ bc->total = total;
+
+ return 0;
+}
+
+static ssize_t
+show_client_busy(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct i915_engine_busy_attribute *i915_attr =
+ container_of(attr, typeof(*i915_attr), attr);
+ struct drm_i915_file_private *file_priv = i915_attr->file_priv;
+ struct busy_ctx bc = { .engine_class = i915_attr->engine_class };
+ int ret;
+
+ ret = mutex_lock_interruptible(&file_priv->context_idr_lock);
+ if (ret)
+ return ret;
+
+ idr_for_each(&file_priv->context_idr, busy_add, &bc);
+
+ mutex_unlock(&file_priv->context_idr_lock);
+
+ return snprintf(buf, PAGE_SIZE, "%llu\n", bc.total);
+}
+
+static const char *uabi_class_names[] = {
+ [I915_ENGINE_CLASS_RENDER] = "0",
+ [I915_ENGINE_CLASS_COPY] = "1",
+ [I915_ENGINE_CLASS_VIDEO] = "2",
+ [COPY_ENGINE_CLASS] = "3",
+};
+
int
i915_gem_add_client(struct drm_i915_private *i915,
struct drm_i915_file_private *file_priv,
struct task_struct *task,
unsigned int serial)
{
- int ret = -ENOMEM;
+ int i, ret = -ENOMEM;
struct device_attribute *attr;
- char id[32];
+
[Intel-gfx] [RFC 3/6] drm/i915: Expose list of clients in sysfs
From: Tvrtko Ursulin
Expose a list of clients with open file handles in sysfs.
This will be a basis for a top-like utility showing per-client and per-
engine GPU load.
Currently we only expose each client's pid and name under opaque numbered
directories in /sys/class/drm/card0/clients/.
For instance:
/sys/class/drm/card0/clients/3/name: Xorg
/sys/class/drm/card0/clients/3/pid: 5664
v2:
Chris Wilson:
* Enclose new members into dedicated structs.
* Protect against failed sysfs registration.
v3:
* sysfs_attr_init.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_drv.h | 19 +
drivers/gpu/drm/i915/i915_gem.c | 121 --
drivers/gpu/drm/i915/i915_sysfs.c | 8 ++
3 files changed, 140 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d0257808734c..1d7a94555a87 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -242,6 +242,20 @@ struct drm_i915_file_private {
/** ban_score: Accumulated score of all ctx bans and fast hangs. */
atomic_t ban_score;
unsigned long hang_timestamp;
+
+ struct i915_drm_client {
+ unsigned int id;
+
+ pid_t pid;
+ char *name;
+
+ struct kobject *root;
+
+ struct {
+ struct device_attribute pid;
+ struct device_attribute name;
+ } attr;
+ } client;
};
/* Interface history:
@@ -2057,6 +2071,11 @@ struct drm_i915_private {
struct i915_pmu pmu;
+ struct i915_drm_clients {
+ struct kobject *root;
+ atomic_t serial;
+ } clients;
+
struct i915_hdcp_comp_master *hdcp_master;
bool hdcp_comp_added;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2fcec1bfb038..0ae5764a58d2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4814,6 +4814,96 @@ int i915_gem_freeze_late(struct drm_i915_private *i915)
return 0;
}
+static ssize_t
+show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct drm_i915_file_private *file_priv =
+ container_of(attr, struct drm_i915_file_private,
+client.attr.name);
+
+ return snprintf(buf, PAGE_SIZE, "%s", file_priv->client.name);
+}
+
+static ssize_t
+show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+ struct drm_i915_file_private *file_priv =
+ container_of(attr, struct drm_i915_file_private,
+client.attr.pid);
+
+ return snprintf(buf, PAGE_SIZE, "%u", file_priv->client.pid);
+}
+
+static int
+i915_gem_add_client(struct drm_i915_private *i915,
+ struct drm_i915_file_private *file_priv,
+ struct task_struct *task,
+ unsigned int serial)
+{
+ int ret = -ENOMEM;
+ struct device_attribute *attr;
+ char id[32];
+
+ if (!i915->clients.root)
+ goto err_name;
+
+ file_priv->client.name = kstrdup(task->comm, GFP_KERNEL);
+ if (!file_priv->client.name)
+ goto err_name;
+
+ snprintf(id, sizeof(id), "%u", serial);
+ file_priv->client.root = kobject_create_and_add(id,
+ i915->clients.root);
+ if (!file_priv->client.root)
+ goto err_client;
+
+ attr = &file_priv->client.attr.name;
+ sysfs_attr_init(&attr->attr);
+ attr->attr.name = "name";
+ attr->attr.mode = 0444;
+ attr->show = show_client_name;
+
+ ret = sysfs_create_file(file_priv->client.root,
+ (struct attribute *)attr);
+ if (ret)
+ goto err_attr_name;
+
+ attr = &file_priv->client.attr.pid;
+ sysfs_attr_init(&attr->attr);
+ attr->attr.name = "pid";
+ attr->attr.mode = 0444;
+ attr->show = show_client_pid;
+
+ ret = sysfs_create_file(file_priv->client.root,
+ (struct attribute *)attr);
+ if (ret)
+ goto err_attr_pid;
+
+ file_priv->client.pid = pid_nr(get_task_pid(task, PIDTYPE_PID));
+
+ return 0;
+
+err_attr_pid:
+ sysfs_remove_file(file_priv->client.root,
+ (struct attribute *)&file_priv->client.attr.name);
+err_attr_name:
+ kobject_put(file_priv->client.root);
+err_client:
+ kfree(file_priv->client.name);
+err_name:
+ return ret;
+}
+
+static void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
+{
+ sysfs_remove_file(file_priv->client.root,
+ (struct attribute *)&file_priv->client.attr.pid);
+ sysfs_remove_file(file_priv->client.root,
+ (struct attribute *)&file_priv->client.attr.name);
+
[Intel-gfx] [RFC 0/6] Per context and per client GPU busyness tracking
From: Tvrtko Ursulin Continuation of the old feature work, rebased to latest drm-tip and adapted to expose data per engine class - making it compatible with the upcoming Virtual Engine uapi. Patch series adds core per context (intel_context) engine busyness tracking and then also exports this data in sysfs, aggregated to per client and per engine class. Exported directory structure looks like this: root@sc:/sys/class/drm/card0# grep . -r clients/ clients/21/busy/3:0 clients/21/busy/1:0 clients/21/busy/2:24712242825 clients/21/busy/0:7830006484 clients/21/pid:5989 clients/21/name:gem_wsim clients/22/busy/3:0 clients/22/busy/1:2778102816 clients/22/busy/2:0 clients/22/busy/0:14434594580 clients/22/pid:5991 clients/22/name:gem_wsim clients/enable_stats:1 Each client has an opaque unique id named directory, under which are its name, pid, and another subdirectory containing accumulated engine busyness per engine class in nanoseconds. Example output from extended intel_gpu_top: == intel-gpu-top - 935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s IMC reads: 1401 MiB/s IMC writes:4 MiB/s ENGINE BUSY MI_SEMA MI_WAIT Render/3D/0 63.73% |███ | 3% 0% Blitter/09.53% |██▊ | 6% 0% Video/0 39.32% |███▊ | 16% 0% Video/1 15.62% |▋ | 0% 0% VideoEnhance/00.00% | | 0% 0% PIDNAME RCS BCS VCS VECS 4084gem_wsim |█▌ ||█ || || | 4086gem_wsim |█▌ || ||███|| | == Tvrtko Ursulin (6): drm/i915: Move intel_engine_context_in/out into intel_lrc.c drm/i915: Track per-context engine busyness drm/i915: Expose list of clients in sysfs drm/i915: Update client name on context create drm/i915: Expose per-engine client busyness drm/i915: Add sysfs toggle to enable per-client engine stats drivers/gpu/drm/i915/gt/intel_context.c | 21 ++ drivers/gpu/drm/i915/gt/intel_context.h | 9 + drivers/gpu/drm/i915/gt/intel_context_types.h | 9 + drivers/gpu/drm/i915/gt/intel_engine.h| 55 - drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 + drivers/gpu/drm/i915/gt/intel_lrc.c | 107 - drivers/gpu/drm/i915/i915_drv.h | 39 drivers/gpu/drm/i915/i915_gem.c | 215 +- drivers/gpu/drm/i915/i915_gem_context.c | 18 +- drivers/gpu/drm/i915/i915_sysfs.c | 81 +++ 10 files changed, 492 insertions(+), 70 deletions(-) -- 2.19.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 4/6] drm/i915: Update client name on context create
From: Tvrtko Ursulin
Some clients have the DRM fd passed to them over a socket by the X server.
Grab the real client and pid when they create their first context and
update the exposed data for more useful enumeration.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_drv.h | 8
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 18 +++---
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1d7a94555a87..b6fa3d45ed22 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3086,6 +3086,14 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
void i915_gem_object_unpin_from_display_plane(struct i915_vma *vma);
int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align);
+
+int
+i915_gem_add_client(struct drm_i915_private *i915,
+ struct drm_i915_file_private *file_priv,
+ struct task_struct *task,
+ unsigned int serial);
+void i915_gem_remove_client(struct drm_i915_file_private *file_priv);
+
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0ae5764a58d2..03cac6ce0c31 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4834,7 +4834,7 @@ show_client_pid(struct device *kdev, struct
device_attribute *attr, char *buf)
return snprintf(buf, PAGE_SIZE, "%u", file_priv->client.pid);
}
-static int
+int
i915_gem_add_client(struct drm_i915_private *i915,
struct drm_i915_file_private *file_priv,
struct task_struct *task,
@@ -4894,7 +4894,7 @@ i915_gem_add_client(struct drm_i915_private *i915,
return ret;
}
-static void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
+void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
{
sysfs_remove_file(file_priv->client.root,
(struct attribute *)&file_priv->client.attr.pid);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 65cefc520e79..1ac2521fc448 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1499,6 +1499,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev,
void *data,
{
struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_context_create_ext *args = data;
+ pid_t pid = pid_nr(get_task_pid(current, PIDTYPE_PID));
+ struct drm_i915_file_private *file_priv = file->driver_priv;
struct create_ext ext_data;
int ret;
@@ -1512,11 +1514,11 @@ int i915_gem_context_create_ioctl(struct drm_device
*dev, void *data,
if (ret)
return ret;
- ext_data.fpriv = file->driver_priv;
+ ext_data.fpriv = file_priv;
if (client_is_banned(ext_data.fpriv)) {
DRM_DEBUG("client %s[%d] banned from creating ctx\n",
- current->comm,
- pid_nr(get_task_pid(current, PIDTYPE_PID)));
+ current->comm, pid);
+
return -EIO;
}
@@ -1524,6 +1526,16 @@ int i915_gem_context_create_ioctl(struct drm_device
*dev, void *data,
if (ret)
return ret;
+ if (file_priv->client.pid != pid) {
+ i915_gem_remove_client(file_priv);
+ ret = i915_gem_add_client(i915, file_priv, current,
+ file_priv->client.id);
+ if (ret) {
+ mutex_unlock(&dev->struct_mutex);
+ return ret;
+ }
+ }
+
ext_data.ctx = i915_gem_create_context(i915, args->flags);
mutex_unlock(&dev->struct_mutex);
if (IS_ERR(ext_data.ctx))
--
2.19.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 05/21] wsim/media-bench: i915 balancing
Quoting Tvrtko Ursulin (2019-05-08 13:10:42)
> @@ -841,7 +846,11 @@ eb_set_engine(struct drm_i915_gem_execbuffer2 *eb,
> if (engine == VCS2 && (flags & VCS2REMAP))
> engine = BCS;
>
> - eb->flags = eb_engine_map[engine];
> + if ((flags & I915) && engine == VCS) {
> + eb->flags = 0;
> + } else {
> + eb->flags = eb_engine_map[engine];
> + }
You drop these brackets in a later patch.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 1/1] intel-gpu-top: Support for client stats
From: Tvrtko Ursulin
Adds support for per-client engine busyness stats i915 exports in sysfs
and produces output like the below:
==
intel-gpu-top - 935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s
IMC reads: 1401 MiB/s
IMC writes:4 MiB/s
ENGINE BUSY MI_SEMA MI_WAIT
Render/3D/0 63.73% |███ | 3% 0%
Blitter/09.53% |██▊ | 6% 0%
Video/0 39.32% |███▊ | 16% 0%
Video/1 15.62% |▋ | 0% 0%
VideoEnhance/00.00% | | 0% 0%
PIDNAME RCS BCS VCS VECS
4084gem_wsim |█▌ ||█ || || |
4086gem_wsim |█▌ || ||███|| |
==
Apart from the existing physical engine utilization it now also shows
utilization per client and per engine class.
Signed-off-by: Tvrtko Ursulin
---
tools/intel_gpu_top.c | 590 +-
1 file changed, 584 insertions(+), 6 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index cc8db7c539ed..88e1ad52d17c 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -659,8 +659,403 @@ static void pmu_sample(struct engines *engines)
}
}
+enum client_status {
+ FREE = 0, /* mbz */
+ ALIVE,
+ PROBE
+};
+
+struct clients;
+
+struct client {
+ struct clients *clients;
+
+ enum client_status status;
+ unsigned int id;
+ unsigned int pid;
+ char name[128];
+ unsigned int samples;
+ unsigned long total;
+ struct engines *engines;
+ unsigned long *val;
+ uint64_t *last;
+};
+
+struct engine_class {
+ unsigned int class;
+ const char *name;
+ unsigned int num_engines;
+};
+
+struct clients {
+ unsigned int num_classes;
+ struct engine_class *class;
+
+ unsigned int num_clients;
+ struct client *client;
+};
+
+#define for_each_client(clients, c, tmp) \
+ for ((tmp) = (clients)->num_clients, c = (clients)->client; \
+(tmp > 0); (tmp)--, (c)++)
+
+#define SYSFS_ENABLE "/sys/class/drm/card0/clients/enable_stats"
+
+bool __stats_enabled;
+
+static int __set_stats(bool val)
+{
+ int fd, ret;
+
+ fd = open(SYSFS_ENABLE, O_WRONLY);
+ if (fd < 0)
+ return -errno;
+
+ ret = write(fd, val ? "1" : "0", 2);
+ if (ret < 0)
+ return -errno;
+ else if (ret < 2)
+ return 1;
+
+ close(fd);
+
+ return 0;
+}
+
+static void __restore_stats(void)
+{
+ int ret;
+
+ if (__stats_enabled)
+ return;
+
+ ret = __set_stats(false);
+ if (ret)
+ fprintf(stderr, "Failed to disable per-client stats! (%d)\n",
+ ret);
+}
+
+static void __restore_stats_signal(int sig)
+{
+ exit(0);
+}
+
+static int enable_stats(void)
+{
+ int fd, ret;
+
+ fd = open(SYSFS_ENABLE, O_RDONLY);
+ if (fd < 0)
+ return -errno;
+
+ close(fd);
+
+ __stats_enabled = filename_to_u64(SYSFS_ENABLE, 10);
+ if (__stats_enabled)
+ return 0;
+
+ ret = __set_stats(true);
+ if (!ret) {
+ if (atexit(__restore_stats))
+ fprintf(stderr, "Failed to register exit handler!");
+
+ if (signal(SIGINT, __restore_stats_signal))
+ fprintf(stderr, "Failed to register signal handler!");
+ } else {
+ fprintf(stderr, "Failed to enable per-client stats! (%d)\n",
+ ret);
+ }
+
+ return ret;
+}
+
+static struct clients *init_clients(void)
+{
+ struct clients *clients = malloc(sizeof(*clients));
+
+ if (enable_stats()) {
+ free(clients);
+ return NULL;
+ }
+
+ return memset(clients, 0, sizeof(*clients));
+}
+
+#define SYSFS_CLIENTS "/sys/class/drm/card0/clients"
+
+static uint64_t read_client_busy(unsigned int id, unsigned int class)
+{
+ char buf[256];
+ ssize_t ret;
+
+ ret = snprintf(buf, sizeof(buf),
+ SYSFS_CLIENTS "/%u/busy/%u",
+ id, class);
+ assert(ret > 0 && ret < sizeof(buf));
+ if (ret <= 0 || ret == sizeof(buf))
+ return 0;
+
+ return filename_to_u64(buf, 10);
+}
+
+static struct client *
+find_client(struct clients *clients, enum client_status status, unsigned int
id)
+{
+ struct client *c;
+ int tmp;
+
+ for_each_client(clients, c, tmp) {
+ if ((status == FREE && c->status ==
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 09/21] gem_wsim: Submit fence support
Quoting Tvrtko Ursulin (2019-05-08 13:10:46)
> From: Tvrtko Ursulin
>
> Add support for submit fences in a way similar to how normal input fences
> are handled. Eg:
>
> 1.RCS.500-1000.0.0
> 1.VCS1.3000.s-1.0
> 1.VCS2.3000.s-2.0
Looks like commands on a punch card. :-p
> Submit fences are signalled when the originating request enters the
> submission backend.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> benchmarks/gem_wsim.c | 20
> benchmarks/wsim/README | 17 +
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index f1fcef5dcfba..5245692df6eb 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -87,6 +87,7 @@ enum w_type
> struct deps
> {
> int nr;
> + bool submit_fence;
> int *list;
> };
>
> @@ -253,17 +254,23 @@ parse_dependencies(unsigned int nr_steps, struct w_step
> *w, char *_desc)
>w->data_deps.list == w->fence_deps.list);
>
> while ((token = strtok_r(tstart, "/", &tctx)) != NULL) {
> + bool submit_fence = false;
> char *str = token;
> struct deps *deps;
> int dep;
>
> tstart = NULL;
>
> - if (strlen(token) > 1 && token[0] == 'f') {
> + if (str[0] == '-' || (str[0] >= '0' && str[0] <= '9')) {
> + deps = &w->data_deps;
> + } else {
> + if (str[0] == 's')
> + submit_fence = true;
> + else if (str[0] != 'f')
> + return -1;
> +
> deps = &w->fence_deps;
> str++;
> - } else {
> - deps = &w->data_deps;
> }
>
> dep = atoi(str);
> @@ -281,6 +288,7 @@ parse_dependencies(unsigned int nr_steps, struct w_step
> *w, char *_desc)
> sizeof(*deps->list) * deps->nr);
> igt_assert(deps->list);
> deps->list[deps->nr - 1] = dep;
> + deps->submit_fence = submit_fence;
> }
> }
>
> @@ -1921,7 +1929,11 @@ do_eb(struct workload *wrk, struct w_step *w, enum
> intel_engine_id engine,
> igt_assert(tgt >= 0 && tgt < w->idx);
> igt_assert(wrk->steps[tgt].emit_fence > 0);
>
> - w->eb.flags |= I915_EXEC_FENCE_IN;
> + if (w->fence_deps.submit_fence)
> + w->eb.flags |= I915_EXEC_FENCE_SUBMIT;
> + else
> + w->eb.flags |= I915_EXEC_FENCE_IN;
> +
> w->eb.rsvd2 = wrk->steps[tgt].emit_fence;
That looked too easy.
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 11/21] gem_wsim: Engine map support
Quoting Tvrtko Ursulin (2019-05-08 13:10:48)
> From: Tvrtko Ursulin
>
> Support new i915 uAPI for configuring contexts with engine maps.
>
> Please refer to the README file for more detailed explanation.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> +static int parse_engine_map(struct w_step *step, const char *_str)
> +{
> + char *token, *tctx = NULL, *tstart = (char *)_str;
> +
> + while ((token = strtok_r(tstart, "|", &tctx))) {
> + enum intel_engine_id engine;
> +
> + tstart = NULL;
> +
> + if (!strcmp(token, "DEFAULT"))
> + return -1;
> + else if (!strcmp(token, "VCS"))
> + return -1;
> +
> + engine = str_to_engine(token);
> + if ((int)engine < 0)
> + return -1;
> +
> + if (engine != VCS1 && engine != VCS2)
> + return -1; /* TODO */
> +
> + step->engine_map_count++;
> + step->engine_map = realloc(step->engine_map,
> + step->engine_map_count *
> + sizeof(step->engine_map[0]));
> + step->engine_map[step->engine_map_count - 1] = engine;
> + if (ctx->engine_map) {
> + I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> +
> ctx->engine_map_count + 1);
> + struct drm_i915_gem_context_param param = {
> + .ctx_id = ctx_id,
> + .param = I915_CONTEXT_PARAM_ENGINES,
> + .size = sizeof(set_engines),
> + .value = to_user_pointer(&set_engines),
> + };
> +
> + set_engines.extensions = 0;
> +
> + /* Reserve slot for virtual engine. */
> + set_engines.engines[0].engine_class =
> + I915_ENGINE_CLASS_INVALID;
> + set_engines.engines[0].engine_instance =
> + I915_ENGINE_CLASS_INVALID_NONE;
> +
> + for (j = 1; j <= ctx->engine_map_count; j++) {
> + set_engines.engines[j].engine_class =
> + I915_ENGINE_CLASS_VIDEO; /* FIXME */
> + set_engines.engines[j].engine_instance =
> + ctx->engine_map[j - 1] - VCS1; /*
> FIXME */
> + }
I would suggest the file format starts with class:instance specifiers.
Too much FIXME that I think will need a file format change.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 12/21] gem_wsim: Save some lines by changing to implicit NULL checking
Quoting Tvrtko Ursulin (2019-05-08 13:10:49) > From: Tvrtko Ursulin > > We can improve the parsing loop readability a bit more by avoiding some > line breaks caused by explicit NULL checks. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 15/21] gem_wsim: Engine bond command
Quoting Tvrtko Ursulin (2019-05-08 13:10:52)
> From: Tvrtko Ursulin
>
> Engine bonds are an i915 uAPI applicable to load balanced contexts with
> engine map. They allow expression rules of engine selection between two
> contexts when submissions are also tied with submit fences.
>
> Please refer to the README for a more detailed description.
I would prefer not to have a hexadecimal mask in the file format? That's
harder than usual to read later on.
bond({master_class:master_instance}, {engine_class:engine_instance}),...
?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 16/21] gem_wsim: Some more example workloads
Quoting Tvrtko Ursulin (2019-05-08 13:10:53) > From: Tvrtko Ursulin > > A few additional workloads useful for experimenting with scheduling. > > Signed-off-by: Tvrtko Ursulin Acked-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 14/21] gem_wsim: Engine map load balance command
Quoting Tvrtko Ursulin (2019-05-08 13:10:51)
> From: Tvrtko Ursulin
>
> A new workload command for enabling a load balanced context map (aka
> Virtual Engine). Example usage:
>
> B.1
>
> This turns on load balancing for context one, assuming it has already been
> configured with an engine map. Only DEFAULT engine specifier can be used
> with load balanced engine maps.
Restriction makes sense for keeping linenoise^W file format simple.
> Signed-off-by: Tvrtko Ursulin
> ---
> @@ -1172,6 +1210,8 @@ prepare_workload(unsigned int id, struct workload *wrk,
> unsigned int flags)
> if (ctx->engine_map) {
> I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
>
> ctx->engine_map_count + 1);
> + I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> +
> ctx->engine_map_count);
> struct drm_i915_gem_context_param param = {
> .ctx_id = ctx_id,
> .param = I915_CONTEXT_PARAM_ENGINES,
> @@ -1179,7 +1219,25 @@ prepare_workload(unsigned int id, struct workload
> *wrk, unsigned int flags)
> .value = to_user_pointer(&set_engines),
> };
>
> - set_engines.extensions = 0;
> + if (ctx->wants_balance) {
> + set_engines.extensions =
> + to_user_pointer(&load_balance);
> +
> + memset(&load_balance, 0,
> sizeof(load_balance));
> + load_balance.base.name =
> + I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> + load_balance.num_siblings =
> + ctx->engine_map_count;
> +
> + for (j = 0; j < ctx->engine_map_count; j++) {
> + load_balance.engines[j].engine_class =
> + I915_ENGINE_CLASS_VIDEO; /*
> FIXME */
> +
> load_balance.engines[j].engine_instance =
> + ctx->engine_map[j] - VCS1; /*
> FIXME */
Ok, more fallout from fixing ctx->engine_map[] first?
Otherwise looks fine.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 13/21] gem_wsim: Compact int command parsing with a macro
Quoting Tvrtko Ursulin (2019-05-08 13:10:50)
> From: Tvrtko Ursulin
>
> Parsing an integer workload descriptor field is a common pattern which we
> can extract to a helper macro and by doing so further improve the
> readability of the main parsing loop.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> benchmarks/gem_wsim.c | 80 ++-
> 1 file changed, 25 insertions(+), 55 deletions(-)
>
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index 4dbfc3e922a9..c2e13d9939c2 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -370,6 +370,15 @@ static int parse_engine_map(struct w_step *step, const
> char *_str)
> return 0;
> }
>
> +#define int_field(_STEP_, _FIELD_, _COND_, _ERR_) \
> + if ((field = strtok_r(fstart, ".", &fctx))) { \
> + tmp = atoi(field); \
> + check_arg(_COND_, _ERR_, nr_steps); \
> + step.type = _STEP_; \
> + step._FIELD_ = tmp; \
> + goto add_step; \
> + } \
More hidden control flow :-p
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 17/21] gem_wsim: Infinite batch support
Quoting Tvrtko Ursulin (2019-05-08 13:10:54)
> From: Tvrtko Ursulin
>
> For simulating frame split workloads it is useful to express a batch which
> ends at the same time as the parallel submission on the respective bonded
> engine. For this we add support for infinite batch durations and the batch
> terminate command ('T'). Syntax looks like this:
>
> 1.RCS.*.0.0
> T.-1
>
> First step starts an infinite batch, and second command terminates the
> infinite batch with the usual relative workload step addressing.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> benchmarks/gem_wsim.c | 119 +++--
> benchmarks/wsim/README | 9 +-
> benchmarks/wsim/frame-split-60fps.wsim | 6 +-
> 3 files changed, 102 insertions(+), 32 deletions(-)
>
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index cc6f4a742c12..97821b723b02 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -86,6 +86,7 @@ enum w_type
> ENGINE_MAP,
> LOAD_BALANCE,
> BOND,
> + TERMINATE,
> };
>
> struct deps
> @@ -113,6 +114,7 @@ struct w_step
> unsigned int context;
> unsigned int engine;
> struct duration duration;
> + bool unbound_duration;
> struct deps data_deps;
> struct deps fence_deps;
> int emit_fence;
> @@ -143,7 +145,7 @@ struct w_step
>
> struct drm_i915_gem_execbuffer2 eb;
> struct drm_i915_gem_exec_object2 *obj;
> - struct drm_i915_gem_relocation_entry reloc[4];
> + struct drm_i915_gem_relocation_entry reloc[5];
> unsigned long bb_sz;
> uint32_t bb_handle;
> uint32_t *seqno_value;
> @@ -153,6 +155,7 @@ struct w_step
> uint32_t *rt1_address;
> uint32_t *latch_value;
> uint32_t *latch_address;
> + uint32_t *recursive_bb_start;
> };
>
> DECLARE_EWMA(uint64_t, rt, 4, 2)
> @@ -491,6 +494,10 @@ parse_workload(struct w_arg *arg, unsigned int flags,
> struct workload *app_w)
>
> step.type = ENGINE_MAP;
> goto add_step;
> + } else if (!strcmp(field, "T")) {
> + int_field(TERMINATE, target,
> + tmp >= 0 || ((int)nr_steps + tmp) <
> 0,
> + "Invalid terminate target at step
> %u!\n");
> } else if (!strcmp(field, "X")) {
> unsigned int nr = 0;
> while ((field = strtok_r(fstart, ".",
> &fctx))) {
> @@ -605,23 +612,28 @@ parse_workload(struct w_arg *arg, unsigned int flags,
> struct workload *app_w)
>
> fstart = NULL;
>
> - tmpl = strtol(field, &sep, 10);
> - check_arg(tmpl <= 0 || tmpl == LONG_MIN ||
> - tmpl == LONG_MAX,
> - "Invalid duration at step %u!\n", nr_steps);
> - step.duration.min = tmpl;
> -
> - if (sep && *sep == '-') {
> - tmpl = strtol(sep + 1, NULL, 10);
> - check_arg(tmpl <= 0 ||
> - tmpl <= step.duration.min ||
> - tmpl == LONG_MIN ||
> + if (field[0] == '*') {
> + step.unbound_duration = true;
> + } else {
> + tmpl = strtol(field, &sep, 10);
> + check_arg(tmpl <= 0 || tmpl == LONG_MIN ||
> tmpl == LONG_MAX,
> - "Invalid duration range at step
> %u!\n",
> + "Invalid duration at step %u!\n",
> nr_steps);
> - step.duration.max = tmpl;
> - } else {
> - step.duration.max = step.duration.min;
> + step.duration.min = tmpl;
> +
> + if (sep && *sep == '-') {
> + tmpl = strtol(sep + 1, NULL, 10);
> + check_arg(tmpl <= 0 ||
> + tmpl <= step.duration.min ||
> + tmpl == LONG_MIN ||
> + tmpl == LONG_MAX,
> + "Invalid duration range at
> step %u!\n",
> + nr_steps);
> + step.duration.max = tmpl;
> + } else {
> + step.duration.max = step.duration.min;
Re: [Intel-gfx] [RFC 5/6] drm/i915: Expose per-engine client busyness
Quoting Tvrtko Ursulin (2019-05-10 14:22:39)
> +static const char *uabi_class_names[] = {
> + [I915_ENGINE_CLASS_RENDER] = "0",
> + [I915_ENGINE_CLASS_COPY] = "1",
> + [I915_ENGINE_CLASS_VIDEO] = "2",
> + [COPY_ENGINE_CLASS] = "3",
I915_ENGINE_CLASS_VIDEO_ENHANCE?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/icl: More workaround for port F detection due to broken VBTs
Add another ICL-Y PCIID that proved to have only 5 ports to the corresponding PCIID list. Meanwhile I'm trying to get a complete list of all PCIIDs with less than 6 ports and/or get a VBT fix to mark these ports non-existant, but until then the only way is to go one-by-one. This fixes the following error on machines with less than 6 port: [drm:intel_power_well_enable [i915]] enabling AUX F [ cut here ] WARN_ON(intel_wait_for_register(&dev_priv->uncore, regs->driver, (0x1 << ((pw_idx) * 2)), (0x1 << ((pw_idx) * 2)), 1)) (Internal reference: BSpec/Index/20584/Issues, HSD/1306084116) Cc: Mika Kahola References: https://bugs.freedesktop.org/show_bug.cgi?id=108915 Signed-off-by: Imre Deak --- include/drm/i915_pciids.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 6477da22af28..6d60ea68c171 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -559,7 +559,6 @@ #define INTEL_ICL_PORT_F_IDS(info) \ INTEL_VGA_DEVICE(0x8A50, info), \ INTEL_VGA_DEVICE(0x8A5C, info), \ - INTEL_VGA_DEVICE(0x8A5D, info), \ INTEL_VGA_DEVICE(0x8A59, info), \ INTEL_VGA_DEVICE(0x8A58, info), \ INTEL_VGA_DEVICE(0x8A52, info), \ @@ -573,7 +572,8 @@ #define INTEL_ICL_11_IDS(info) \ INTEL_ICL_PORT_F_IDS(info), \ - INTEL_VGA_DEVICE(0x8A51, info) + INTEL_VGA_DEVICE(0x8A51, info), \ + INTEL_VGA_DEVICE(0x8A5D, info) /* EHL */ #define INTEL_EHL_IDS(info) \ -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 5/6] drm/i915: Expose per-engine client busyness
On 10/05/2019 14:57, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-10 14:22:39)
+static const char *uabi_class_names[] = {
+ [I915_ENGINE_CLASS_RENDER] = "0",
+ [I915_ENGINE_CLASS_COPY] = "1",
+ [I915_ENGINE_CLASS_VIDEO] = "2",
+ [COPY_ENGINE_CLASS] = "3",
I915_ENGINE_CLASS_VIDEO_ENHANCE?
Yeah.. probably an interruption in the middle of converting this array. :)
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 3/6] drm/i915: Expose list of clients in sysfs
Quoting Tvrtko Ursulin (2019-05-10 14:22:37) > From: Tvrtko Ursulin > > Expose a list of clients with open file handles in sysfs. > > This will be a basis for a top-like utility showing per-client and per- > engine GPU load. > > Currently we only expose each client's pid and name under opaque numbered > directories in /sys/class/drm/card0/clients/. > > For instance: > > /sys/class/drm/card0/clients/3/name: Xorg > /sys/class/drm/card0/clients/3/pid: 5664 Random thought, should this be a symlink to /proc/$pid or a pidfd? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Per context and per client GPU busyness tracking
== Series Details ==
Series: Per context and per client GPU busyness tracking
URL : https://patchwork.freedesktop.org/series/60506/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
3ae0f10d21d1 drm/i915: Move intel_engine_context_in/out into intel_lrc.c
e76b877d4273 drm/i915: Track per-context engine busyness
83be221853b1 drm/i915: Expose list of clients in sysfs
-:96: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#96: FILE: drivers/gpu/drm/i915/i915_gem.c:4839:
+i915_gem_add_client(struct drm_i915_private *i915,
+ struct drm_i915_file_private *file_priv,
total: 0 errors, 0 warnings, 1 checks, 201 lines checked
4912359f0d14 drm/i915: Update client name on context create
-:24: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#24: FILE: drivers/gpu/drm/i915/i915_drv.h:3092:
+i915_gem_add_client(struct drm_i915_private *i915,
+ struct drm_i915_file_private *file_priv,
total: 0 errors, 0 warnings, 1 checks, 68 lines checked
4c2ec282f33b drm/i915: Expose per-engine client busyness
-:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#25:
Render/3D/0 63.73% |███ | 3% 0%
-:95: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u64' over 'uint64_t'
#95: FILE: drivers/gpu/drm/i915/i915_gem.c:4849:
+ uint64_t total = bc->total;
-:128: WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should
probably be static const char * const
#128: FILE: drivers/gpu/drm/i915/i915_gem.c:4882:
+static const char *uabi_class_names[] = {
-:185: ERROR:CODE_INDENT: code indent should use tabs where possible
#185: FILE: drivers/gpu/drm/i915/i915_gem.c:4955:
+^I^I^I^I(struct attribute *)attr);$
-:185: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#185: FILE: drivers/gpu/drm/i915/i915_gem.c:4955:
+ ret = sysfs_create_file(file_priv->client.busy_root,
+ (struct attribute *)attr);
total: 1 errors, 2 warnings, 2 checks, 161 lines checked
816d25263191 drm/i915: Add sysfs toggle to enable per-client engine stats
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 29/40] drm/i915: Move GEM object waiting to its own file
Chris Wilson writes:
> Continuing the decluttering of i915_gem.c by moving the object wait
> decomposition into its own file.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 8 +
> drivers/gpu/drm/i915/gem/i915_gem_wait.c | 277 +
> drivers/gpu/drm/i915/i915_drv.h| 7 -
> drivers/gpu/drm/i915/i915_gem.c| 254 ---
> drivers/gpu/drm/i915/i915_utils.h | 10 -
> 6 files changed, 286 insertions(+), 271 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_wait.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e5348c355987..a4cc2f7f9bc6 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -105,6 +105,7 @@ gem-y += \
> gem/i915_gem_stolen.o \
> gem/i915_gem_tiling.o \
> gem/i915_gem_userptr.o \
> + gem/i915_gem_wait.o \
> gem/i915_gemfs.o
> i915-y += \
> $(gem-y) \
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 509d145d808a..23bca003fbfb 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -436,4 +436,12 @@ static inline void __start_cpu_write(struct
> drm_i915_gem_object *obj)
> obj->cache_dirty = true;
> }
>
> +int i915_gem_object_wait(struct drm_i915_gem_object *obj,
> + unsigned int flags,
> + long timeout);
> +int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
> + unsigned int flags,
> + const struct i915_sched_attr *attr);
> +#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> new file mode 100644
> index ..fed5c751ef37
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> @@ -0,0 +1,277 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2016 Intel Corporation
> + */
> +
> +#include
> +#include
> +
> +#include "gt/intel_engine.h"
> +
> +#include "i915_gem_ioctls.h"
> +#include "i915_gem_object.h"
> +
> +static long
> +i915_gem_object_wait_fence(struct dma_fence *fence,
> +unsigned int flags,
> +long timeout)
> +{
> + BUILD_BUG_ON(I915_WAIT_INTERRUPTIBLE != 0x1);
> +
> + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> + return timeout;
> +
> + if (dma_fence_is_i915(fence))
> + return i915_request_wait(to_request(fence), flags, timeout);
> +
> + return dma_fence_wait_timeout(fence,
> + flags & I915_WAIT_INTERRUPTIBLE,
> + timeout);
> +}
> +
> +static long
> +i915_gem_object_wait_reservation(struct reservation_object *resv,
> + unsigned int flags,
> + long timeout)
> +{
> + unsigned int seq = __read_seqcount_begin(&resv->seq);
> + struct dma_fence *excl;
> + bool prune_fences = false;
> +
> + if (flags & I915_WAIT_ALL) {
> + struct dma_fence **shared;
> + unsigned int count, i;
> + int ret;
> +
> + ret = reservation_object_get_fences_rcu(resv,
> + &excl, &count, &shared);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < count; i++) {
> + timeout = i915_gem_object_wait_fence(shared[i],
> + flags, timeout);
> + if (timeout < 0)
> + break;
> +
> + dma_fence_put(shared[i]);
> + }
> +
> + for (; i < count; i++)
> + dma_fence_put(shared[i]);
> + kfree(shared);
> +
> + /*
> + * If both shared fences and an exclusive fence exist,
> + * then by construction the shared fences must be later
> + * than the exclusive fence. If we successfully wait for
> + * all the shared fences, we know that the exclusive fence
> + * must all be signaled. If all the shared fences are
> + * signaled, we can prune the array and recover the
> + * floating references on the fences/requests.
> + */
> + prune_fences = count && timeout >= 0;
> + } else {
> + excl = reservation_object_get_excl_rcu(resv);
> + }
> +
> + if (excl && timeout >= 0)
> + timeout = i915_gem_object_wait_fence(excl, flags, timeout);
> +
> + dma_fence_put(excl);
> +
> + /*
> + * Opportunistically
Re: [Intel-gfx] [RFC 2/6] drm/i915: Track per-context engine busyness
Quoting Tvrtko Ursulin (2019-05-10 14:22:36)
> +static inline void
> +intel_context_in(struct intel_context *ce, bool submit)
> +{
> + struct intel_engine_cs *engine = ce->engine;
> unsigned long flags;
> + ktime_t now;
>
> if (READ_ONCE(engine->stats.enabled) == 0)
> return;
>
> write_seqlock_irqsave(&engine->stats.lock, flags);
>
> + if (submit) {
> + now = ktime_get();
> + intel_context_stats_start(&ce->stats, now);
> + } else {
> + now = 0;
> + }
> +
> if (engine->stats.enabled > 0) {
> - if (engine->stats.active++ == 0)
> - engine->stats.start = ktime_get();
> + if (engine->stats.active++ == 0) {
> + if (!now)
> + now = ktime_get();
> + engine->stats.start = now;
> + }
> GEM_BUG_ON(engine->stats.active == 0);
> }
>
> @@ -473,8 +503,9 @@ intel_engine_context_in(struct intel_engine_cs *engine)
> }
>
> static inline void
> -intel_engine_context_out(struct intel_engine_cs *engine)
> +intel_context_out(struct intel_context *ce)
> {
> + struct intel_engine_cs *engine = ce->engine;
ce->bound/ce->inflight at this point.
> unsigned long flags;
>
> if (READ_ONCE(engine->stats.enabled) == 0)
> @@ -483,14 +514,25 @@ intel_engine_context_out(struct intel_engine_cs *engine)
> write_seqlock_irqsave(&engine->stats.lock, flags);
>
> if (engine->stats.enabled > 0) {
> + struct execlist_port *next_port = &engine->execlists.port[1];
> + ktime_t now = ktime_get();
> ktime_t last;
>
> + intel_context_stats_stop(&ce->stats, now);
> +
> + if (port_isset(next_port)) {
> + struct i915_request *next_rq =
> port_request(next_port);
> +
> + intel_context_stats_start(&next_rq->hw_context->stats,
> + now);
Oh crikey. Yeah, I am thinking we need to bite bullet and add the explicit
tracking to ELSP promotion. As luck would happen...
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 30/40] drm/i915: Move GEM object busy checking to its own file
Chris Wilson writes: > Continuing the decluttering of i915_gem.c by moving the object busy > checking into its own file. > > Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 29/40] drm/i915: Move GEM object waiting to its own file
Quoting Mika Kuoppala (2019-05-10 15:17:12)
> > diff --git a/drivers/gpu/drm/i915/i915_utils.h
> > b/drivers/gpu/drm/i915/i915_utils.h
> > index edfc69acdaac..9911f53382a5 100644
> > --- a/drivers/gpu/drm/i915/i915_utils.h
> > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > @@ -218,16 +218,6 @@ static inline unsigned long
> > msecs_to_jiffies_timeout(const unsigned int m)
> > return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> > }
> >
> > -static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > -{
> > - /* nsecs_to_jiffies64() does not guard against overflow */
> > - if (NSEC_PER_SEC % HZ &&
> > - div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
> > - return MAX_JIFFY_OFFSET;
> > -
> > -return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > -}
>
> Seems that the wait stuff was only user so jiffiying the timeout. Just looks
> generic for other usage too.
But not generic enough to be in jiffies.h :)
I didn't want to clutter a common header with a one-off. But it can be
moved around on a whim, so not too concerned if this is not its final
resting place.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/16] lib, treewide: add new match_string() helper/macro
On Fri, May 10, 2019 at 09:15:27AM +, Ardelean, Alexandru wrote: > On Wed, 2019-05-08 at 16:22 +0300, Alexandru Ardelean wrote: > > On Wed, 2019-05-08 at 15:18 +0200, Greg KH wrote: > > > On Wed, May 08, 2019 at 04:11:28PM +0300, Andy Shevchenko wrote: > > > > On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote: > > > > Can you split include/linux/ change from the rest? > > > > > > That would break the build, why do you want it split out? This makes > > > sense all as a single patch to me. > > > > > > > Not really. > > It would be just be the new match_string() helper/macro in a new commit. > > And the conversions of the simple users of match_string() (the ones using > > ARRAY_SIZE()) in another commit. > > > > I should have asked in my previous reply. > Leave this as-is or re-formulate in 2 patches ? Depends on on what you would like to spend your time: collecting Acks for all pieces in treewide patch or send new API first followed up by per driver / module update in next cycle. I also have no strong preference. And I think it's good to add Heikki Krogerus to Cc list for both patch series, since he is the author of sysfs variant and may have something to comment on the rest. -- With Best Regards, Andy Shevchenko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: More workaround for port F detection due to broken VBTs
== Series Details == Series: drm/i915/icl: More workaround for port F detection due to broken VBTs URL : https://patchwork.freedesktop.org/series/60508/ State : warning == Summary == $ dim checkpatch origin/drm-tip 28212f668a27 drm/i915/icl: More workaround for port F detection due to broken VBTs -:11: WARNING:TYPO_SPELLING: 'existant' may be misspelled - perhaps 'existent'? #11: 6 ports and/or get a VBT fix to mark these ports non-existant, but until -:18: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #18: WARN_ON(intel_wait_for_register(&dev_priv->uncore, regs->driver, (0x1 << ((pw_idx) * 2)), (0x1 << ((pw_idx) * 2)), 1)) total: 0 errors, 2 warnings, 0 checks, 16 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 31/40] drm/i915: Move GEM client throttling to its own file
Chris Wilson writes: > Continuing the decluttering of i915_gem.c by moving the client self > throttling into its own file. > > Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 34/40] drm/i915: Rename intel_context.active to .inflight
Chris Wilson writes:
> Rename the engine this HW context is currently active upon (that we are
> flying upon) to disambiguate between the mixture of different active
> terms (and prevent conflict in future patches).
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/gt/intel_context_types.h | 2 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c | 22 +--
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
> b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index 963a312430e6..825fcf0ac9c4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -37,7 +37,7 @@ struct intel_context {
>
> struct i915_gem_context *gem_context;
> struct intel_engine_cs *engine;
> - struct intel_engine_cs *active;
> + struct intel_engine_cs *inflight;
Active_on came to my mind when first reading this. As discussed
in irc 'active' is already quite overloaded so inflight seems
fitting.
Reviewed-by: Mika Kuoppala
>
> struct list_head signal_link;
> struct list_head signals;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 64bd25a9e6f5..5e418bf46c46 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -460,7 +460,7 @@ __unwind_incomplete_requests(struct intel_engine_cs
> *engine, int boost)
> __i915_request_unsubmit(rq);
> unwind_wa_tail(rq);
>
> - GEM_BUG_ON(rq->hw_context->active);
> + GEM_BUG_ON(rq->hw_context->inflight);
>
> /*
>* Push the request back into the queue for later resubmission.
> @@ -557,11 +557,11 @@ execlists_user_end(struct intel_engine_execlists
> *execlists)
> static inline void
> execlists_context_schedule_in(struct i915_request *rq)
> {
> - GEM_BUG_ON(rq->hw_context->active);
> + GEM_BUG_ON(rq->hw_context->inflight);
>
> execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
> intel_engine_context_in(rq->engine);
> - rq->hw_context->active = rq->engine;
> + rq->hw_context->inflight = rq->engine;
> }
>
> static void kick_siblings(struct i915_request *rq)
> @@ -576,7 +576,7 @@ static void kick_siblings(struct i915_request *rq)
> static inline void
> execlists_context_schedule_out(struct i915_request *rq, unsigned long status)
> {
> - rq->hw_context->active = NULL;
> + rq->hw_context->inflight = NULL;
> intel_engine_context_out(rq->engine);
> execlists_context_status_change(rq, status);
> trace_i915_request_out(rq);
> @@ -820,7 +820,7 @@ static bool virtual_matches(const struct virtual_engine
> *ve,
> const struct i915_request *rq,
> const struct intel_engine_cs *engine)
> {
> - const struct intel_engine_cs *active;
> + const struct intel_engine_cs *inflight;
>
> if (!(rq->execution_mask & engine->mask)) /* We peeked too soon! */
> return false;
> @@ -834,8 +834,8 @@ static bool virtual_matches(const struct virtual_engine
> *ve,
>* we reuse the register offsets). This is a very small
>* hystersis on the greedy seelction algorithm.
>*/
> - active = READ_ONCE(ve->context.active);
> - if (active && active != engine)
> + inflight = READ_ONCE(ve->context.inflight);
> + if (inflight && inflight != engine)
> return false;
>
> return true;
> @@ -1023,7 +1023,7 @@ static void execlists_dequeue(struct intel_engine_cs
> *engine)
> u32 *regs = ve->context.lrc_reg_state;
> unsigned int n;
>
> - GEM_BUG_ON(READ_ONCE(ve->context.active));
> + GEM_BUG_ON(READ_ONCE(ve->context.inflight));
> virtual_update_register_offsets(regs, engine);
>
> if (!list_empty(&ve->context.signals))
> @@ -1501,7 +1501,7 @@ static void execlists_context_unpin(struct
> intel_context *ce)
>* had the chance to run yet; let it run before we teardown the
>* reference it may use.
>*/
> - engine = READ_ONCE(ce->active);
> + engine = READ_ONCE(ce->inflight);
> if (unlikely(engine)) {
> unsigned long flags;
>
> @@ -1509,7 +1509,7 @@ static void execlists_context_unpin(struct
> intel_context *ce)
> process_csb(engine);
> spin_unlock_irqrestore(&engine->timeline.lock, flags);
>
> - GEM_BUG_ON(READ_ONCE(ce->active));
> + GEM_BUG_ON(READ_ONCE(ce->inflight));
> }
>
> i915_gem_context_unpin_hw_id(ce->gem_context);
> @@ -3103,7 +3103,7 @@ static void virtual_context_destroy(struct kref *kref)
> unsigned int n;
>
> GEM_BUG_ON(ve->request);
> - GEM_BUG_ON
Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
On Fri, May 10, 2019 at 2:12 PM Paul Kocialkowski wrote: > > Hi, > > On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote: > > DRM API for generating uevent for a status changes of connector's > > property. > > > > This uevent will have following details related to the status change: > > > > HOTPLUG=1, CONNECTOR= and PROPERTY= > > > > Need ACK from this uevent from userspace consumer. > > So we just had some discussions over on IRC and at about the hotplug > issue and came up with similar ideas: > https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html > > The conclusions of these discussions so far would be to have a more or > less fine grain of uevent reporting depending on what happened. The > point is that we need to cover different cases: > - one or more properties changed; > - the connector status changed; > - something else about the connector changed (e.g. EDID/modes) > > For the first case, we can send out: > HOTPLUG=1 > CONNECTOR= > PROPERTY= > > and no reprobe is required. > > For the second one, something like: > HOTPLUG=1 > CONNECTOR= > STATUS=Connected/Disconnected > > and a connector probe is needed for connected, but not for > disconnected; > > For the third one, we can only indicate the connector: > HOTPLUG=1 > CONNECTOR= > > and a reprobe of the connector is always needed There's no material difference between this one and the previous one. Plus there's no beenfit in supplying the actual value of the property, i.e. we can reuse the same PROPERTY= trick. Here's why: - A side effect of forcing a probe on a connector is that you get to read all the properties, so supplying them is kinda pointless. - You can read STATUS without forcing a reprobe, if you want to avoid the reprobe for disconnected. I'd kinda not recommend that though, feels a bit like overoptimizing. And for reasonable connectors (i.e. dp) reprobing a disconnected output is fast. HDMI is ... less reasonable unfortunately, but oh well. - There's no way to only reprobe status, you can only ever reprobe everything with the current ioctl and implementations. Having an option to reprobe only parts of it doesn't seem useful to me (we need to read the EDID anyway, and that's the expensive part of reprobing in almost all cases). In a way PROPERTY= simply tells userspace that it needs to reprobe this connector. At that point we need to figure out whether this is a good uapi or not, and that's where the epoch comes in. There's two reasons for an epoch: - We need it internally because I'm not goinig to wire a new return value through hundreds of connector probe functions. It's much easier to have an epoch counter which we set from e.g. drm_set_edid and similar functions that update probe state. - If userspace misses an event and there's no epoch, we're forcing userspace to reprobe everything. Use case would be if a compositor is switched away we probably don't want to piss of the current compositor by blocking it's own probe kernel calls by doing our own (probe is single-threaded in the kernel through the dev->mode_config.mutex). If it can read the epoch property (which it can do without forcing a reprobe) userspace would know which connectors it needs to check and reprobe. Hence why epoch, it's a bit more robust userspace api. Ofc you could also require that userspace needs to keep parsing all uevents and make a list of all connectors it needs to reprobe when it's back to being the active compositor. But just comparing a current epoch with the one you cached from the last full probe is much easier. Another thing: None of this we can for connectors with unreliable hdp. Or at least you'll piss of users if you cache always. The sad thing is that HDMI is unreliable, at least on some machines/screen combos (you never get a hpd irq if you plug in/unplug). So real compositors still need to reprobe when the user asks for it. igt can probably get away without reprobing. -Daniel > Then we still have the legacy case: > HOTPLUG=1 > > where userspace is expected to reprobe all the connectors. > > I think this would deserve to be a separate series on its own. So I am > proposing to take this one off your plate and come up with another > seres implementing this proposal. What do you think? > > Cheers, > > Paul > > > v2: > > Minor fixes at KDoc comments [Daniel] > > v3: > > Check the property is really attached with connector [Daniel] > > > > Signed-off-by: Ramalingam C > > Reviewed-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_sysfs.c | 35 +++ > > include/drm/drm_sysfs.h | 5 - > > 2 files changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > > index 18b1ac442997..63fa951a20db 100644 > > --- a/drivers/gpu/drm/drm_sysfs.c > > +++ b/drivers/gpu/drm/drm_sysfs.c > > @@ -21,6 +21,7 @@ > > #include > > #include > > #include "drm_internal.h" > > +#include "drm_crtc_internal.h" > > > > #define to_drm_minor(d) de
Re: [Intel-gfx] [PATCH] RFC: console: hack up console_lock more v3
On Fri, May 10, 2019 at 11:15 AM Petr Mladek wrote:
> On Thu 2019-05-09 18:43:12, Daniel Vetter wrote:
> > One thing to keep in mind is that the kernel is already dying, and
> > things will come crashing down later on
>
> This is important information. I havn't seen it mentioned earlier.
I thought I explained that, sorry if this wasn't clear.
> > (I've seen this only in dmesg
> > tails capture in pstore in our CI, i.e. the box died for good). I just
> > want to make sure that the useful information isn't overwritten by
> > more dmesg splats that happen as a consequence of us somehow trying to
> > run things on an offline cpu. Once console_unlock has completed in
> > your above backtrace and the important stuff has gone out I'm totally
> > fine with the kernel just dying. Pulling the wake_up_process out from
> > under the semaphore.lock is enough to prevent lockdep from dumping
> > more stuff while we're trying to print the important things,
>
> With the more stuff you mean the lockdep splat? If yes, it might
> make sense to call debug_locks_off() earlier in panic().
I'm not sure this only happens in panics (I'm honestly not sure at all
why we're complaining about offline cpus). But in general I've seen
piles of dying systems that dump WARNINGS and then eventually panic.
Still would be good to not have a pointless lockdep splat in between.
> > and I think the untangling of the locking hiararchy is useful irrespective
> > of this lockdep splat. Plus Peter doesn't sound like he likes to roll
> > out more printk_deferred kind of things.
> >
> > But if you think I should do the printk_deferred thing too I can look
> > into that. Just not quite sure what that's supposed to look like now.
>
> Your patch might remove the particular lockdep splat. It might be
> worth it (Peter mentioned also an optimization effect). Anyway
> it will not prevent the deadlock.
>
> The only way to avoid the deadlock is to use printk_deferred()
> with the current printk() code.
>
>
> Finally, I have recently worked on similar problem with dying system.
> I sent the following patch for testing. I wonder if it might be
> acceptable upstream:
>
>
> From: Petr Mladek
> Subject: sched/x86: Do not warn about offline CPUs when all are being stopped
> Patch-mainline: No, just for testing
> References: bsc#1104406
>
> The warning about rescheduling offline CPUs cause dealock when
> the CPUs need to get stopped using NMI. It might happen with
> logbuf_lock, locks used by console drivers, especially tty.
> But it might also be caused by a registered kernel message
> dumper, for example, pstore.
>
> The warning is pretty common when there is a high load and
> CPUs are being stopped by native_stop_other_cpus(). But
> they are not really useful in this context. And they scrolls
> the really important messages off the screen.
>
> We need to fix printk() in the long term. But disabling
> the message looks reasonable at least in the meantime.
>
> Signed-off-by: Petr Mladek
> ---
> arch/x86/kernel/smp.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> --- a/arch/x86/kernel/smp.c
> +++ b/arch/x86/kernel/smp.c
> @@ -124,7 +124,8 @@ static bool smp_no_nmi_ipi = false;
> */
> static void native_smp_send_reschedule(int cpu)
> {
> - if (unlikely(cpu_is_offline(cpu))) {
> + if (unlikely(cpu_is_offline(cpu) &&
> +atomic_read(&stopping_cpu) < 0)) {
Hm yeah this sounds like another fix which would stop things from
getting worse when the machine is dying. I can try and stuff this (or
something like this, I honestly have no idea about cpu handling code
so no idea what's reasonable) into our CI and see what happens.
Thanks, Daniel
> WARN(1, "sched: Unexpected reschedule of offline CPU#%d!\n",
> cpu);
> return;
> }
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Per context and per client GPU busyness tracking
== Series Details ==
Series: Per context and per client GPU busyness tracking
URL : https://patchwork.freedesktop.org/series/60506/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6073 -> Patchwork_13002
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/
Known issues
Here are the changes found in Patchwork_13002 that come from known issues:
### IGT changes ###
Possible fixes
* igt@i915_selftest@live_hangcheck:
- {fi-icl-u2}:[INCOMPLETE][1] ([fdo#107713] / [fdo#108569]) ->
[PASS][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
Participating hosts (47 -> 45)
--
Additional (3): fi-icl-y fi-blb-e6850 fi-apl-guc
Missing(5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan
fi-byt-clapper
Build changes
-
* Linux: CI_DRM_6073 -> Patchwork_13002
CI_DRM_6073: c059ddabfe60a5072ace44a34a9de9b4202df6ec @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4978: b9b3646d4f04dd0204ead2a1a10f9e1806a0b622 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13002: 816d2526319110e8920c8995c49d4f7f4c95515c @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
816d25263191 drm/i915: Add sysfs toggle to enable per-client engine stats
4c2ec282f33b drm/i915: Expose per-engine client busyness
4912359f0d14 drm/i915: Update client name on context create
83be221853b1 drm/i915: Expose list of clients in sysfs
e76b877d4273 drm/i915: Track per-context engine busyness
3ae0f10d21d1 drm/i915: Move intel_engine_context_in/out into intel_lrc.c
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/icl: More workaround for port F detection due to broken VBTs
== Series Details ==
Series: drm/i915/icl: More workaround for port F detection due to broken VBTs
URL : https://patchwork.freedesktop.org/series/60508/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6073 -> Patchwork_13003
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/
Known issues
Here are the changes found in Patchwork_13003 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [PASS][1] -> [INCOMPLETE][2] ([fdo#108602] /
[fdo#108744])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-skl-6700k2: [PASS][3] -> [INCOMPLETE][4] ([fdo#104108])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-skl-6700k2/igt@kms_chamel...@common-hpd-after-suspend.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/fi-skl-6700k2/igt@kms_chamel...@common-hpd-after-suspend.html
Possible fixes
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [DMESG-FAIL][5] ([fdo#110235]) -> [PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@i915_selftest@live_hangcheck:
- {fi-icl-u2}:[INCOMPLETE][7] ([fdo#107713] / [fdo#108569]) ->
[PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (47 -> 44)
--
Additional (3): fi-icl-y fi-blb-e6850 fi-apl-guc
Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan
fi-gdg-551 fi-byt-clapper
Build changes
-
* Linux: CI_DRM_6073 -> Patchwork_13003
CI_DRM_6073: c059ddabfe60a5072ace44a34a9de9b4202df6ec @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4978: b9b3646d4f04dd0204ead2a1a10f9e1806a0b622 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13003: 28212f668a27819de69a172ee37dd33ed3755915 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
28212f668a27 drm/i915/icl: More workaround for port F detection due to broken
VBTs
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] kernel/locking/semaphore: use wake_q in up()
On Fri, May 10, 2019 at 11:28 AM Petr Mladek wrote:
>
> On Thu 2019-05-09 22:06:33, Daniel Vetter wrote:
> > console_trylock, called from within printk, can be called from pretty
> > much anywhere. Including try_to_wake_up. Note that this isn't common,
> > usually the box is in pretty bad shape at that point already. But it
> > really doesn't help when then lockdep jumps in and spams the logs,
> > potentially obscuring the real backtrace we're really interested in.
> > One case I've seen (slightly simplified backtrace):
> >
> > Fix this specific locking recursion by moving the wake_up_process out
> > from under the semaphore.lock spinlock, using wake_q as recommended by
> > Peter Zijlstra.
>
> It might make sense to mention also the optimization effect mentioned
> by Peter.
>
> > diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> > index 561acdd39960..7a6f33715688 100644
> > --- a/kernel/locking/semaphore.c
> > +++ b/kernel/locking/semaphore.c
> > @@ -169,6 +169,14 @@ int down_timeout(struct semaphore *sem, long timeout)
> > }
> > EXPORT_SYMBOL(down_timeout);
> >
> > +/* Functions for the contended case */
> > +
> > +struct semaphore_waiter {
> > + struct list_head list;
> > + struct task_struct *task;
> > + bool up;
> > +};
> > +
> > /**
> > * up - release the semaphore
> > * @sem: the semaphore to release
> > @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> > void up(struct semaphore *sem)
> > {
> > unsigned long flags;
> > + struct semaphore_waiter *waiter;
> > + DEFINE_WAKE_Q(wake_q);
>
> We need to call wake_q_init(&wake_q) to make sure that
> it is empty.
DEFINE_WAKE_Q does that already, and if it didn't, I'd wonder how I
managed to boot with this patch. console_lock is usally terribly
contented because thanks to fbcon we must do a full display modeset
while holding it, which takes forever. As long as anyone printks
meanwhile (guaranteed while loading drivers really) you have
contention.
-Daniel
> Best Regards,
> Petr
>
> > raw_spin_lock_irqsave(&sem->lock, flags);
> > - if (likely(list_empty(&sem->wait_list)))
> > + if (likely(list_empty(&sem->wait_list))) {
> > sem->count++;
> > - else
> > - __up(sem);
> > + } else {
> > + waiter = list_first_entry(&sem->wait_list,
> > +struct semaphore_waiter, list);
> > + list_del(&waiter->list);
> > + waiter->up = true;
> > + wake_q_add(&wake_q, waiter->task);
> > + }
> > raw_spin_unlock_irqrestore(&sem->lock, flags);
> > +
> > + wake_up_q(&wake_q);
> > }
> > EXPORT_SYMBOL(up);
> >
> > -/* Functions for the contended case */
> > -
> > -struct semaphore_waiter {
> > - struct list_head list;
> > - struct task_struct *task;
> > - bool up;
> > -};
> > -
> > /*
> > * Because this function is inlined, the 'state' parameter will be
> > * constant, and thus optimised away by the compiler. Likewise the
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [RFC i-g-t 1/1] intel-gpu-top: Support for client stats
Quoting Tvrtko Ursulin (2019-05-10 14:23:12)
> From: Tvrtko Ursulin
>
> Adds support for per-client engine busyness stats i915 exports in sysfs
> and produces output like the below:
>
> ==
> intel-gpu-top - 935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s
>
> IMC reads: 1401 MiB/s
> IMC writes:4 MiB/s
>
> ENGINE BUSY MI_SEMA MI_WAIT
> Render/3D/0 63.73% |███ | 3% 0%
>Blitter/09.53% |██▊ | 6% 0%
> Video/0 39.32% |███▊ | 16% 0%
> Video/1 15.62% |▋ | 0% 0%
> VideoEnhance/00.00% | | 0% 0%
>
> PIDNAME RCS BCS VCS VECS
> 4084gem_wsim |█▌ ||█ || || |
> 4086gem_wsim |█▌ || ||███|| |
> ==
>
> Apart from the existing physical engine utilization it now also shows
> utilization per client and per engine class.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> tools/intel_gpu_top.c | 590 +-
> 1 file changed, 584 insertions(+), 6 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index cc8db7c539ed..88e1ad52d17c 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -659,8 +659,403 @@ static void pmu_sample(struct engines *engines)
> }
> }
>
> +enum client_status {
> + FREE = 0, /* mbz */
> + ALIVE,
> + PROBE
> +};
> +
> +struct clients;
> +
> +struct client {
> + struct clients *clients;
> +
> + enum client_status status;
> + unsigned int id;
> + unsigned int pid;
> + char name[128];
> + unsigned int samples;
> + unsigned long total;
> + struct engines *engines;
> + unsigned long *val;
> + uint64_t *last;
> +};
> +
> +struct engine_class {
> + unsigned int class;
> + const char *name;
> + unsigned int num_engines;
> +};
> +
> +struct clients {
> + unsigned int num_classes;
> + struct engine_class *class;
> +
> + unsigned int num_clients;
> + struct client *client;
> +};
> +
> +#define for_each_client(clients, c, tmp) \
> + for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> +(tmp > 0); (tmp)--, (c)++)
> +
> +#define SYSFS_ENABLE "/sys/class/drm/card0/clients/enable_stats"
> +
> +bool __stats_enabled;
> +
> +static int __set_stats(bool val)
> +{
> + int fd, ret;
> +
> + fd = open(SYSFS_ENABLE, O_WRONLY);
> + if (fd < 0)
> + return -errno;
> +
> + ret = write(fd, val ? "1" : "0", 2);
close(fd);
Might as well still be tidy on error when it's trivial to do so.
> + if (ret < 0)
> + return -errno;
> + else if (ret < 2)
> + return 1;
> +
> + close(fd);
> +
> + return 0;
> +}
> +
> +static void __restore_stats(void)
> +{
> + int ret;
> +
> + if (__stats_enabled)
> + return;
> +
> + ret = __set_stats(false);
> + if (ret)
> + fprintf(stderr, "Failed to disable per-client stats! (%d)\n",
> + ret);
> +}
> +
> +static void __restore_stats_signal(int sig)
> +{
> + exit(0);
> +}
> +
> +static int enable_stats(void)
> +{
> + int fd, ret;
> +
> + fd = open(SYSFS_ENABLE, O_RDONLY);
> + if (fd < 0)
> + return -errno;
> +
> + close(fd);
> +
> + __stats_enabled = filename_to_u64(SYSFS_ENABLE, 10);
> + if (__stats_enabled)
> + return 0;
> +
> + ret = __set_stats(true);
> + if (!ret) {
> + if (atexit(__restore_stats))
> + fprintf(stderr, "Failed to register exit handler!");
> +
> + if (signal(SIGINT, __restore_stats_signal))
> + fprintf(stderr, "Failed to register signal handler!");
That really suggests an alternative mechanism where the stats are only
active for as long as the open(sysfs/stats) is. However, iirc, sysfs
doesn't allow us to hook into the open!
In which case we could hook into the write, and keep it enabled for as
long as the user write("1") until the fd is closed. Food for thought, I
hope you convince me we don't need optional stats in the first place :)
> + } else {
> + fprintf(stderr, "Failed to enable per-client stats! (%d)\n",
> + ret);
> + }
> +
> + return ret;
> +}
> +
> +static struct clients *init_clients(void)
> +{
> + struct clients *clients = malloc(sizeof(*clients));
We purport this to be a user-facing tool, it should g
[Intel-gfx] ✓ Fi.CI.IGT: success for Per context and per client GPU busyness tracking
== Series Details == Series: Per context and per client GPU busyness tracking URL : https://patchwork.freedesktop.org/series/60506/ State : success == Summary == CI Bug Log - changes from CI_DRM_6073_full -> Patchwork_13002_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13002_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_softpin@noreloc-s3: - shard-apl: ([PASS][1], [PASS][2]) -> [DMESG-WARN][3] ([fdo#108566]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-apl7/igt@gem_soft...@noreloc-s3.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-apl8/igt@gem_soft...@noreloc-s3.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-apl7/igt@gem_soft...@noreloc-s3.html * igt@i915_pm_rpm@basic-pci-d3-state: - shard-skl: ([PASS][4], [PASS][5]) -> [INCOMPLETE][6] ([fdo#107807]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl1/igt@i915_pm_...@basic-pci-d3-state.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl2/igt@i915_pm_...@basic-pci-d3-state.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-skl2/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_pm_rpm@gem-execbuf-stress: - shard-skl: ([PASS][7], [PASS][8]) -> [INCOMPLETE][9] ([fdo#107803] / [fdo#107807]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl7/igt@i915_pm_...@gem-execbuf-stress.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl9/igt@i915_pm_...@gem-execbuf-stress.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-skl7/igt@i915_pm_...@gem-execbuf-stress.html * igt@i915_pm_rpm@legacy-planes-dpms: - shard-skl: [PASS][10] -> [INCOMPLETE][11] ([fdo#107807]) +1 similar issue [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl5/igt@i915_pm_...@legacy-planes-dpms.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-skl2/igt@i915_pm_...@legacy-planes-dpms.html * igt@kms_dp_dsc@basic-dsc-enable-edp: - shard-iclb: [PASS][12] -> [SKIP][13] ([fdo#109349]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb2/igt@kms_dp_...@basic-dsc-enable-edp.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-iclb1/igt@kms_dp_...@basic-dsc-enable-edp.html * igt@kms_flip@2x-flip-vs-modeset-vs-hang: - shard-hsw: ([PASS][14], [PASS][15]) -> [INCOMPLETE][16] ([fdo#103540]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-hsw2/igt@kms_f...@2x-flip-vs-modeset-vs-hang.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-hsw1/igt@kms_f...@2x-flip-vs-modeset-vs-hang.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-hsw1/igt@kms_f...@2x-flip-vs-modeset-vs-hang.html * igt@kms_flip@flip-vs-expired-vblank: - shard-glk: ([PASS][17], [PASS][18]) -> [FAIL][19] ([fdo#102887] / [fdo#105363]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk5/igt@kms_f...@flip-vs-expired-vblank.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk2/igt@kms_f...@flip-vs-expired-vblank.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-glk1/igt@kms_f...@flip-vs-expired-vblank.html * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite: - shard-iclb: [PASS][20] -> [FAIL][21] ([fdo#103167]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb2/igt@kms_frontbuffer_track...@fbc-rgb565-draw-pwrite.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-iclb6/igt@kms_frontbuffer_track...@fbc-rgb565-draw-pwrite.html * igt@kms_psr@psr2_primary_mmap_gtt: - shard-iclb: [PASS][22] -> [SKIP][23] ([fdo#109441]) +2 similar issues [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb2/igt@kms_psr@psr2_primary_mmap_gtt.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-iclb6/igt@kms_psr@psr2_primary_mmap_gtt.html * igt@tools_test@tools_test: - shard-glk: [PASS][24] -> [SKIP][25] ([fdo#109271]) [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk4/igt@tools_test@tools_test.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13002/shard-glk4/igt@tools_test@tools_test.html Possible fixes * igt@gem_tiled_swapping@non-threaded: - shard-hsw: ([FAIL][26], [PASS][27]) ([fdo#108686]) -> [PASS][28] [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-hsw4/igt@gem_tiled_swapp...@non-threaded.html [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-hsw6/igt@gem_tiled_swapp..
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/icl: More workaround for port F detection due to broken VBTs
== Series Details == Series: drm/i915/icl: More workaround for port F detection due to broken VBTs URL : https://patchwork.freedesktop.org/series/60508/ State : success == Summary == CI Bug Log - changes from CI_DRM_6073_full -> Patchwork_13003_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13003_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_eio@in-flight-10ms: - shard-glk: ([PASS][1], [PASS][2]) -> [FAIL][3] ([fdo#105957]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk2/igt@gem_...@in-flight-10ms.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk5/igt@gem_...@in-flight-10ms.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-glk7/igt@gem_...@in-flight-10ms.html * igt@i915_pm_rpm@basic-pci-d3-state: - shard-skl: ([PASS][4], [PASS][5]) -> [INCOMPLETE][6] ([fdo#107807]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl1/igt@i915_pm_...@basic-pci-d3-state.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl2/igt@i915_pm_...@basic-pci-d3-state.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-skl2/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_pm_rpm@i2c: - shard-iclb: [PASS][7] -> [DMESG-WARN][8] ([fdo#109982]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb5/igt@i915_pm_...@i2c.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-iclb2/igt@i915_pm_...@i2c.html * igt@i915_pm_rpm@legacy-planes-dpms: - shard-skl: [PASS][9] -> [INCOMPLETE][10] ([fdo#107807]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl5/igt@i915_pm_...@legacy-planes-dpms.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-skl1/igt@i915_pm_...@legacy-planes-dpms.html * igt@kms_dp_dsc@basic-dsc-enable-edp: - shard-iclb: [PASS][11] -> [SKIP][12] ([fdo#109349]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb2/igt@kms_dp_...@basic-dsc-enable-edp.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-iclb7/igt@kms_dp_...@basic-dsc-enable-edp.html * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible: - shard-glk: ([PASS][13], [PASS][14]) -> [FAIL][15] ([fdo#105363]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk3/igt@kms_f...@2x-flip-vs-expired-vblank-interruptible.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-glk7/igt@kms_f...@2x-flip-vs-expired-vblank-interruptible.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-glk7/igt@kms_f...@2x-flip-vs-expired-vblank-interruptible.html * igt@kms_flip@2x-flip-vs-modeset-vs-hang: - shard-hsw: ([PASS][16], [PASS][17]) -> [INCOMPLETE][18] ([fdo#103540]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-hsw2/igt@kms_f...@2x-flip-vs-modeset-vs-hang.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-hsw1/igt@kms_f...@2x-flip-vs-modeset-vs-hang.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-hsw7/igt@kms_f...@2x-flip-vs-modeset-vs-hang.html * igt@kms_flip@flip-vs-expired-vblank-interruptible: - shard-skl: ([PASS][19], [PASS][20]) -> [FAIL][21] ([fdo#105363]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl6/igt@kms_f...@flip-vs-expired-vblank-interruptible.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-skl4/igt@kms_f...@flip-vs-expired-vblank-interruptible.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-skl1/igt@kms_f...@flip-vs-expired-vblank-interruptible.html * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render: - shard-iclb: [PASS][22] -> [FAIL][23] ([fdo#103167]) +1 similar issue [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb4/igt@kms_frontbuffer_track...@fbc-1p-offscren-pri-indfb-draw-render.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-iclb1/igt@kms_frontbuffer_track...@fbc-1p-offscren-pri-indfb-draw-render.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render: - shard-iclb: ([PASS][24], [PASS][25]) -> [FAIL][26] ([fdo#103167]) +3 similar issues [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb2/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-cur-indfb-draw-render.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6073/shard-iclb7/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-cur-indfb-draw-render.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13003/shard-iclb2/igt@kms_frontbuffer_track
Re: [Intel-gfx] [PATCH v3 1/2] drm/i915: Make sandybridge_pcode_read() deal with the second data register
On Fri, May 03, 2019 at 10:08:30PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> The pcode mailbox has two data registers. So far we've only ever used
> the one, but that's about to change. Expose the second data register to
> the callers of sandybridge_pcode_read().
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Matt Roper
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/intel_pm.c | 12 +++-
> drivers/gpu/drm/i915/intel_sideband.c | 15 +--
> drivers/gpu/drm/i915/intel_sideband.h | 3 ++-
> 4 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 14cd83e9ea8b..203088f6f269 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1494,7 +1494,7 @@ static int gen6_drpc_info(struct seq_file *m)
>
> if (INTEL_GEN(dev_priv) <= 7)
> sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS,
> -&rc6vids);
> +&rc6vids, NULL);
>
> seq_printf(m, "RC1e Enabled: %s\n",
> yesno(rcctl1 & GEN6_RC_CTL_RC1e_ENABLE));
> @@ -1777,7 +1777,7 @@ static int i915_ring_freq_table(struct seq_file *m,
> void *unused)
> ia_freq = gpu_freq;
> sandybridge_pcode_read(dev_priv,
> GEN6_PCODE_READ_MIN_FREQ_TABLE,
> -&ia_freq);
> +&ia_freq, NULL);
> seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
> intel_gpu_freq(dev_priv, (gpu_freq *
>(IS_GEN9_BC(dev_priv) ||
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ef9fc77f8162..b043a96e123c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2822,7 +2822,7 @@ static void intel_read_wm_latency(struct
> drm_i915_private *dev_priv,
> val = 0; /* data0 to be programmed to 0 for first set */
> ret = sandybridge_pcode_read(dev_priv,
>GEN9_PCODE_READ_MEM_LATENCY,
> - &val);
> + &val, NULL);
>
> if (ret) {
> DRM_ERROR("SKL Mailbox read error = %d\n", ret);
> @@ -2841,7 +2841,7 @@ static void intel_read_wm_latency(struct
> drm_i915_private *dev_priv,
> val = 1; /* data0 to be programmed to 1 for second set */
> ret = sandybridge_pcode_read(dev_priv,
>GEN9_PCODE_READ_MEM_LATENCY,
> - &val);
> + &val, NULL);
> if (ret) {
> DRM_ERROR("SKL Mailbox read error = %d\n", ret);
> return;
> @@ -7061,7 +7061,7 @@ static void gen6_init_rps_frequencies(struct
> drm_i915_private *dev_priv)
>
> if (sandybridge_pcode_read(dev_priv,
> HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> -&ddcc_status) == 0)
> +&ddcc_status, NULL) == 0)
> rps->efficient_freq =
> clamp_t(u8,
> ((ddcc_status >> 8) & 0xff),
> @@ -7408,7 +7408,8 @@ static void gen6_enable_rc6(struct drm_i915_private
> *dev_priv)
> GEN6_RC_CTL_HW_ENABLE);
>
> rc6vids = 0;
> - ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS,
> &rc6vids);
> + ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS,
> + &rc6vids, NULL);
> if (IS_GEN(dev_priv, 6) && ret) {
> DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n");
> } else if (IS_GEN(dev_priv, 6) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff)
> < 450)) {
> @@ -8555,7 +8556,8 @@ void intel_init_gt_powersave(struct drm_i915_private
> *dev_priv)
> IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv)) {
> u32 params = 0;
>
> - sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, ¶ms);
> + sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS,
> +¶ms, NULL);
> if (params & BIT(31)) { /* OC supported */
> DRM_DEBUG_DRIVER("Overclocking supported, max: %dMHz,
> overclock: %dMHz\n",
>(rps->max_freq & 0xff) * 50,
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c
> b/drivers/gpu/drm/i915/intel_sideband.c
> index 87b5a14c7ca8..a115625e980c 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_s
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Fri, May 03, 2019 at 10:08:31PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> ICL has so many planes that it can easily exceed the maximum
> effective memory bandwidth of the system. We must therefore check
> that we don't exceed that limit.
>
> The algorithm is very magic number heavy and lacks sufficient
> explanation for now. We also have no sane way to query the
> memory clock and timings, so we must rely on a combination of
> raw readout from the memory controller and hardcoded assumptions.
> The memory controller values obviously change as the system
> jumps between the different SAGV points, so we try to stabilize
> it first by disabling SAGV for the duration of the readout.
>
> The utilized bandwidth is tracked via a device wide atomic
> private object. That is actually not robust because we can't
> afford to enforce strict global ordering between the pipes.
> Thus I think I'll need to change this to simply chop up the
> available bandwidth between all the active pipes. Each pipe
> can then do whatever it wants as long as it doesn't exceed
> its budget. That scheme will also require that we assume that
> any number of planes could be active at any time.
>
> TODO: make it robust and deal with all the open questions
>
> v2: Sleep longer after disabling SAGV
> v3: Poll for the dclk to get raised (seen it take 250ms!)
> If the system has 2133MT/s memory then we pointlessly
> wait one full second :(
> v4: Use the new pcode interface to get the qgv points rather
> that using hardcoded numbers
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 229 ++
> drivers/gpu/drm/i915/i915_drv.h | 10 +
> drivers/gpu/drm/i915/i915_reg.h | 3 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> drivers/gpu/drm/i915/intel_bw.c | 181 +
> drivers/gpu/drm/i915/intel_bw.h | 46 +
> drivers/gpu/drm/i915/intel_display.c | 40 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 10 files changed, 533 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> create mode 100644 drivers/gpu/drm/i915/intel_bw.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 68106fe35a04..139a0fc19390 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> intel_atomic.o \
> intel_atomic_plane.o \
> intel_bios.o \
> + intel_bw.o \
> intel_cdclk.o \
> intel_color.o \
> intel_combo_phy.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5ed864752c7b..b7fa7b51c2e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -70,6 +70,7 @@
> #include "intel_overlay.h"
> #include "intel_pipe_crc.h"
> #include "intel_pm.h"
> +#include "intel_sideband.h"
> #include "intel_sprite.h"
> #include "intel_uc.h"
>
> @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> +struct intel_qgv_point {
> + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> +};
> +
> +struct intel_sagv_info {
> + struct intel_qgv_point points[3];
> + u8 num_points;
> + u8 num_channels;
> + u8 t_bl;
> + enum intel_dram_type dram_type;
> +};
> +
> +static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
> + struct intel_sagv_info *si)
> +{
> + u32 val = 0;
> + int ret;
> +
> + ret = sandybridge_pcode_read(dev_priv,
> + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> + &val, NULL);
> + if (ret)
> + return ret;
> +
> + switch (val & 0xf) {
> + case 0:
> + si->dram_type = INTEL_DRAM_DDR4;
> + break;
> + case 1:
> + si->dram_type = INTEL_DRAM_DDR3;
> + break;
> + case 2:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + case 3:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + default:
> + MISSING_CASE(val & 0xf);
> + break;
> + }
> +
> + si->num_channels = (val & 0xf0) >> 4;
> + si->num_points = (val & 0xf00) >> 8;
> +
> + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> +
> + return 0;
> +}
> +
> +static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> + struct intel_qgv_point *sp,
> + int point)
> +{
> + u32 val = 0, val2;
> + int ret;
> +
> + ret = sandybridge_pcode_read(dev_priv,
> +
