Re: [Intel-gfx] [PATCH 06/19] drm/vmwgfx: Drop the cursor locking hack
On Thu, Mar 23, 2017 at 07:22:31AM +0100, Thomas Hellstrom wrote: > On 03/22/2017 10:50 PM, Daniel Vetter wrote: > > It's been around forever, no one bothered to address the FIXME, so I > > presume it's all fine. > > > > Cc: Sinclair Yeh > > Cc: Thomas Hellstrom > > Signed-off-by: Daniel Vetter > > NAK. We need to properly address this. Probably as part of the atomic > update. So could someone with vmwgfx understanding explain this? Note that the FIXME was originally added by me years ago, because I wasn't sure (only about 90%) that this is safe, and was essentially pleading for a vmwgfx expert to review this? Since it didn't happen I presume it's not that terribly and probably safe ... I'm still 90% sure that this is correct, but I'd love for a vmwgfx to audit it. Replying with a NAK is kinda not the response I was hoping for (and yes I guess I should have explained what's going on here better, but it's just a git blame of the FIXME comment away). Thanks, Daniel > /Thomas > > > > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 25 - > > 1 file changed, 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > index d492d57d5309..424b3fc57203 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > > @@ -148,15 +148,6 @@ int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, > > struct drm_file *file_priv, > > s32 hotspot_x, hotspot_y; > > int ret; > > > > - /* > > -* FIXME: Unclear whether there's any global state touched by the > > -* cursor_set function, especially vmw_cursor_update_position looks > > -* suspicious. For now take the easy route and reacquire all locks. We > > -* can do this since the caller in the drm core doesn't check anything > > -* which is protected by any looks. > > -*/ > > - drm_modeset_unlock_crtc(crtc); > > - drm_modeset_lock_all(dev_priv->dev); > > hotspot_x = hot_x + du->hotspot_x; > > hotspot_y = hot_y + du->hotspot_y; > > > > @@ -224,9 +215,6 @@ int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, > > struct drm_file *file_priv, > > } > > > > out: > > - drm_modeset_unlock_all(dev_priv->dev); > > - drm_modeset_lock_crtc(crtc, crtc->cursor); > > - > > return ret; > > } > > > > @@ -239,25 +227,12 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, > > int x, int y) > > du->cursor_x = x + du->set_gui_x; > > du->cursor_y = y + du->set_gui_y; > > > > - /* > > -* FIXME: Unclear whether there's any global state touched by the > > -* cursor_set function, especially vmw_cursor_update_position looks > > -* suspicious. For now take the easy route and reacquire all locks. We > > -* can do this since the caller in the drm core doesn't check anything > > -* which is protected by any looks. > > -*/ > > - drm_modeset_unlock_crtc(crtc); > > - drm_modeset_lock_all(dev_priv->dev); > > - > > vmw_cursor_update_position(dev_priv, shown, > >du->cursor_x + du->hotspot_x + > >du->core_hotspot_x, > >du->cursor_y + du->hotspot_y + > >du->core_hotspot_y); > > > > - drm_modeset_unlock_all(dev_priv->dev); > > - drm_modeset_lock_crtc(crtc, crtc->cursor); > > - > > return 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] drm/i915: Actually pass the reclaim gfp_t along to shmemfs!
On ke, 2017-03-22 at 22:34 +, Chris Wilson wrote: > Words cannot describe the embarrassment at creating a new gfp_t relaim to > only prevent the oomkiller but allow direct|kswapd reclaim, and then not > use it in the shmem_read_mapping_page_gfp(). > > Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the > oom for gfx allocations") > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen > Cc: Daniel Vetter Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/19] drm/vmwgfx: Drop the cursor locking hack
On Thu, Mar 23, 2017 at 08:28:32AM +0100, Daniel Vetter wrote: > On Thu, Mar 23, 2017 at 07:22:31AM +0100, Thomas Hellstrom wrote: > > On 03/22/2017 10:50 PM, Daniel Vetter wrote: > > > It's been around forever, no one bothered to address the FIXME, so I > > > presume it's all fine. > > > > > > Cc: Sinclair Yeh > > > Cc: Thomas Hellstrom > > > Signed-off-by: Daniel Vetter > > > > NAK. We need to properly address this. Probably as part of the atomic > > update. > > So could someone with vmwgfx understanding explain this? Note that the > FIXME was originally added by me years ago, because I wasn't sure (only > about 90%) that this is safe, and was essentially pleading for a vmwgfx > expert to review this? > > Since it didn't happen I presume it's not that terribly and probably safe > ... > > I'm still 90% sure that this is correct, but I'd love for a vmwgfx to > audit it. Replying with a NAK is kinda not the response I was hoping for > (and yes I guess I should have explained what's going on here better, but > it's just a git blame of the FIXME comment away). Bit more context even: This lock dropping dance is _not_ safe from a drm core perspective. But when I've done the original kms locking rework the tradeoff between upsetting core state a bit and totally breaking vmwgfx leaned towards not breaking vmwgfx. And iirc you or Syeh promised to look at this and then either remove the FIXME, maybe with a vmwgfx lock/unlock added if there's a gap (I looked, didn't find one, but I don't understand vmwgfx in details really). Thanks, Daniel -- 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
[Intel-gfx] [PATCH] drm: Make the decision to keep vblank irq enabled earlier
We want to provide the vblank irq shadow for pageflip events as well as vblank queries. Such events are completed within the vblank interrupt handler, and so the current check for disabling the irq will disable it from with the same interrupt as the last pageflip event. If we move the decision on whether to disable the irq (based on there no being no remaining vblank events, i.e. vblank->refcount == 0) to before we signal the events, we will only disable the irq on the interrupt after the last event was signaled. In the normal course of events, this will keep the vblank irq enabled for the entire flip sequence whereas before it would flip-flop around every interrupt. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Daniel Vetter Cc: Michel Dänzer Cc: Laurent Pinchart Cc: Dave Airlie , Cc: Mario Kleiner --- drivers/gpu/drm/drm_irq.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 5b77057e91ca..1d6bcee3708f 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1741,6 +1741,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; unsigned long irqflags; + bool disable_irq; if (WARN_ON_ONCE(!dev->num_crtcs)) return false; @@ -1768,16 +1769,19 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) spin_unlock(&dev->vblank_time_lock); wake_up(&vblank->queue); - drm_handle_vblank_events(dev, pipe); /* With instant-off, we defer disabling the interrupt until after -* we finish processing the following vblank. The disable has to -* be last (after drm_handle_vblank_events) so that the timestamp -* is always accurate. +* we finish processing the following vblank after all events have +* been signaled. The disable has to be last (after +* drm_handle_vblank_events) so that the timestamp is always accurate. */ - if (dev->vblank_disable_immediate && - drm_vblank_offdelay > 0 && - !atomic_read(&vblank->refcount)) + disable_irq = (dev->vblank_disable_immediate && + drm_vblank_offdelay > 0 && + !atomic_read(&vblank->refcount)); + + drm_handle_vblank_events(dev, pipe); + + if (disable_irq) vblank_disable_fn((unsigned long)vblank); spin_unlock_irqrestore(&dev->event_lock, irqflags); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
> From: Dong, Chuanxiao > Sent: Thursday, March 23, 2017 1:29 PM > > Ping for review. GVT relies on the notification before a request submitted > and after a request completed, just like when using execlist mode submission. > > > -Original Message- > > From: Dong, Chuanxiao > > Sent: Wednesday, March 22, 2017 2:35 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: intel-gvt-...@lists.freedesktop.org; Dong, Chuanxiao > > ; Zheng, Xiao ; Tian, > > Kevin > > Subject: [PATCH v3] drm/i915/scheduler: add gvt notification for guc > > submission > > > > GVT request needs a manual mmio load/restore. Before GuC submit a > > request, send notification to gvt for mmio loading. And after the GuC > > finished this GVT request, notify gvt again for mmio restore. This > > follows the usage when using execlists submission. > > > > v2: use context_status_change instead of execlists_context_status_change > > for better understanding (ZhengXiao) > > v3: remove the comment as it is obvious and not friendly to > > the caller (Kevin) > > > > Cc: xiao.zh...@intel.com > > Cc: kevin.t...@intel.com > > Signed-off-by: Chuanxiao Dong Reviewed-by: Kevin Tian > > --- > > drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++ > > drivers/gpu/drm/i915/intel_lrc.c | 21 +++-- > > drivers/gpu/drm/i915/intel_lrc.h | 13 + > > 3 files changed, 19 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > > b/drivers/gpu/drm/i915/i915_guc_submission.c > > index 055467a..0195547 100644 > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > > @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct > > drm_i915_gem_request *rq) > > unsigned long flags; > > int b_ret; > > > > + context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); > > + > > /* WA to flush out the pending GMADR writes to ring buffer. */ > > if (i915_vma_is_map_and_fenceable(rq->ring->vma)) > > POSTING_READ_FW(GUC_STATUS); > > @@ -634,6 +636,7 @@ static void i915_guc_irq_handler(unsigned long > data) > > rq = port[0].request; > > while (rq && i915_gem_request_completed(rq)) { > > trace_i915_gem_request_out(rq); > > + context_status_change(rq, > > INTEL_CONTEXT_SCHEDULE_OUT); > > i915_gem_request_put(rq); > > port[0].request = port[1].request; > > port[1].request = NULL; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index eec1e71..24c69b5 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct > > i915_gem_context *ctx, > > return ctx->engine[engine->id].lrc_desc; } > > > > -static inline void > > -execlists_context_status_change(struct drm_i915_gem_request *rq, > > - unsigned long status) > > -{ > > - /* > > -* Only used when GVT-g is enabled now. When GVT-g is disabled, > > -* The compiler should eliminate this function as dead-code. > > -*/ > > - if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) > > - return; > > - > > - atomic_notifier_call_chain(&rq->engine->context_status_notifier, > > - status, rq); > > -} > > - > > static void > > execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 > > *reg_state) { @@ -350,7 +335,7 @@ static void > > execlists_submit_ports(struct intel_engine_cs *engine) > > > > GEM_BUG_ON(port[0].count > 1); > > if (!port[0].count) > > - execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > > INTEL_CONTEXT_SCHEDULE_IN); > > desc[0] = execlists_update_context(port[0].request); > > GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); > @@ > > -358,7 +343,7 @@ static void execlists_submit_ports(struct > > intel_engine_cs *engine) > > > > if (port[1].request) { > > GEM_BUG_ON(port[1].count); > > - execlists_context_status_change(port[1].request, > > + context_status_change(port[1].request, > > > > INTEL_CONTEXT_SCHEDULE_IN); > > desc[1] = execlists_update_context(port[1].request); > > GEM_DEBUG_EXEC(port[1].context_id = > upper_32_bits(desc[1])); @@ > > -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data) > > if (--port[0].count == 0) { > > GEM_BUG_ON(status & > > GEN8_CTX_STATUS_PREEMPTED); > > > > GEM_BUG_ON(!i915_gem_request_completed(port[0].request)); > > - > > execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > > INTEL_CONTEXT_SCHEDULE_OUT); > > > > > > trace_i915_gem_request_out(port[0].request); > >
Re: [Intel-gfx] [PATCH] drm: Make the decision to keep vblank irq enabled earlier
Better subject might be drm: Use vblank irq shadow for entire flip sequence -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for more drmP.h cleanup (rev7)
== Series Details == Series: more drmP.h cleanup (rev7) URL : https://patchwork.freedesktop.org/series/21670/ State : success == Summary == Series 21670v7 more drmP.h cleanup https://patchwork.freedesktop.org/api/1.0/series/21670/revisions/7/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 462s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 453s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 584s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 539s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 511s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 504s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 443s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 524s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 499s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 485s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 479s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 598s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 490s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 522s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 469s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 548s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 422s b1ecc49ac71e484430083e5b3b7cd75d4976a465 drm-tip: 2017y-03m-23d-07h-40m-49s UTC integration manifest f1b0b85 drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool 7dd3e06 drm/vblank: Remove DRM_VBLANKTIME_IN_VBLANK 4ce75fc drm/todo: Add tinydrm refactoring ideas 4bb2f71 drm: document drm_ioctl.[hc] 3b85dbb drm: Extract drm_ioctl.h 7218a99 drm: Consolidate and document sysfs support 6c65c2f drm: update todo.rst a7ee095 drm/debugfs: Add kerneldoc 64feb24 drm: document driver interface for CRC capturing c0d8423 drm: Extract drm_debugfs.h de0dcf8 drm: drop extern from function decls == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4265/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH] completion: shellcheck bash completion and fix issues
Add bash_completion to 'make shellcheck' target, and fix the failures. Signed-off-by: Jani Nikula --- Makefile| 2 +- bash_completion | 13 +++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 30fc3dd3bff8..c1fe02c1b565 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ SC_EXCLUDE := \ -e SC2143 shellcheck: - shellcheck $(SC_EXCLUDE) dim + shellcheck $(SC_EXCLUDE) dim bash_completion clean: rm -f drm-intel.html drm-intel-flow.svg dim.html drm-misc.html diff --git a/bash_completion b/bash_completion index 9f659b4ebcce..7dfc4b86cb13 100644 --- a/bash_completion +++ b/bash_completion @@ -3,7 +3,7 @@ dim () { if [[ "x$1" = "xcd" ]]; then - cd `cat ~/.dim-last-path` + cd $(cat ~/.dim-last-path) || exit else command dim "$@" fi @@ -12,6 +12,7 @@ dim () _dim () { local args arg cur prev words cword split + local nightly_branches upstream_branches opts cmds aliasref # require bash-completion with _init_completion type -t _init_completion >/dev/null 2>&1 || return @@ -26,18 +27,18 @@ _dim () # args = number of arguments _count_args - local nightly_branches="$(dim list-branches)" - local upstream_branches="$(dim list-upstreams)" + nightly_branches="$(dim list-branches)" + upstream_branches="$(dim list-upstreams)" if [ -z "${arg}" ]; then # top level completion case "${cur}" in -*) - local opts="-d -f -i" + opts="-d -f -i" COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) ;; *) - local cmds="$(dim list-commands) $(dim list-aliases | sed 's/\t.*//')" + cmds="$(dim list-commands) $(dim list-aliases | sed 's/\t.*//')" COMPREPLY=( $(compgen -W "${cmds}" -- ${cur}) ) ;; esac @@ -45,7 +46,7 @@ _dim () fi # complete aliases like the actual command - local aliasref=$(dim list-aliases | sed -n "s/^${arg}\t\(.*\)/\1/p") + aliasref=$(dim list-aliases | sed -n "s/^${arg}\t\(.*\)/\1/p") if [[ -n "$aliasref" ]]; then arg="$aliasref" fi -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/19] drm/vmwgfx: Drop the cursor locking hack
Hi, Daniel, On 03/23/2017 08:31 AM, Daniel Vetter wrote: > On Thu, Mar 23, 2017 at 08:28:32AM +0100, Daniel Vetter wrote: >> On Thu, Mar 23, 2017 at 07:22:31AM +0100, Thomas Hellstrom wrote: >>> On 03/22/2017 10:50 PM, Daniel Vetter wrote: It's been around forever, no one bothered to address the FIXME, so I presume it's all fine. Cc: Sinclair Yeh Cc: Thomas Hellstrom Signed-off-by: Daniel Vetter >>> NAK. We need to properly address this. Probably as part of the atomic >>> update. >> So could someone with vmwgfx understanding explain this? Note that the >> FIXME was originally added by me years ago, because I wasn't sure (only >> about 90%) that this is safe, and was essentially pleading for a vmwgfx >> expert to review this? >> >> Since it didn't happen I presume it's not that terribly and probably safe >> ... >> >> I'm still 90% sure that this is correct, but I'd love for a vmwgfx to >> audit it. Replying with a NAK is kinda not the response I was hoping for >> (and yes I guess I should have explained what's going on here better, but >> it's just a git blame of the FIXME comment away). So the code has been left in place because it works. Altering it now will create unnecessary merge conflicts with the atomic code, and the change isn't tested and audited which means we need to drop focus from what we're doing and audit and test code that isn't going to be used anyway for not apparent reason? But otoh put in the below context there indeed is a reason. From a quick audit of the existing code it seems like at least vmw_cursor_update_position is touching global device state so I think at a minimum we need to take a spinlock in that function. Otherwise it seems to be safe. But I prefer if we can do that as part of the atomic update? Thanks, Thomas > Bit more context even: This lock dropping dance is _not_ safe from a drm > core perspective. But when I've done the original kms locking rework the > tradeoff between upsetting core state a bit and totally breaking vmwgfx > leaned towards not breaking vmwgfx. And iirc you or Syeh promised to look > at this and then either remove the FIXME, maybe with a vmwgfx lock/unlock > added if there's a gap (I looked, didn't find one, but I don't understand > vmwgfx in details really). > > Thanks, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/8] drm: Add driver-private objects to atomic state
Op 22-03-17 om 22:05 schreef Pandiyan, Dhinakaran: > On Wed, 2017-03-22 at 11:00 +0100, Maarten Lankhorst wrote: >> Op 16-03-17 om 08:10 schreef Dhinakaran Pandiyan: >>> From: "Pandiyan, Dhinakaran" >>> >>> It is necessary to track states for objects other than connector, crtc >>> and plane for atomic modesets. But adding objects like DP MST link >>> bandwidth to drm_atomic_state would mean that a non-core object will be >>> modified by the core helper functions for swapping and clearing >>> it's state. So, lets add void * objects and helper functions that operate >>> on void * types to keep these objects and states private to the core. >>> Drivers can then implement specific functions to swap and clear states. >>> The other advantage having just void * for these objects in >>> drm_atomic_state is that objects of different types can be managed in the >>> same state array. >>> >>> v2: Added docs and new iterator to filter private objects (Daniel) >>> v3: Macro alignment (Chris) >>> >>> Cc: Daniel Vetter >>> Cc: Archit Taneja >>> Cc: Maarten Lankhorst >>> Cc: Chris Wilson >>> Cc: Harry Wentland >>> Acked-by: Harry Wentland >>> Suggested-by: Daniel Vetter >>> Signed-off-by: Dhinakaran Pandiyan >> Mostly looks good, but too many null checks. I think it's best to get rid of >> them all >> by freeing state->driver_private in default_clear() or setting >> num_private_objs to 0. >> It would remove the need for all null checks I think.. >> >> ~Maarten >> > Did you mean the NULL checks in this loop inside > drm_atomic_get_private_obj_state() > > + for (i = 0; i < state->num_private_objs; i++) > + if (obj == state->private_objs[i].obj && > + state->private_objs[i].obj_state) > + return state->private_objs[i].obj_state; > > and the fact that I am not setting num_private_objs = 0 in > drm_atomic_state_default_clear() ? All of them, the NULL check in default_clear goes away, same for get_private_obj_state, and __for_each_private_obj ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Wait for all fences before installing an exclusive clflush fence
Ensure that before we overwrite the reservation_object with our exclusive fence for the pending clflush operation, that we do wait upon all the fences in the current reservation_object. Fixes: 57822dc6b9cf ("drm/i915: Perform object clflushing asynchronously") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Matthew Auld --- drivers/gpu/drm/i915/i915_gem_clflush.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c index d925fb582ba7..ffd01e02fe94 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c @@ -168,7 +168,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, i915_sw_fence_await_reservation(&clflush->wait, obj->resv, NULL, - false, I915_FENCE_TIMEOUT, + true, I915_FENCE_TIMEOUT, GFP_KERNEL); reservation_object_lock(obj->resv, NULL); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Restore marking context objects as dirty on pinning
== Series Details == Series: drm/i915: Restore marking context objects as dirty on pinning URL : https://patchwork.freedesktop.org/series/21708/ State : failure == Summary == Series 21708v1 drm/i915: Restore marking context objects as dirty on pinning https://patchwork.freedesktop.org/api/1.0/series/21708/revisions/1/mbox/ Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-b-frame-sequence: pass -> INCOMPLETE (fi-byt-j1900) Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 457s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 452s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 582s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 536s fi-byt-j1900 total:226 pass:206 dwarn:0 dfail:0 fail:0 skip:19 time: 0s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 501s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 436s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 514s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 496s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 483s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 492s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 600s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 490s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 524s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 459s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 548s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 419s b1ecc49ac71e484430083e5b3b7cd75d4976a465 drm-tip: 2017y-03m-23d-07h-40m-49s UTC integration manifest de7a9a9 drm/i915: Restore marking context objects as dirty on pinning == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4266/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [GIT PULL] more GVT-g fixes for 4.11
Hi, Here's updated gvt fixes for 4.11. Major ones are for KVM reference fix, guest gtt partial update fix for very big guest mem, and Changbin's following fix for gvt context notification. Thanks. -- The following changes since commit 2958b9013fcbabeeba221161d0712f5259f1e15d: drm/i915/gvt: Fix gvt scheduler interval time (2017-03-17 16:46:45 +0800) are available in the git repository at: https://github.com/01org/gvt-linux.git tags/gvt-fixes-2017-03-23 for you to fetch changes up to bc2d4b62db67f817b09c782219996630e9c2f5e2: drm/i915/gvt: Use force single submit flag to distinguish gvt request from i915 request (2017-03-22 13:18:56 +0800) gvt-fixes-2017-03-23 - KVM reference fix from Alex - shadow gtt entry partial update fix from Xiaoguang - gvt context notification check (Changbin) - other misc fixes. Alex Williamson (1): drm/i915/kvmgt: Hold struct kvm reference Changbin Du (1): drm/i915/gvt: Use force single submit flag to distinguish gvt request from i915 request Chuanxiao Dong (1): drm/i915/gvt: fix wrong offset when loading RCS mocs Pei Zhang (1): drm/i915/gvt: add write handler for mmio mbctl Xiaoguang Chen (1): drm/i915/gvt: set shadow entry to scratch page while p2m failed Xu Han (1): drm/i915/gvt: Fix guest fail to read EDID leading to black guest console issue. drivers/gpu/drm/i915/gvt/edid.c | 3 ++- drivers/gpu/drm/i915/gvt/gtt.c | 8 ++-- drivers/gpu/drm/i915/gvt/handlers.c | 10 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 2 ++ drivers/gpu/drm/i915/gvt/render.c| 2 +- drivers/gpu/drm/i915/gvt/scheduler.c | 7 ++- 6 files changed, 26 insertions(+), 6 deletions(-) -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ 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: Remove superfluous hw_flags from mi_set_context()
== Series Details == Series: drm/i915: Remove superfluous hw_flags from mi_set_context() URL : https://patchwork.freedesktop.org/series/21709/ State : success == Summary == Series 21709v1 drm/i915: Remove superfluous hw_flags from mi_set_context() https://patchwork.freedesktop.org/api/1.0/series/21709/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 466s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 458s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 587s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 544s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 505s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 509s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 444s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 437s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 522s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 498s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 479s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 480s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 608s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 491s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 513s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 464s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 551s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time: 422s b1ecc49ac71e484430083e5b3b7cd75d4976a465 drm-tip: 2017y-03m-23d-07h-40m-49s UTC integration manifest add74ce drm/i915: Remove superfluous hw_flags from mi_set_context() == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4267/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for wire acquire ctx through legacy modeset paths
== Series Details == Series: wire acquire ctx through legacy modeset paths URL : https://patchwork.freedesktop.org/series/21710/ State : warning == Summary == Series 21710v1 wire acquire ctx through legacy modeset paths https://patchwork.freedesktop.org/api/1.0/series/21710/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload-final: pass -> DMESG-WARN (fi-skl-6770hq) fdo#100248 Test kms_busy: Subgroup basic-flip-default-a: pass -> DMESG-WARN (fi-bdw-gvtdvm) pass -> DMESG-WARN (fi-skl-gvtdvm) pass -> DMESG-WARN (fi-kbl-7500u) pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-ivb-3770) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-bxt-j4205) pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-byt-n2820) fdo#100117 pass -> DMESG-WARN (fi-ilk-650) pass -> DMESG-WARN (fi-hsw-4770) pass -> DMESG-WARN (fi-hsw-4770r) Subgroup basic-flip-default-b: pass -> DMESG-WARN (fi-bdw-gvtdvm) pass -> DMESG-WARN (fi-skl-gvtdvm) pass -> DMESG-WARN (fi-kbl-7500u) pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-ivb-3770) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-bxt-j4205) fdo#99728 pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-ilk-650) pass -> DMESG-WARN (fi-hsw-4770) pass -> DMESG-WARN (fi-hsw-4770r) Subgroup basic-flip-default-c: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-skl-gvtdvm) pass -> DMESG-WARN (fi-kbl-7500u) pass -> DMESG-WARN (fi-bdw-gvtdvm) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-ivb-3770) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-bxt-j4205) fdo#99728 pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-hsw-4770r) pass -> DMESG-WARN (fi-hsw-4770) Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-bsw-n3050) Subgroup basic-flip-after-cursor-legacy: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-bsw-n3050) Subgroup basic-flip-after-cursor-varying-size: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-byt-n2820) fdo#100094 pass -> DMESG-WARN (fi-bsw-n3050) Subgroup basic-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-byt-n2820) fdo#100094 pass -> DMESG-WARN (fi-bsw-n3050) Subgroup basic-flip-before-cursor-varying-size: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-byt-n2820) fdo#100094 pass -> DMESG-WARN (fi-bsw-n3050) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-bxt-j4205) pass -> DMESG-WARN (fi-kbl-7500u) fdo#100179 pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-ivb-3770) pass -> DMESG-WARN (fi-skl-
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: Skip unused fw_domains
Chris Wilson writes: > Use find-first-set bitop to quickly scan through the fw_domains mask and > skip iterating over unused domains. > > v2: Move the WARN into the caller, to prevent compiler warnings in > normal builds. > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_debugfs.c | 3 ++- > drivers/gpu/drm/i915/i915_drv.h | 30 +++ > drivers/gpu/drm/i915/intel_uncore.c | 48 > - > 3 files changed, 48 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 29bf11d8b620..fcac795d4396 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1461,9 +1461,10 @@ static int i915_forcewake_domains(struct seq_file *m, > void *data) > { > struct drm_i915_private *dev_priv = node_to_i915(m->private); > struct intel_uncore_forcewake_domain *fw_domain; > + unsigned int tmp; > > spin_lock_irq(&dev_priv->uncore.lock); > - for_each_fw_domain(fw_domain, dev_priv) { > + for_each_fw_domain(fw_domain, dev_priv, tmp) { > seq_printf(m, "%s.wake_count = %u\n", > intel_uncore_forcewake_domain_to_str(fw_domain->id), > fw_domain->wake_count); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4c9de7d00eaf..589f91c4e585 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -703,9 +703,9 @@ enum forcewake_domain_id { > }; > > enum forcewake_domains { > - FORCEWAKE_RENDER = (1 << FW_DOMAIN_ID_RENDER), > - FORCEWAKE_BLITTER = (1 << FW_DOMAIN_ID_BLITTER), > - FORCEWAKE_MEDIA = (1 << FW_DOMAIN_ID_MEDIA), > + FORCEWAKE_RENDER = BIT(FW_DOMAIN_ID_RENDER), > + FORCEWAKE_BLITTER = BIT(FW_DOMAIN_ID_BLITTER), > + FORCEWAKE_MEDIA = BIT(FW_DOMAIN_ID_MEDIA), > FORCEWAKE_ALL = (FORCEWAKE_RENDER | >FORCEWAKE_BLITTER | >FORCEWAKE_MEDIA) > @@ -790,15 +790,19 @@ struct intel_uncore { > int unclaimed_mmio_check; > }; > > +#define __mask_next_bit(mask) ({ \ > + int __idx = ffs(mask) - 1; \ > + mask &= ~BIT(__idx);\ > + __idx; \ > +}) > + > /* Iterate over initialised fw domains */ > -#define for_each_fw_domain_masked(domain__, mask__, dev_priv__) \ > - for ((domain__) = &(dev_priv__)->uncore.fw_domain[0]; \ > - (domain__) < &(dev_priv__)->uncore.fw_domain[FW_DOMAIN_ID_COUNT]; \ > - (domain__)++) \ > - for_each_if ((mask__) & (domain__)->mask) > +#define for_each_fw_domain_masked(domain__, mask__, dev_priv__, tmp__) \ > + for (tmp__ = (mask__); \ > + tmp__ ? (domain__ = > &(dev_priv__)->uncore.fw_domain[__mask_next_bit(tmp__)]), 1 : 0;) > > -#define for_each_fw_domain(domain__, dev_priv__) \ > - for_each_fw_domain_masked(domain__, FORCEWAKE_ALL, dev_priv__) > +#define for_each_fw_domain(domain__, dev_priv__, tmp__) \ > + for_each_fw_domain_masked(domain__, (dev_priv__)->uncore.fw_domains, > dev_priv__, tmp__) > > #define CSR_VERSION(major, minor)((major) << 16 | (minor)) > #define CSR_VERSION_MAJOR(version) ((version) >> 16) > @@ -2581,12 +2585,6 @@ static inline struct drm_i915_private > *huc_to_i915(struct intel_huc *huc) >(id__)++) \ > for_each_if ((engine__) = (dev_priv__)->engine[(id__)]) > > -#define __mask_next_bit(mask) ({ \ > - int __idx = ffs(mask) - 1; \ > - mask &= ~BIT(__idx);\ > - __idx; \ > -}) > - > /* Iterator over subset of engines selected by mask */ > #define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \ > for (tmp__ = mask__ & INTEL_INFO(dev_priv__)->ring_mask;\ > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index aa2e7401efdc..2e79ca129202 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -118,13 +118,16 @@ static void > fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains > fw_domains) > { > struct intel_uncore_forcewake_domain *d; > + unsigned int tmp; > + > + GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains); > > - for_each_fw_domain_masked(d, fw_domains, i915) { > + for_each_fw_domain_masked(d, fw_domains, i915, tmp) { > fw_domain_wait_ack_clear(i915, d); > fw_domain_get(i915, d); > } > > - for_each_fw_domain_masked(d, fw_domains, i915) > + for_each_fw_domain_masked(d, fw_domains, i915, tmp) >
Re: [Intel-gfx] [PATCH] drm/i915: Remove superfluous hw_flags from mi_set_context()
On 22/03/2017 21:03, Chris Wilson wrote: Why have both hw_flags and flags, when just one will do? Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_context.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 486051ed681d..8fc8b3d15a0f 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -576,25 +576,25 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) } static inline int -mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) +mi_set_context(struct drm_i915_gem_request *req, u32 flags) { struct drm_i915_private *dev_priv = req->i915; struct intel_engine_cs *engine = req->engine; enum intel_engine_id id; - u32 *cs, flags = hw_flags | MI_MM_SPACE_GTT; const int num_rings = /* Use an extended w/a on ivb+ if signalling from other rings */ i915.semaphores ? INTEL_INFO(dev_priv)->num_rings - 1 : 0; int len; + u32 *cs; - /* These flags are for resource streamer on HSW+ */ + flags |= MI_MM_SPACE_GTT; if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8) - flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN); - else if (INTEL_GEN(dev_priv) < 8) - flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN); - + /* These flags are for resource streamer on HSW+ */ + flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN; + else + flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN; len = 4; if (INTEL_GEN(dev_priv) >= 7) Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
On ke, 2017-03-22 at 14:34 +0800, Chuanxiao Dong wrote: > GVT request needs a manual mmio load/restore. Before GuC submit > a request, send notification to gvt for mmio loading. And after > the GuC finished this GVT request, notify gvt again for mmio > restore. This follows the usage when using execlists submission. > > v2: use context_status_change instead of execlists_context_status_change > for better understanding (ZhengXiao) > v3: remove the comment as it is obvious and not friendly to > the caller (Kevin) > > Cc: xiao.zh...@intel.com > Cc: kevin.t...@intel.com > Signed-off-by: Chuanxiao Dong > @@ -350,7 +335,7 @@ static void execlists_submit_ports(struct intel_engine_cs > *engine) > > GEM_BUG_ON(port[0].count > 1); > if (!port[0].count) > - execlists_context_status_change(port[0].request, > + context_status_change(port[0].request, > INTEL_CONTEXT_SCHEDULE_IN); Fix indent. > desc[0] = execlists_update_context(port[0].request); > GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); > @@ -358,7 +343,7 @@ static void execlists_submit_ports(struct intel_engine_cs > *engine) > > if (port[1].request) { > GEM_BUG_ON(port[1].count); > - execlists_context_status_change(port[1].request, > + context_status_change(port[1].request, > INTEL_CONTEXT_SCHEDULE_IN); Ditto. > @@ -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data) > if (--port[0].count == 0) { > GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED); > > GEM_BUG_ON(!i915_gem_request_completed(port[0].request)); > - execlists_context_status_change(port[0].request, > + context_status_change(port[0].request, > > INTEL_CONTEXT_SCHEDULE_OUT); Ditto. > @@ -87,5 +87,18 @@ uint64_t intel_lr_context_descriptor(struct > i915_gem_context *ctx, > /* Execlists */ > int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv, > int enable_execlists); > +static inline void > +context_status_change(struct drm_i915_gem_request *rq, unsigned long status) This needs intel_lr_ prefix now that it's in .h file. With those changes (make sure context_status_change doesn't become over character 80 line), this is; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore marking context objects as dirty on pinning
On 22/03/2017 20:59, Chris Wilson wrote: Commit e8a9c58fcd9a ("drm/i915: Unify active context tracking between legacy/execlists/guc") converted the legacy intel_ringbuffer submission to the same context pinning mechanism as execlists - that is to pin the context until the subsequent request is retired. Previously it used the vma retirement of the context object to keep itself pinned until the next request (after i915_vma_move_to_active()). In the conversion, I missed that the vma retirement was also responsible for marking the object as dirty. Mark the context object as dirty when pinning (equivalent to execlists) which ensures that if the context is swapped out due to mempressure or suspend/hibernation, when it is loaded back in it does so with the previous state (and not all zero). Fixes: e8a9c58fcd9a ("drm/i915: Unify active context tracking between legacy/execlists/guc") Reported-by: Dennis Gilmore Reported-by: Mathieu Marquer Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100181 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: # v4.11-rc1 --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0ca5ea7a9407..62756eb2bd4a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1440,6 +1440,8 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine, ret = context_pin(ctx); if (ret) goto error; + + ce->state->obj->mm.dirty = true; } /* The kernel context is only used as a placeholder for flushing the Reviewed-by: Tvrtko Ursulin Could it be related to 99671, it also mentions suspend? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
On Thu, Mar 23, 2017 at 11:37:32AM +0200, Joonas Lahtinen wrote: > On ke, 2017-03-22 at 14:34 +0800, Chuanxiao Dong wrote: > > @@ -87,5 +87,18 @@ uint64_t intel_lr_context_descriptor(struct > > i915_gem_context *ctx, > > /* Execlists */ > > int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv, > > int enable_execlists); > > +static inline void > > +context_status_change(struct drm_i915_gem_request *rq, unsigned long > > status) > > This needs intel_lr_ prefix now that it's in .h file. But not in this header. The event is not strictly tied to execlists, it consumes request for starters - but on the other hand it is gvt specific. I'm tempted to say intel_gvt_notify_context_status(). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore marking context objects as dirty on pinning
On Thu, Mar 23, 2017 at 09:41:30AM +, Tvrtko Ursulin wrote: > > On 22/03/2017 20:59, Chris Wilson wrote: > >Commit e8a9c58fcd9a ("drm/i915: Unify active context tracking between > >legacy/execlists/guc") converted the legacy intel_ringbuffer submission > >to the same context pinning mechanism as execlists - that is to pin the > >context until the subsequent request is retired. Previously it used the > >vma retirement of the context object to keep itself pinned until the > >next request (after i915_vma_move_to_active()). In the conversion, I > >missed that the vma retirement was also responsible for marking the > >object as dirty. Mark the context object as dirty when pinning > >(equivalent to execlists) which ensures that if the context is swapped > >out due to mempressure or suspend/hibernation, when it is loaded back in > >it does so with the previous state (and not all zero). > > > >Fixes: e8a9c58fcd9a ("drm/i915: Unify active context tracking between > >legacy/execlists/guc") > >Reported-by: Dennis Gilmore > >Reported-by: Mathieu Marquer > >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3 > >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100181 > >Signed-off-by: Chris Wilson > >Cc: Tvrtko Ursulin > >Cc: # v4.11-rc1 > >--- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > >diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > >b/drivers/gpu/drm/i915/intel_ringbuffer.c > >index 0ca5ea7a9407..62756eb2bd4a 100644 > >--- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >@@ -1440,6 +1440,8 @@ static int intel_ring_context_pin(struct > >intel_engine_cs *engine, > > ret = context_pin(ctx); > > if (ret) > > goto error; > >+ > >+ce->state->obj->mm.dirty = true; > > } > > > > /* The kernel context is only used as a placeholder for flushing the > > > > Reviewed-by: Tvrtko Ursulin > > Could it be related to 99671, it also mentions suspend? No, I do not believe so. The symptoms do not match (but that may just be snb handling the failure differently) but more tellingly it started (or at least based on the bug reports became more prevalent) before the breakage was introduced. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
On 22/03/2017 06:34, Chuanxiao Dong wrote: GVT request needs a manual mmio load/restore. Before GuC submit a request, send notification to gvt for mmio loading. And after the GuC finished this GVT request, notify gvt again for mmio restore. This follows the usage when using execlists submission. v2: use context_status_change instead of execlists_context_status_change for better understanding (ZhengXiao) v3: remove the comment as it is obvious and not friendly to the caller (Kevin) Cc: xiao.zh...@intel.com Cc: kevin.t...@intel.com Signed-off-by: Chuanxiao Dong --- drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++ drivers/gpu/drm/i915/intel_lrc.c | 21 +++-- drivers/gpu/drm/i915/intel_lrc.h | 13 + 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 055467a..0195547 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq) unsigned long flags; int b_ret; + context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); + /* WA to flush out the pending GMADR writes to ring buffer. */ if (i915_vma_is_map_and_fenceable(rq->ring->vma)) POSTING_READ_FW(GUC_STATUS); @@ -634,6 +636,7 @@ static void i915_guc_irq_handler(unsigned long data) rq = port[0].request; while (rq && i915_gem_request_completed(rq)) { trace_i915_gem_request_out(rq); + context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT); Does GVT care that the context will still be active on the GPU for a small window after this notification? (User interrupt happens before context complete, which GuC hides from the driver.) Regards, Tvrtko i915_gem_request_put(rq); port[0].request = port[1].request; port[1].request = NULL; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index eec1e71..24c69b5 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, return ctx->engine[engine->id].lrc_desc; } -static inline void -execlists_context_status_change(struct drm_i915_gem_request *rq, - unsigned long status) -{ - /* -* Only used when GVT-g is enabled now. When GVT-g is disabled, -* The compiler should eliminate this function as dead-code. -*/ - if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) - return; - - atomic_notifier_call_chain(&rq->engine->context_status_notifier, - status, rq); -} - static void execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state) { @@ -350,7 +335,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) GEM_BUG_ON(port[0].count > 1); if (!port[0].count) - execlists_context_status_change(port[0].request, + context_status_change(port[0].request, INTEL_CONTEXT_SCHEDULE_IN); desc[0] = execlists_update_context(port[0].request); GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); @@ -358,7 +343,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) if (port[1].request) { GEM_BUG_ON(port[1].count); - execlists_context_status_change(port[1].request, + context_status_change(port[1].request, INTEL_CONTEXT_SCHEDULE_IN); desc[1] = execlists_update_context(port[1].request); GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1])); @@ -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data) if (--port[0].count == 0) { GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED); GEM_BUG_ON(!i915_gem_request_completed(port[0].request)); - execlists_context_status_change(port[0].request, + context_status_change(port[0].request, INTEL_CONTEXT_SCHEDULE_OUT); trace_i915_gem_request_out(port[0].request); diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index e8015e7..51e1be9 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -87,5 +87,18 @@ uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, /* Execlists */ int intel_sanitize_enable_execlists(struct drm_i915_private *de
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Actually pass the reclaim gfp_t along to shmemfs!
== Series Details == Series: drm/i915: Actually pass the reclaim gfp_t along to shmemfs! URL : https://patchwork.freedesktop.org/series/21721/ State : success == Summary == Series 21721v1 drm/i915: Actually pass the reclaim gfp_t along to shmemfs! https://patchwork.freedesktop.org/api/1.0/series/21721/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 459s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 463s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 571s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 539s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 509s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 507s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 442s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 440s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 525s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 497s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 486s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 486s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 606s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 495s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 520s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 468s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 549s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time: 423s b1ecc49ac71e484430083e5b3b7cd75d4976a465 drm-tip: 2017y-03m-23d-07h-40m-49s UTC integration manifest ff1cc80 drm/i915: Actually pass the reclaim gfp_t along to shmemfs! == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4270/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Actually pass the reclaim gfp_t along to shmemfs!
On Thu, Mar 23, 2017 at 09:29:47AM +0200, Joonas Lahtinen wrote: > On ke, 2017-03-22 at 22:34 +, Chris Wilson wrote: > > Words cannot describe the embarrassment at creating a new gfp_t relaim to > > only prevent the oomkiller but allow direct|kswapd reclaim, and then not > > use it in the shmem_read_mapping_page_gfp(). > > > > Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur > > the oom for gfx allocations") > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Cc: Daniel Vetter > > Reviewed-by: Joonas Lahtinen Thanks for not being smug, pushed. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Restore marking context objects as dirty on pinning
On Thu, Mar 23, 2017 at 09:03:30AM -, Patchwork wrote: > == Series Details == > > Series: drm/i915: Restore marking context objects as dirty on pinning > URL : https://patchwork.freedesktop.org/series/21708/ > State : failure > > == Summary == > > Series 21708v1 drm/i915: Restore marking context objects as dirty on pinning > https://patchwork.freedesktop.org/api/1.0/series/21708/revisions/1/mbox/ > > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-b-frame-sequence: > pass -> INCOMPLETE (fi-byt-j1900) > Subgroup suspend-read-crc-pipe-a: > fail -> PASS (fi-skl-6700k) Lalalala, pushed anyway. Thanks for the review. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove superfluous hw_flags from mi_set_context()
On Thu, Mar 23, 2017 at 09:37:35AM +, Tvrtko Ursulin wrote: > > On 22/03/2017 21:03, Chris Wilson wrote: > >Why have both hw_flags and flags, when just one will do? > > > >Signed-off-by: Chris Wilson > >--- > > drivers/gpu/drm/i915/i915_gem_context.c | 14 +++--- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > >b/drivers/gpu/drm/i915/i915_gem_context.c > >index 486051ed681d..8fc8b3d15a0f 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_context.c > >+++ b/drivers/gpu/drm/i915/i915_gem_context.c > >@@ -576,25 +576,25 @@ void i915_gem_context_close(struct drm_device *dev, > >struct drm_file *file) > > } > > > > static inline int > >-mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) > >+mi_set_context(struct drm_i915_gem_request *req, u32 flags) > > { > > struct drm_i915_private *dev_priv = req->i915; > > struct intel_engine_cs *engine = req->engine; > > enum intel_engine_id id; > >-u32 *cs, flags = hw_flags | MI_MM_SPACE_GTT; > > const int num_rings = > > /* Use an extended w/a on ivb+ if signalling from other rings */ > > i915.semaphores ? > > INTEL_INFO(dev_priv)->num_rings - 1 : > > 0; > > int len; > >+u32 *cs; > > > >-/* These flags are for resource streamer on HSW+ */ > >+flags |= MI_MM_SPACE_GTT; > > if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8) > >-flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN); > >-else if (INTEL_GEN(dev_priv) < 8) > >-flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN); > >- > >+/* These flags are for resource streamer on HSW+ */ > >+flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN; > >+else > >+flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN; > > > > len = 4; > > if (INTEL_GEN(dev_priv) >= 7) > > > > Reviewed-by: Tvrtko Ursulin Thanks, those brackets had irked me for a long time :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 2/7] dim: abstract and fix maintainer scope check for dinq
Separate maintainer scope check from checkpatch, and only do the check on applying patches. Also, fix the failures on grep not matching. Fixes: 56e53a49e28f ("dim: declare and assign separately") Signed-off-by: Jani Nikula --- dim | 33 - 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/dim b/dim index ddcc18f17f0d..268bc6ca280d 100755 --- a/dim +++ b/dim @@ -698,6 +698,7 @@ function dim_apply_branch fi checkpatch_commit HEAD + check_maintainer $branch HEAD eval $DRY $DIM_POST_APPLY_ACTION } @@ -1080,6 +1081,25 @@ function dim_conf dim_checkout drm-intel-next-fixes "$@" } +# $1 branch +# $2 commit +function check_maintainer +{ + local branch commit + + branch=$1 + commit=$2 + +if [ "$branch" = "drm-intel-next-queued" ]; then + if non_i915_files=$(git diff-tree --no-commit-id --name-only -r $commit | \ + grep -v "^\(drivers/gpu/drm/i915/\|include/drm/i915\|include/uapi/drm/i915\)") && [[ -n "$non_i915_files" ]]; then + echo -e "The following files are outside of i915 maintenance scope:\n" + echo "$non_i915_files" + echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files." + fi + fi +} + # $1 is the git sha1 to check function checkpatch_commit { @@ -1094,19 +1114,6 @@ function checkpatch_commit if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then warn_or_fail "New BUG macro added" fi - -if [ "$branch" = "drm-intel-next-queued" ]; then - # FIXME: this relies on local assignment not failing on command - # substitution failures - non_i915_files=$(git diff-tree --no-commit-id --name-only -r $commit | \ - grep -v "^\(drivers/gpu/drm/i915/\|include/drm/i915\|include/uapi/drm/i915\)") - - if [ -n "$non_i915_files" ]; then - echo -e "The following files are outside of i915 maintenance scope:\n" - echo "$non_i915_files" - echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files." - fi - fi } # turn $1 in to a git commit range -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 7/7] dim: propagate errors from check_maintainer
Signed-off-by: Jani Nikula --- dim | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dim b/dim index 7b6275e7796f..989674ab7a91 100755 --- a/dim +++ b/dim @@ -701,7 +701,9 @@ function dim_apply_branch if ! checkpatch_commit HEAD; then rv=1 fi - check_maintainer $branch HEAD + if ! check_maintainer $branch HEAD; then + rv=1 + fi eval $DRY $DIM_POST_APPLY_ACTION @@ -1090,7 +1092,7 @@ function dim_conf # $2 commit function check_maintainer { - local branch commit + local branch commit rv branch=$1 commit=$2 @@ -1101,8 +1103,11 @@ function check_maintainer echo -e "The following files are outside of i915 maintenance scope:\n" echo "$non_i915_files" echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files." + rv=1 fi fi + + return $rv } # $1 is the git sha1 to check -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 4/7] dim: return exit code from checkpatch_commit
Return the exit code to let callers make better decisions on what to do. Signed-off-by: Jani Nikula --- dim | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dim b/dim index e6943d34c43f..c282782b429d 100755 --- a/dim +++ b/dim @@ -697,7 +697,7 @@ function dim_apply_branch echo "No message-id found in the patch file." fi - checkpatch_commit HEAD + checkpatch_commit HEAD || true check_maintainer $branch HEAD eval $DRY $DIM_POST_APPLY_ACTION @@ -893,7 +893,7 @@ dim_alias_ar=apply-resolved function dim_apply_resolved { make $DIM_MAKE_OPTIONS && git add -u && git am --resolved - checkpatch_commit HEAD + checkpatch_commit HEAD || true eval $DRY $DIM_POST_APPLY_ACTION } @@ -1103,17 +1103,22 @@ function check_maintainer # $1 is the git sha1 to check function checkpatch_commit { - local commit cmd bug_lines non_i915_files + local commit cmd bug_lines non_i915_files rv commit=$1 cmd="git show --pretty=email $commit" git --no-pager log --oneline -1 $commit - $cmd | scripts/checkpatch.pl -q --emacs --strict - || true + if ! $cmd | scripts/checkpatch.pl -q --emacs --strict -; then + rv=1 + fi if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then echoerr "WARNING: New BUG macro added" + rv=1 fi + + return $rv } # turn $1 in to a git commit range -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 1/7] dim: don't fail on grep not matching
Oops, the comment told us to watch out for this. Fixes: 56e53a49e28f ("dim: declare and assign separately") Signed-off-by: Jani Nikula --- dim | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dim b/dim index c1ac9e546ea9..ddcc18f17f0d 100755 --- a/dim +++ b/dim @@ -1091,10 +1091,7 @@ function checkpatch_commit git --no-pager log --oneline -1 $commit $cmd | scripts/checkpatch.pl -q --emacs --strict - || true - # FIXME: this relies on local assignment not failing on command - # substitution failures - bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') - if test "$bug_lines" -eq 1; then + if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then warn_or_fail "New BUG macro added" fi -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 5/7] dim: make dim checkpatch subcommand return exit codes
Signed-off-by: Jani Nikula --- dim | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dim b/dim index c282782b429d..e23617c8907c 100755 --- a/dim +++ b/dim @@ -1179,13 +1179,17 @@ dim_alias_check_patch=checkpatch dim_alias_cp=checkpatch function dim_checkpatch { - local range + local range rv range=$(rangeish "$1") for commit in $(git rev-list --reverse $range); do - checkpatch_commit $commit || true + if ! checkpatch_commit $commit; then + rv=1 + fi done + + return $rv } function dim_sparse -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 6/7] dim: make dim apply-branch subcommand return exit codes
For example, piping from emacs only pops up the stdout/stderr buffer when the exit code is non-zero. Signed-off-by: Jani Nikula --- dim | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dim b/dim index e23617c8907c..7b6275e7796f 100755 --- a/dim +++ b/dim @@ -667,7 +667,7 @@ dim_alias_ab=apply-branch dim_alias_sob=apply-branch function dim_apply_branch { - local branch file message_id commiter_email patch_from sob + local branch file message_id commiter_email patch_from sob rv branch=$1 shift @@ -694,13 +694,18 @@ function dim_apply_branch if [ -n "$message_id" ]; then dim_commit_add_tag "Link: http://patchwork.freedesktop.org/patch/msgid/$message_id"; else - echo "No message-id found in the patch file." + echoerr "WARNING: No message-id found in the patch file." + rv=1 fi - checkpatch_commit HEAD || true + if ! checkpatch_commit HEAD; then + rv=1 + fi check_maintainer $branch HEAD eval $DRY $DIM_POST_APPLY_ACTION + + return $rv } function dim_add_link -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [dim PATCH 3/7] dim: only issue warnings on new BUG macros
We apply the patch regardless of the warn_or_fail result, so just switch to using an error message. Signed-off-by: Jani Nikula --- dim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dim b/dim index 268bc6ca280d..e6943d34c43f 100755 --- a/dim +++ b/dim @@ -1112,7 +1112,7 @@ function checkpatch_commit $cmd | scripts/checkpatch.pl -q --emacs --strict - || true if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then - warn_or_fail "New BUG macro added" + echoerr "WARNING: New BUG macro added" fi } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: Skip unused fw_domains
On Thu, Mar 23, 2017 at 11:36:53AM +0200, Mika Kuoppala wrote: > Chris Wilson writes: > > @@ -1253,9 +1269,9 @@ static void intel_uncore_fw_domains_init(struct > > drm_i915_private *dev_priv) > >FORCEWAKE_MT, FORCEWAKE_MT_ACK); > > > > spin_lock_irq(&dev_priv->uncore.lock); > > - fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL); > > + fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER); > > Well we are skipping here the unused ones so I guess it fits > to the commit desc. This patch added the bug on to catch use with an invalid fw_domain (that would have just been iterated over previously). I suppose I could have split this chunk into a prep patch... "In the next patch we will begin to sanity check that we do not attempt to obtain the forcewake on an unsupport domain. However, that is exactly what we do during our actual initialisation of fw_domains - rectify it before it explodes." -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/19] drm/vmwgfx: Drop the cursor locking hack
On Thu, Mar 23, 2017 at 09:35:25AM +0100, Thomas Hellstrom wrote: > Hi, Daniel, > > On 03/23/2017 08:31 AM, Daniel Vetter wrote: > > On Thu, Mar 23, 2017 at 08:28:32AM +0100, Daniel Vetter wrote: > >> On Thu, Mar 23, 2017 at 07:22:31AM +0100, Thomas Hellstrom wrote: > >>> On 03/22/2017 10:50 PM, Daniel Vetter wrote: > It's been around forever, no one bothered to address the FIXME, so I > presume it's all fine. > > Cc: Sinclair Yeh > Cc: Thomas Hellstrom > Signed-off-by: Daniel Vetter > >>> NAK. We need to properly address this. Probably as part of the atomic > >>> update. > >> So could someone with vmwgfx understanding explain this? Note that the > >> FIXME was originally added by me years ago, because I wasn't sure (only > >> about 90%) that this is safe, and was essentially pleading for a vmwgfx > >> expert to review this? > >> > >> Since it didn't happen I presume it's not that terribly and probably safe > >> ... > >> > >> I'm still 90% sure that this is correct, but I'd love for a vmwgfx to > >> audit it. Replying with a NAK is kinda not the response I was hoping for > >> (and yes I guess I should have explained what's going on here better, but > >> it's just a git blame of the FIXME comment away). > > So the code has been left in place because it works. Altering it now > will create unnecessary merge conflicts with the atomic code, and the > change isn't tested and audited which means we need to drop focus from > what we're doing and audit and test code that isn't going to be used > anyway for not apparent reason? But otoh put in the below context there > indeed is a reason. > > From a quick audit of the existing code it seems like at least > vmw_cursor_update_position is touching global device state so I think at > a minimum we need to take a spinlock in that function. Otherwise it > seems to be safe. Note that you're holding the crtc lock already, which gives you exclusion against concurrent page_flips, mode_sets and property changes. Note also that page_flips themselves also only hold the crtc lock, so you can run multiple page_flips in parallel on different crtc (iirc vmwgfx has multiple crtc, if not this discussion is entirely moot). tbh I'd be surprised if my patch really breaks something that hasn't been a pre-existing issue for a long time. The original commit which added this FIXME comment is from 2012. Note also that because it's a hack, you already have a pretty a real race with the core drm state keeping, and no one seems to have hit that either. I mean I can dig through vmwgfx code and do the audit, but it'll take a few hours and vmwgfx is it's own world, so much harder to understand (for me). > But I prefer if we can do that as part of the atomic update? When does that vmwgfx atomic happen? -Daniel -- 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
[Intel-gfx] ✓ Fi.CI.BAT: success for Enhancement to intel_dp_aux_backlight driver (rev2)
== Series Details == Series: Enhancement to intel_dp_aux_backlight driver (rev2) URL : https://patchwork.freedesktop.org/series/21086/ State : success == Summary == Series 21086v2 Enhancement to intel_dp_aux_backlight driver https://patchwork.freedesktop.org/api/1.0/series/21086/revisions/2/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 462s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 464s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 582s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 552s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 506s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 499s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 440s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 435s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 517s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 502s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 491s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 486s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 603s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 480s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 518s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 458s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 553s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 416s b1ecc49ac71e484430083e5b3b7cd75d4976a465 drm-tip: 2017y-03m-23d-07h-40m-49s UTC integration manifest 3f57180 drm/i915: Set PWM divider to match desired frequency in vbt e1c5fc5 drm: Add definition for eDP backlight frequency cd5652c drm/i915: Store brightness level in aux backlight driver 63c8dbb drm/i915: Support dynamic backlight via DPCD register 9e1d371 drm/i915: Correctly enable blacklight adjustment via DPCD ad5adc3 drm/i915: Add DPCD preferred mode for backlight control == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4271/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 1/7] drm/i915: Eliminate per-fw_domain i915 backpointer
Pass along the drm_i915_private pointer from the caller, rather than looking it up from each fw_domain during fw_domains_get/_put. This allows us to then eliminate the backpointer, in exchange for a more complicated unwrapping procedure in the rare intel_uncore_fw_release_timer(). Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.h | 33 drivers/gpu/drm/i915/intel_uncore.c | 77 - 2 files changed, 60 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a5947a496d0a..4c9de7d00eaf 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -732,21 +732,25 @@ intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv, struct intel_uncore_funcs { void (*force_wake_get)(struct drm_i915_private *dev_priv, - enum forcewake_domains domains); + enum forcewake_domains domains); void (*force_wake_put)(struct drm_i915_private *dev_priv, - enum forcewake_domains domains); - - uint8_t (*mmio_readb)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace); - uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace); - uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace); - uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace); - - void (*mmio_writeb)(struct drm_i915_private *dev_priv, i915_reg_t r, - uint8_t val, bool trace); - void (*mmio_writew)(struct drm_i915_private *dev_priv, i915_reg_t r, - uint16_t val, bool trace); - void (*mmio_writel)(struct drm_i915_private *dev_priv, i915_reg_t r, - uint32_t val, bool trace); + enum forcewake_domains domains); + + uint8_t (*mmio_readb)(struct drm_i915_private *dev_priv, + i915_reg_t r, bool trace); + uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, + i915_reg_t r, bool trace); + uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, + i915_reg_t r, bool trace); + uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, + i915_reg_t r, bool trace); + + void (*mmio_writeb)(struct drm_i915_private *dev_priv, + i915_reg_t r, uint8_t val, bool trace); + void (*mmio_writew)(struct drm_i915_private *dev_priv, + i915_reg_t r, uint16_t val, bool trace); + void (*mmio_writel)(struct drm_i915_private *dev_priv, + i915_reg_t r, uint32_t val, bool trace); }; struct intel_forcewake_range { @@ -771,7 +775,6 @@ struct intel_uncore { enum forcewake_domains fw_domains_active; struct intel_uncore_forcewake_domain { - struct drm_i915_private *i915; enum forcewake_domain_id id; enum forcewake_domains mask; unsigned wake_count; diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 09f5f02d7901..aa2e7401efdc 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -52,10 +52,11 @@ intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id) } static inline void -fw_domain_reset(const struct intel_uncore_forcewake_domain *d) +fw_domain_reset(struct drm_i915_private *i915, + const struct intel_uncore_forcewake_domain *d) { WARN_ON(!i915_mmio_reg_valid(d->reg_set)); - __raw_i915_write32(d->i915, d->reg_set, d->val_reset); + __raw_i915_write32(i915, d->reg_set, d->val_reset); } static inline void @@ -69,9 +70,10 @@ fw_domain_arm_timer(struct intel_uncore_forcewake_domain *d) } static inline void -fw_domain_wait_ack_clear(const struct intel_uncore_forcewake_domain *d) +fw_domain_wait_ack_clear(struct drm_i915_private *i915, +const struct intel_uncore_forcewake_domain *d) { - if (wait_for_atomic((__raw_i915_read32(d->i915, d->reg_ack) & + if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) & FORCEWAKE_KERNEL) == 0, FORCEWAKE_ACK_TIMEOUT_MS)) DRM_ERROR("%s: timed out waiting for forcewake ack to clear.\n", @@ -79,15 +81,17 @@ fw_domain_wait_ack_clear(const struct intel_uncore_forcewake_domain *d) } static inline void -fw_domain_get(const struct intel_uncore_forcewake_domain *d) +fw_domain_get(struct drm_i915_private *i915, + const struct intel_uncore_forcewake_domain *d) { - __raw_i915_write32(d->i915, d-
[Intel-gfx] [CI 4/7] drm/i915: Skip unused fw_domains
Use find-first-set bitop to quickly scan through the fw_domains mask and skip iterating over unused domains. v2: Move the WARN into the caller, to prevent compiler warnings in normal builds. Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_debugfs.c | 3 ++- drivers/gpu/drm/i915/i915_drv.h | 30 +- drivers/gpu/drm/i915/intel_uncore.c | 42 + 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 29bf11d8b620..fcac795d4396 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1461,9 +1461,10 @@ static int i915_forcewake_domains(struct seq_file *m, void *data) { struct drm_i915_private *dev_priv = node_to_i915(m->private); struct intel_uncore_forcewake_domain *fw_domain; + unsigned int tmp; spin_lock_irq(&dev_priv->uncore.lock); - for_each_fw_domain(fw_domain, dev_priv) { + for_each_fw_domain(fw_domain, dev_priv, tmp) { seq_printf(m, "%s.wake_count = %u\n", intel_uncore_forcewake_domain_to_str(fw_domain->id), fw_domain->wake_count); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4c9de7d00eaf..589f91c4e585 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -703,9 +703,9 @@ enum forcewake_domain_id { }; enum forcewake_domains { - FORCEWAKE_RENDER = (1 << FW_DOMAIN_ID_RENDER), - FORCEWAKE_BLITTER = (1 << FW_DOMAIN_ID_BLITTER), - FORCEWAKE_MEDIA = (1 << FW_DOMAIN_ID_MEDIA), + FORCEWAKE_RENDER = BIT(FW_DOMAIN_ID_RENDER), + FORCEWAKE_BLITTER = BIT(FW_DOMAIN_ID_BLITTER), + FORCEWAKE_MEDIA = BIT(FW_DOMAIN_ID_MEDIA), FORCEWAKE_ALL = (FORCEWAKE_RENDER | FORCEWAKE_BLITTER | FORCEWAKE_MEDIA) @@ -790,15 +790,19 @@ struct intel_uncore { int unclaimed_mmio_check; }; +#define __mask_next_bit(mask) ({ \ + int __idx = ffs(mask) - 1; \ + mask &= ~BIT(__idx);\ + __idx; \ +}) + /* Iterate over initialised fw domains */ -#define for_each_fw_domain_masked(domain__, mask__, dev_priv__) \ - for ((domain__) = &(dev_priv__)->uncore.fw_domain[0]; \ -(domain__) < &(dev_priv__)->uncore.fw_domain[FW_DOMAIN_ID_COUNT]; \ -(domain__)++) \ - for_each_if ((mask__) & (domain__)->mask) +#define for_each_fw_domain_masked(domain__, mask__, dev_priv__, tmp__) \ + for (tmp__ = (mask__); \ +tmp__ ? (domain__ = &(dev_priv__)->uncore.fw_domain[__mask_next_bit(tmp__)]), 1 : 0;) -#define for_each_fw_domain(domain__, dev_priv__) \ - for_each_fw_domain_masked(domain__, FORCEWAKE_ALL, dev_priv__) +#define for_each_fw_domain(domain__, dev_priv__, tmp__) \ + for_each_fw_domain_masked(domain__, (dev_priv__)->uncore.fw_domains, dev_priv__, tmp__) #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) #define CSR_VERSION_MAJOR(version) ((version) >> 16) @@ -2581,12 +2585,6 @@ static inline struct drm_i915_private *huc_to_i915(struct intel_huc *huc) (id__)++) \ for_each_if ((engine__) = (dev_priv__)->engine[(id__)]) -#define __mask_next_bit(mask) ({ \ - int __idx = ffs(mask) - 1; \ - mask &= ~BIT(__idx);\ - __idx; \ -}) - /* Iterator over subset of engines selected by mask */ #define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \ for (tmp__ = mask__ & INTEL_INFO(dev_priv__)->ring_mask;\ diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 4ad5751a3faf..2e79ca129202 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -118,13 +118,16 @@ static void fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *d; + unsigned int tmp; + + GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains); - for_each_fw_domain_masked(d, fw_domains, i915) { + for_each_fw_domain_masked(d, fw_domains, i915, tmp) { fw_domain_wait_ack_clear(i915, d); fw_domain_get(i915, d); } - for_each_fw_domain_masked(d, fw_domains, i915) + for_each_fw_domain_masked(d, fw_domains, i915, tmp) fw_domain_wait_ack(i915, d); i915->uncore.fw_domains_active |= fw_domains; @@ -134,8 +137,11 @@
[Intel-gfx] [CI 3/7] drm/i915: Use correct fw_domains during reset
In the next patch we will begin to sanity check that we do not attempt to obtain the forcewake on an unsupport domain. However, that is exactly what we do during reset of the fw_domains - rectify it before it explodes. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_uncore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 6e415e8a3aa3..4ad5751a3faf 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -308,7 +308,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, if (fw) dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); - fw_domains_reset(dev_priv, FORCEWAKE_ALL); + fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains); if (restore) { /* If reset with a user forcewake, try to restore */ if (fw) -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 5/7] drm/i915: Remove posting-read for forcewake put
We can relax the requirement upon ourselves that the forcewake is released immediately and just allow it to occur naturally following our mmio request. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_uncore.c | 33 + 2 files changed, 1 insertion(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 589f91c4e585..bb0e89d6255c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -783,7 +783,6 @@ struct intel_uncore { u32 val_set; u32 val_clear; i915_reg_t reg_ack; - i915_reg_t reg_post; u32 val_reset; } fw_domain[FW_DOMAIN_ID_COUNT]; diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 2e79ca129202..5464c0418bab 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -105,15 +105,6 @@ fw_domain_put(struct drm_i915_private *i915, __raw_i915_write32(i915, d->reg_set, d->val_clear); } -static inline void -fw_domain_posting_read(struct drm_i915_private *i915, - const struct intel_uncore_forcewake_domain *d) -{ - /* something from same cacheline, but not from the set register */ - if (i915_mmio_reg_valid(d->reg_post)) - __raw_posting_read(i915, d->reg_post); -} - static void fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains) { @@ -141,28 +132,13 @@ fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains) GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains); - for_each_fw_domain_masked(d, fw_domains, i915, tmp) { + for_each_fw_domain_masked(d, fw_domains, i915, tmp) fw_domain_put(i915, d); - fw_domain_posting_read(i915, d); - } i915->uncore.fw_domains_active &= ~fw_domains; } static void -fw_domains_posting_read(struct drm_i915_private *i915) -{ - struct intel_uncore_forcewake_domain *d; - unsigned int tmp; - - /* No need to do for all, just do for first found */ - for_each_fw_domain(d, i915, tmp) { - fw_domain_posting_read(i915, d); - break; - } -} - -static void fw_domains_reset(struct drm_i915_private *i915, enum forcewake_domains fw_domains) { @@ -176,8 +152,6 @@ fw_domains_reset(struct drm_i915_private *i915, for_each_fw_domain_masked(d, fw_domains, i915, tmp) fw_domain_reset(i915, d); - - fw_domains_posting_read(i915); } static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv) @@ -1180,11 +1154,6 @@ static void fw_domain_init(struct drm_i915_private *dev_priv, d->val_clear = _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL); } - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) - d->reg_post = FORCEWAKE_ACK_VLV; - else if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) - d->reg_post = ECOBUS; - d->id = domain_id; BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER)); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 2/7] drm/i915: Use correct fw_domains during initialisation
In the next patch we will begin to sanity check that we do not attempt to obtain the forcewake on an unsupport domain. However, that is exactly what we do during our actual initialisation of fw_domains - rectify it before it explodes. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_uncore.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index aa2e7401efdc..6e415e8a3aa3 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1253,9 +1253,9 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv) FORCEWAKE_MT, FORCEWAKE_MT_ACK); spin_lock_irq(&dev_priv->uncore.lock); - fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL); + fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER); ecobus = __raw_i915_read32(dev_priv, ECOBUS); - fw_domains_put_with_fifo(dev_priv, FORCEWAKE_ALL); + fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER); spin_unlock_irq(&dev_priv->uncore.lock); if (!(ecobus & FORCEWAKE_MT_ENABLE)) { -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 7/7] drm/i915: Drop uncore spinlock for reading debugfs forcewake counters
The set of available structs is not protected by the spinlock, and for the single read we can use READ_ONCE instead. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_debugfs.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fcac795d4396..f30591f44d3a 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1459,17 +1459,14 @@ static int ironlake_drpc_info(struct seq_file *m) static int i915_forcewake_domains(struct seq_file *m, void *data) { - struct drm_i915_private *dev_priv = node_to_i915(m->private); + struct drm_i915_private *i915 = node_to_i915(m->private); struct intel_uncore_forcewake_domain *fw_domain; unsigned int tmp; - spin_lock_irq(&dev_priv->uncore.lock); - for_each_fw_domain(fw_domain, dev_priv, tmp) { + for_each_fw_domain(fw_domain, i915, tmp) seq_printf(m, "%s.wake_count = %u\n", intel_uncore_forcewake_domain_to_str(fw_domain->id), - fw_domain->wake_count); - } - spin_unlock_irq(&dev_priv->uncore.lock); + READ_ONCE(fw_domain->wake_count)); return 0; } -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 6/7] drm/i915: All fw_domains share the same set/clear/reset values
Since we reuse the same values for each fw_domain, move them onto uncore. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.h | 11 +- drivers/gpu/drm/i915/intel_uncore.c | 40 +++-- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bb0e89d6255c..2911c49113b0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -774,16 +774,17 @@ struct intel_uncore { enum forcewake_domains fw_domains; enum forcewake_domains fw_domains_active; + u32 fw_set; + u32 fw_clear; + u32 fw_reset; + struct intel_uncore_forcewake_domain { enum forcewake_domain_id id; enum forcewake_domains mask; unsigned wake_count; struct hrtimer timer; i915_reg_t reg_set; - u32 val_set; - u32 val_clear; i915_reg_t reg_ack; - u32 val_reset; } fw_domain[FW_DOMAIN_ID_COUNT]; int unclaimed_mmio_check; @@ -3956,14 +3957,14 @@ u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, #define POSTING_READ16(reg)(void)I915_READ16_NOTRACE(reg) #define __raw_read(x, s) \ -static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \ +static inline uint##x##_t __raw_i915_read##x(const struct drm_i915_private *dev_priv, \ i915_reg_t reg) \ { \ return read##s(dev_priv->regs + i915_mmio_reg_offset(reg)); \ } #define __raw_write(x, s) \ -static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \ +static inline void __raw_i915_write##x(const struct drm_i915_private *dev_priv, \ i915_reg_t reg, uint##x##_t val) \ { \ write##s(val, dev_priv->regs + i915_mmio_reg_offset(reg)); \ diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 5464c0418bab..6d1ea26b2493 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -55,8 +55,7 @@ static inline void fw_domain_reset(struct drm_i915_private *i915, const struct intel_uncore_forcewake_domain *d) { - WARN_ON(!i915_mmio_reg_valid(d->reg_set)); - __raw_i915_write32(i915, d->reg_set, d->val_reset); + __raw_i915_write32(i915, d->reg_set, i915->uncore.fw_reset); } static inline void @@ -70,7 +69,7 @@ fw_domain_arm_timer(struct intel_uncore_forcewake_domain *d) } static inline void -fw_domain_wait_ack_clear(struct drm_i915_private *i915, +fw_domain_wait_ack_clear(const struct drm_i915_private *i915, const struct intel_uncore_forcewake_domain *d) { if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) & @@ -84,11 +83,11 @@ static inline void fw_domain_get(struct drm_i915_private *i915, const struct intel_uncore_forcewake_domain *d) { - __raw_i915_write32(i915, d->reg_set, d->val_set); + __raw_i915_write32(i915, d->reg_set, i915->uncore.fw_set); } static inline void -fw_domain_wait_ack(struct drm_i915_private *i915, +fw_domain_wait_ack(const struct drm_i915_private *i915, const struct intel_uncore_forcewake_domain *d) { if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) & @@ -99,10 +98,10 @@ fw_domain_wait_ack(struct drm_i915_private *i915, } static inline void -fw_domain_put(struct drm_i915_private *i915, +fw_domain_put(const struct drm_i915_private *i915, const struct intel_uncore_forcewake_domain *d) { - __raw_i915_write32(i915, d->reg_set, d->val_clear); + __raw_i915_write32(i915, d->reg_set, i915->uncore.fw_clear); } static void @@ -1139,21 +1138,13 @@ static void fw_domain_init(struct drm_i915_private *dev_priv, WARN_ON(d->wake_count); + WARN_ON(!i915_mmio_reg_valid(reg_set)); + WARN_ON(!i915_mmio_reg_valid(reg_ack)); + d->wake_count = 0; d->reg_set = reg_set; d->reg_ack = reg_ack; - if (IS_GEN6(dev_priv)) { - d->val_reset = 0; - d->val_set = FORCEWAKE_KERNEL; - d->val_clear = 0; - } else { - /* WaRsClearFWBitsAtReset:bdw,skl */ - d->val_reset = _MASKED_BIT_DISABLE(0x); - d->val_set = _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL); - d->val_clear = _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL); - } - d->id = domain_id; BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER)); @@ -1165,7 +1156,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv, hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); d->timer.function = intel_uncore_fw_release_timer; - dev_priv->uncore.fw_domains |= (1 << domain_id);
Re: [Intel-gfx] [PATCH 06/19] drm/vmwgfx: Drop the cursor locking hack
On 03/23/2017 11:10 AM, Daniel Vetter wrote: > On Thu, Mar 23, 2017 at 09:35:25AM +0100, Thomas Hellstrom wrote: >> Hi, Daniel, >> >> On 03/23/2017 08:31 AM, Daniel Vetter wrote: >>> On Thu, Mar 23, 2017 at 08:28:32AM +0100, Daniel Vetter wrote: On Thu, Mar 23, 2017 at 07:22:31AM +0100, Thomas Hellstrom wrote: > On 03/22/2017 10:50 PM, Daniel Vetter wrote: >> It's been around forever, no one bothered to address the FIXME, so I >> presume it's all fine. >> >> Cc: Sinclair Yeh >> Cc: Thomas Hellstrom >> Signed-off-by: Daniel Vetter > NAK. We need to properly address this. Probably as part of the atomic > update. So could someone with vmwgfx understanding explain this? Note that the FIXME was originally added by me years ago, because I wasn't sure (only about 90%) that this is safe, and was essentially pleading for a vmwgfx expert to review this? Since it didn't happen I presume it's not that terribly and probably safe ... I'm still 90% sure that this is correct, but I'd love for a vmwgfx to audit it. Replying with a NAK is kinda not the response I was hoping for (and yes I guess I should have explained what's going on here better, but it's just a git blame of the FIXME comment away). >> So the code has been left in place because it works. Altering it now >> will create unnecessary merge conflicts with the atomic code, and the >> change isn't tested and audited which means we need to drop focus from >> what we're doing and audit and test code that isn't going to be used >> anyway for not apparent reason? But otoh put in the below context there >> indeed is a reason. >> >> From a quick audit of the existing code it seems like at least >> vmw_cursor_update_position is touching global device state so I think at >> a minimum we need to take a spinlock in that function. Otherwise it >> seems to be safe. > Note that you're holding the crtc lock already, which gives you exclusion > against concurrent page_flips, mode_sets and property changes. Note also > that page_flips themselves also only hold the crtc lock, so you can run > multiple page_flips in parallel on different crtc (iirc vmwgfx has > multiple crtc, if not this discussion is entirely moot). > > tbh I'd be surprised if my patch really breaks something that hasn't been > a pre-existing issue for a long time. The original commit which added this > FIXME comment is from 2012. Note also that because it's a hack, you > already have a pretty a real race with the core drm state keeping, and no > one seems to have hit that either. > > I mean I can dig through vmwgfx code and do the audit, but it'll take a > few hours and vmwgfx is it's own world, so much harder to understand (for > me). > I'm thinking of the situation when someone would call a cursor_set ioctl in parallell for two crtcs at the same time and race writing the position registers? Note that the device has only a single global cursor. Admittedly the effects of a race would probably be small, but I'd rather see it being properly protected. >> But I prefer if we can do that as part of the atomic update? > When does that vmwgfx atomic happen? > -Daniel We're targeting 4.12, which means the code that is currently under testing will need to be sent out for review pretty soon. It's already in our standalone testing repo at git://git.freedesktop.org/git/mesa/vmwgfx but the cursor code hasn't been fixed in that repo yet. BTW is this blocking some other core drm work you're doing? Thanks, /Thomas ___ 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: Reduce Data Link N value for 1 lane DP->hdmi converters
== Series Details == Series: drm/i915: Reduce Data Link N value for 1 lane DP->hdmi converters URL : https://patchwork.freedesktop.org/series/21724/ State : success == Summary == Series 21724v1 drm/i915: Reduce Data Link N value for 1 lane DP->hdmi converters https://patchwork.freedesktop.org/api/1.0/series/21724/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 468s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 455s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 581s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 551s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 498s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 505s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 437s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 447s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 516s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 496s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 486s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 486s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 600s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 489s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 521s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 458s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 551s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 420s 8229a8c712c22ff8e94e3244d4fd942a7dcd89af drm-tip: 2017y-03m-23d-09h-57m-34s UTC integration manifest 18752d2 drm/i915: Reduce Data Link N value for 1 lane DP->hdmi converters == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4272/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Various improvements around the GuC topic
== Series Details == Series: Various improvements around the GuC topic URL : https://patchwork.freedesktop.org/series/21726/ State : success == Summary == Series 21726v1 Various improvements around the GuC topic https://patchwork.freedesktop.org/api/1.0/series/21726/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 460s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 461s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 593s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 523s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 511s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 502s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 441s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 431s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 436s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 509s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 496s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 478s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 468s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 588s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 480s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 513s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 446s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 558s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 416s 8229a8c712c22ff8e94e3244d4fd942a7dcd89af drm-tip: 2017y-03m-23d-09h-57m-34s UTC integration manifest f003ba2 HAX Enable GuC loading & submission 43d51b6 drm/i915/guc: Move guc_interrupts_release next to guc_interrupts_capture 19b8f8a drm/i915/guc: Split out the mmio_white_list struct 22675ac drm/i915/guc: Refactor the concept "GuC context descriptor" into "GuC stage descriptor" 98c3280 drm/i915/guc: A little bit more of doorbell sanitization a5d1810 drm/i915/guc: Wait for doorbell to be inactive before deallocating cf3850d drm/i915/guc: Improve the GuC documentation & comments about proxy submissions 7112b1b drm/i915/guc: Make intel_guc_send a function pointer cdbbc12 drm/i915/guc: Break out the GuC log extras into their own "runtime" struct 0009b27 drm/i915/guc: The Additional Data Struct (ADS) should get enabled together with GuC submission c11c121 drm/i915/guc: Add onion teardown to the GuC setup d1a1ced drm/i915/guc: Keep the ctx_pool_vaddr mapped, for easy access 5956a8b drm/i915/guc: Sanitize GuC client initialization == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4273/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with drm: Make the decision to keep vblank irq enabled earlier (rev2)
== Series Details == Series: series starting with drm: Make the decision to keep vblank irq enabled earlier (rev2) URL : https://patchwork.freedesktop.org/series/21318/ State : success == Summary == Series 21318v2 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/21318/revisions/2/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 467s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 463s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 578s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 537s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 512s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 503s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 438s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 446s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 518s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 496s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 480s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 481s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 597s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 490s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 517s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 458s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 554s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time: 418s fi-hsw-4770 failed to collect. IGT log at Patchwork_4274/fi-hsw-4770/igt.log 8229a8c712c22ff8e94e3244d4fd942a7dcd89af drm-tip: 2017y-03m-23d-09h-57m-34s UTC integration manifest 0ed1ad7 drm: Peek at the current counter/timestamp for vblank queries 562e3b3 drm: Make the decision to keep vblank irq enabled earlier == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4274/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Wait for all fences before installing an exclusive clflush fence
== Series Details == Series: drm/i915: Wait for all fences before installing an exclusive clflush fence URL : https://patchwork.freedesktop.org/series/21746/ State : success == Summary == Series 21746v1 drm/i915: Wait for all fences before installing an exclusive clflush fence https://patchwork.freedesktop.org/api/1.0/series/21746/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 462s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 452s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 581s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 537s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 511s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 502s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 440s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 443s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 516s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 503s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 483s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 495s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 601s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 490s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 529s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 463s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 549s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 417s 8229a8c712c22ff8e94e3244d4fd942a7dcd89af drm-tip: 2017y-03m-23d-09h-57m-34s UTC integration manifest 33a1f7d drm/i915: Wait for all fences before installing an exclusive clflush fence == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4275/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reduce Data Link N value for 1 lane DP->hdmi converters
On Wed, Mar 22, 2017 at 04:27:36PM -0700, clinton.a.tay...@intel.com wrote: > From: Clint Taylor > > Several major vendor USB-C->HDMI converters fail to recover a 5.4 GHz 1 lane > signal if the Data Link N is greater than 0x8. > Patch detects when 1 lane 5.4 GHz signal is being used and makes the maximum > value 20 bit instead of the maximum specification supported 24 bit value. > > Cc: Jani Nikula > Cc: Anusha Srivatsa > > Signed-off-by: Clint Taylor > --- > drivers/gpu/drm/i915/i915_reg.h |2 ++ > drivers/gpu/drm/i915/intel_display.c | 15 +++ > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 04c8f69..838d8d5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4869,6 +4869,8 @@ enum { > > #define DATA_LINK_M_N_MASK (0xff) > #define DATA_LINK_N_MAX (0x80) > +/* Maximum N value useable on some DP->HDMI converters */ > +#define DATA_LINK_REDUCED_N_MAX (0x8) > > #define _PIPEA_DATA_N_G4X0x70054 > #define _PIPEB_DATA_N_G4X0x71054 > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 010e5dd..6e1fdd2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6315,9 +6315,10 @@ static int intel_crtc_compute_config(struct intel_crtc > *crtc, > } > > static void compute_m_n(unsigned int m, unsigned int n, > - uint32_t *ret_m, uint32_t *ret_n) > + uint32_t *ret_m, uint32_t *ret_n, > + uint32_t max_link_n) > { > - *ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX); > + *ret_n = min_t(unsigned int, roundup_pow_of_two(n), max_link_n); > *ret_m = div_u64((uint64_t) m * *ret_n, n); > intel_reduce_m_n_ratio(ret_m, ret_n); > } > @@ -6327,14 +6328,20 @@ static void compute_m_n(unsigned int m, unsigned int > n, > int pixel_clock, int link_clock, > struct intel_link_m_n *m_n) > { > + uint32_t max_link_n = DATA_LINK_N_MAX; > m_n->tu = 64; > > + if ((nlanes==1) && (link_clock >= 54)) > + max_link_n = DATA_LINK_REDUCED_N_MAX; This could use a good comment explaining what's going on. I'd also like to see a list of affected dongles as part of the comment. Historically we've had far too many magic quirks for some unspecified piece of hardware which makes it really difficult to change the code later as there's no clue how one could even test it. > + > compute_m_n(bits_per_pixel * pixel_clock, > link_clock * nlanes * 8, > - &m_n->gmch_m, &m_n->gmch_n); > + &m_n->gmch_m, &m_n->gmch_n, > + max_link_n); > > compute_m_n(pixel_clock, link_clock, > - &m_n->link_m, &m_n->link_n); > + &m_n->link_m, &m_n->link_n, > + max_link_n); > } > > static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) > -- > 1.7.9.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [CI,1/7] drm/i915: Eliminate per-fw_domain i915 backpointer
== Series Details == Series: series starting with [CI,1/7] drm/i915: Eliminate per-fw_domain i915 backpointer URL : https://patchwork.freedesktop.org/series/21752/ State : warning == Summary == Series 21752v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/21752/revisions/1/mbox/ Test kms_force_connector_basic: Subgroup force-connector-state: pass -> SKIP (fi-ivb-3520m) Subgroup force-edid: pass -> SKIP (fi-ivb-3520m) Subgroup force-load-detect: pass -> SKIP (fi-ivb-3520m) Subgroup prune-stale-modes: pass -> SKIP (fi-ivb-3520m) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 457s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 464s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 586s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 544s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 512s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 508s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 437s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 442s fi-ivb-3520m total:278 pass:256 dwarn:0 dfail:0 fail:0 skip:22 time: 517s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 511s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 484s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 480s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 601s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 480s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 532s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 460s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 551s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 424s 8229a8c712c22ff8e94e3244d4fd942a7dcd89af drm-tip: 2017y-03m-23d-09h-57m-34s UTC integration manifest d61c3fe drm/i915: Drop uncore spinlock for reading debugfs forcewake counters 6d3abb8 drm/i915: All fw_domains share the same set/clear/reset values 07b60e2 drm/i915: Remove posting-read for forcewake put e8e1859 drm/i915: Skip unused fw_domains 5927577 drm/i915: Use correct fw_domains during reset 6977658 drm/i915: Use correct fw_domains during initialisation 26d1e37 drm/i915: Eliminate per-fw_domain i915 backpointer == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4276/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Wait for all fences before installing an exclusive clflush fence
Chris Wilson writes: > Ensure that before we overwrite the reservation_object with our > exclusive fence for the pending clflush operation, that we do wait upon > all the fences in the current reservation_object. > > Fixes: 57822dc6b9cf ("drm/i915: Perform object clflushing asynchronously") > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen > Cc: Matthew Auld Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gem_clflush.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c > b/drivers/gpu/drm/i915/i915_gem_clflush.c > index d925fb582ba7..ffd01e02fe94 100644 > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c > @@ -168,7 +168,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object > *obj, > > i915_sw_fence_await_reservation(&clflush->wait, > obj->resv, NULL, > - false, I915_FENCE_TIMEOUT, > + true, I915_FENCE_TIMEOUT, > GFP_KERNEL); > > reservation_object_lock(obj->resv, NULL); > -- > 2.11.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [CI,1/7] drm/i915: Eliminate per-fw_domain i915 backpointer
On Thu, Mar 23, 2017 at 11:56:11AM -, Patchwork wrote: > == Series Details == > > Series: series starting with [CI,1/7] drm/i915: Eliminate per-fw_domain i915 > backpointer > URL : https://patchwork.freedesktop.org/series/21752/ > State : warning > > == Summary == > > Series 21752v1 Series without cover letter > https://patchwork.freedesktop.org/api/1.0/series/21752/revisions/1/mbox/ > > Test kms_force_connector_basic: > Subgroup force-connector-state: > pass -> SKIP (fi-ivb-3520m) > Subgroup force-edid: > pass -> SKIP (fi-ivb-3520m) > Subgroup force-load-detect: > pass -> SKIP (fi-ivb-3520m) > Subgroup prune-stale-modes: > pass -> SKIP (fi-ivb-3520m) Another unconnected skip afaict. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Wait for all fences before installing an exclusive clflush fence
On Thu, Mar 23, 2017 at 01:55:33PM +0200, Mika Kuoppala wrote: > Chris Wilson writes: > > > Ensure that before we overwrite the reservation_object with our > > exclusive fence for the pending clflush operation, that we do wait upon > > all the fences in the current reservation_object. > > > > Fixes: 57822dc6b9cf ("drm/i915: Perform object clflushing asynchronously") > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Cc: Matthew Auld > > Reviewed-by: Mika Kuoppala Thanks, I realised my mistake when doing async page loading. Now you've read this patch, you should be ready for those... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
From: Colin Ian King info is being checked to see if it is a null pointer, however, vpgu is dereferencing info before this check, leading to a potential null pointer dereference. If info is null, then the error message being printed by macro gvt_vgpu_err and this requires vpgu to exist. We can use a null vpgu as the macro has a sanity check to see if vpgu is null, so this is OK. Detected with CoverityScan, CID#1420672 ("Dereference nefore null check") Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err") Signed-off-by: Colin Ian King --- drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 1ea3eb270de8..f8619a772c44 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -1339,9 +1339,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev) static bool kvmgt_guest_exit(struct kvmgt_guest_info *info) { - struct intel_vgpu *vgpu = info->vgpu; - if (!info) { + struct intel_vgpu *vgpu = NULL; + gvt_vgpu_err("kvmgt_guest_info invalid\n"); return false; } -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v3 1/7] lib/igt_kms: Add forcing TEST_ONLY for atomic commits
On Wed, 2017-03-08 at 15:59 +0100, Maarten Lankhorst wrote: > Op 01-02-17 om 14:18 schreef Mika Kahola: > > > > Add an option to force atomic commits to do commits with TEST_ONLY > > flag > > first before doing the actual commit. > > > > Signed-off-by: Mika Kahola > > --- > > lib/igt_kms.c | 18 +- > > lib/igt_kms.h | 1 + > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > > index 4ba6316..c513ef8 100644 > > --- a/lib/igt_kms.c > > +++ b/lib/igt_kms.c > > @@ -2453,7 +2453,23 @@ static int igt_atomic_commit(igt_display_t > > *display, uint32_t flags, void *user_ > > igt_atomic_prepare_connector_commit(output, req); > > } > > > > - ret = drmModeAtomicCommit(display->drm_fd, req, flags, > > user_data); > > + if (display->force_test_atomic && > > + !(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { > > + unsigned int test_flags = flags & > > ~DRM_MODE_PAGE_FLIP_EVENT; > > + int test_ret; > > + > > + test_flags |= DRM_MODE_ATOMIC_TEST_ONLY; > > + > > + test_ret = drmModeAtomicCommit(display->drm_fd, > > req, test_flags, user_data); > > + ret = drmModeAtomicCommit(display->drm_fd, req, > > flags, user_data); > > + > > + if (test_ret) > > + igt_assert_eq(test_ret, ret); > > + else > > + igt_assert(ret != -EINVAL); > > + } else > > + ret = drmModeAtomicCommit(display->drm_fd, req, > > flags, user_data); > > + > > drmModeAtomicFree(req); > > return ret; > > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h > > index 2562618..e45fc21 100644 > > --- a/lib/igt_kms.h > > +++ b/lib/igt_kms.h > > @@ -338,6 +338,7 @@ struct igt_display { > > igt_pipe_t *pipes; > > bool has_cursor_plane; > > bool is_atomic; > > + bool force_test_atomic; > > }; > > > > void igt_display_init(igt_display_t *display, int drm_fd); > Sorry, didn't notice it before but force_test_atomic is a big > behavioral switch that > might force other tests to potentially fail. If this a subtest fails > and the flag is > not explictly cleared. This is also why you need to set it separately > even if false. So for this test it would be adequate if we clear this flag in case of error? I'll spin another round of this patch series. It also requires some rebasing too. > > I really wish there was a igt_subtest_exit_handler, I could remove a > lot of boilerplate in the KMS tests with that. > > My personal wishlist for it: > - Complete all sw_sync fences > - Remove all allocated pipe_crc's, so next test won't fail due to fd > already being assigned. > - Clean igt_display_t with a synchronous commit. > - Clear any pending drm events without blocking. > - Remove any leftover framebuffers allocated in the subtest. This may > fail if fb's become > invalid, like in the kms_rmfb test. > This would be nice. Asserts may leave things in unstable state. > Be careful with dereferencing fb's, the previous pointer may not be > valid any more if it > was allocated on the stack. It should probably require an extra > memory allocation during > fb alloc to keep track. > > This would allow us to kill most of the teardown boilerplate in igt > kms tests, and increases > reliability when a subtest fails, the next subtest is a lot more > likely to succeed. > > ~Maarten > -- Mika Kahola - Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reduce Data Link N value for 1 lane DP->hdmi converters
On Thu, 23 Mar 2017, clinton.a.tay...@intel.com wrote: > From: Clint Taylor > > Several major vendor USB-C->HDMI converters fail to recover a 5.4 GHz 1 lane > signal if the Data Link N is greater than 0x8. > Patch detects when 1 lane 5.4 GHz signal is being used and makes the maximum > value 20 bit instead of the maximum specification supported 24 bit value. > > Cc: Jani Nikula > Cc: Anusha Srivatsa > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93578 > Signed-off-by: Clint Taylor > --- > drivers/gpu/drm/i915/i915_reg.h |2 ++ > drivers/gpu/drm/i915/intel_display.c | 15 +++ > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 04c8f69..838d8d5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4869,6 +4869,8 @@ enum { > > #define DATA_LINK_M_N_MASK (0xff) > #define DATA_LINK_N_MAX (0x80) > +/* Maximum N value useable on some DP->HDMI converters */ > +#define DATA_LINK_REDUCED_N_MAX (0x8) > > #define _PIPEA_DATA_N_G4X0x70054 > #define _PIPEB_DATA_N_G4X0x71054 > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 010e5dd..6e1fdd2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6315,9 +6315,10 @@ static int intel_crtc_compute_config(struct intel_crtc > *crtc, > } > > static void compute_m_n(unsigned int m, unsigned int n, > - uint32_t *ret_m, uint32_t *ret_n) > + uint32_t *ret_m, uint32_t *ret_n, > + uint32_t max_link_n) > { > - *ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX); > + *ret_n = min_t(unsigned int, roundup_pow_of_two(n), max_link_n); If there's evidence suggesting "certain other operating systems" always use a max (or fixed value) of 0x8, perhaps we should just follow suit? Simpler and less magical. > *ret_m = div_u64((uint64_t) m * *ret_n, n); > intel_reduce_m_n_ratio(ret_m, ret_n); > } > @@ -6327,14 +6328,20 @@ static void compute_m_n(unsigned int m, unsigned int > n, > int pixel_clock, int link_clock, > struct intel_link_m_n *m_n) > { > + uint32_t max_link_n = DATA_LINK_N_MAX; > m_n->tu = 64; > > + if ((nlanes==1) && (link_clock >= 54)) Is the problem really dependent on these conditions? You can get the same problematic N value with nlanes == 2 && link_clock == 27 too. BR, Jani. > + max_link_n = DATA_LINK_REDUCED_N_MAX; > + > compute_m_n(bits_per_pixel * pixel_clock, > link_clock * nlanes * 8, > - &m_n->gmch_m, &m_n->gmch_n); > + &m_n->gmch_m, &m_n->gmch_n, > + max_link_n); > > compute_m_n(pixel_clock, link_clock, > - &m_n->link_m, &m_n->link_n); > + &m_n->link_m, &m_n->link_n, > + max_link_n); > } > > static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Actually pass the reclaim gfp_t along to shmemfs!
On Thu, 23 Mar 2017, Chris Wilson wrote: > On Thu, Mar 23, 2017 at 09:29:47AM +0200, Joonas Lahtinen wrote: >> On ke, 2017-03-22 at 22:34 +, Chris Wilson wrote: >> > Words cannot describe the embarrassment at creating a new gfp_t relaim to >> > only prevent the oomkiller but allow direct|kswapd reclaim, and then not >> > use it in the shmem_read_mapping_page_gfp(). >> > >> > Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur >> > the oom for gfx allocations") >> > Signed-off-by: Chris Wilson >> > Cc: Joonas Lahtinen >> > Cc: Daniel Vetter >> >> Reviewed-by: Joonas Lahtinen > > Thanks for not being smug, pushed. He can't afford to be, he reviewed the commit being fixed! ;) BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On Thu, Mar 23, 2017 at 12:22:30PM +, Colin King wrote: > From: Colin Ian King > > info is being checked to see if it is a null pointer, however, vpgu is > dereferencing info before this check, leading to a potential null > pointer dereference. If info is null, then the error message being > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > use a null vpgu as the macro has a sanity check to see if vpgu is null, > so this is OK. It is never NULL, it gets checked by its only caller. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 0/7] Validate TEST_ONLY correctness against full atomic commit
This test case adds TEST_ONLY flag to the following test cases to test atomic commit correctness. - kms_plane_multiple - kms_atomic_transitions - kms_plane_scaling - kms_rotation_crc - kms_plane_lowres The test randomly selects one of the above test cases and tests atomic commit. If the test fails with TEST_ONLY flag the real deal atomic commit is executed and the outcome is verified. v2: Add flag to toggle TEST_ONLY for atomic commit. Remove DRM_MODE_PAGE_FLIP_EVENT flag, if enabled, before trying atomic commit with TEST_ONLY flag (Maarten) v3: flag to indicate TEST_ONLY selection in igt core separate test cases for TEST_ONLY atomic commits (Maarten) v4: Clear force_test_atomic flag if atomic commit fails (Maarten) For: VIZ-6956 Mika Kahola (7): lib/igt_kms: Add forcing TEST_ONLY for atomic commits tests/kms_plane_multiple: Add TEST_ONLY flag Cc: maarten.lankho...@linux.intel.com tests/kms_atomic_transition: Add TEST_ONLY flag tests/kms_plane_scaling: Add TEST_ONLY flag tests/kms_rotation_crc: Add TEST_ONLY flag tests/kms_plane_lowres: Add TEST_ONLY flag tests/kms_cursor_legacy: Add TEST_ONLY flag lib/igt_kms.c | 22 +++- lib/igt_kms.h | 1 + tests/kms_atomic_transition.c | 72 ++--- tests/kms_cursor_legacy.c | 230 +- tests/kms_plane_lowres.c | 40 +++- tests/kms_plane_multiple.c| 48 +++-- tests/kms_plane_scaling.c | 28 +++-- tests/kms_rotation_crc.c | 139 + 8 files changed, 518 insertions(+), 62 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 6/7] tests/kms_plane_lowres: Add TEST_ONLY flag
Add TEST_ONLY flag to test atomic modesetting commits without actual real-life commit. Signed-off-by: Mika Kahola --- tests/kms_plane_lowres.c | 40 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c index 6f15960..e6cf06f 100644 --- a/tests/kms_plane_lowres.c +++ b/tests/kms_plane_lowres.c @@ -318,21 +318,53 @@ test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier) static void run_tests_for_pipe(data_t *data, enum pipe pipe) { + igt_subtest_f("pipe-%s-tiling-none-with-test", + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = true; + test_plane_position(data, pipe, LOCAL_DRM_FORMAT_MOD_NONE); + } + igt_subtest_f("pipe-%s-tiling-none", - kmstest_pipe_name(pipe)) + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = false; test_plane_position(data, pipe, LOCAL_DRM_FORMAT_MOD_NONE); + } + + igt_subtest_f("pipe-%s-tiling-x-with-test", + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = true; + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_X_TILED); + } igt_subtest_f("pipe-%s-tiling-x", - kmstest_pipe_name(pipe)) + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = false; test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_X_TILED); + } + + igt_subtest_f("pipe-%s-tiling-y-with-test", + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = true; + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Y_TILED); + } igt_subtest_f("pipe-%s-tiling-y", - kmstest_pipe_name(pipe)) + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = false; test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Y_TILED); + } + + igt_subtest_f("pipe-%s-tiling-yf-with-test", + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = true; + test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Yf_TILED); + } igt_subtest_f("pipe-%s-tiling-yf", - kmstest_pipe_name(pipe)) + kmstest_pipe_name(pipe)) { + data->display.force_test_atomic = false; test_plane_position(data, pipe, LOCAL_I915_FORMAT_MOD_Yf_TILED); + } } static data_t data; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 5/7] tests/kms_rotation_crc: Add TEST_ONLY flag
Add TEST_ONLY flag to test atomic modesetting commits without actual real-life commit. Signed-off-by: Mika Kahola --- tests/kms_rotation_crc.c | 139 +++ 1 file changed, 139 insertions(+) diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index 4347884..68e9326 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -522,46 +522,109 @@ igt_main igt_display_init(&data.display, data.gfx_fd); } igt_subtest_f("primary-rotation-180") { + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_180; + test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY); + } + + igt_subtest_f("primary-rotation-180-with-test") { + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY); } igt_subtest_f("sprite-rotation-180") { + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_180; + test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); + } + + igt_subtest_f("sprite-rotation-180-with-test") { + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); } igt_subtest_f("cursor-rotation-180") { + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_180; + test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR); + } + + igt_subtest_f("cursor-rotation-180-with-test") { + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_180; test_plane_rotation(&data, DRM_PLANE_TYPE_CURSOR); } igt_subtest_f("primary-rotation-90") { igt_require(gen >= 9); + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_90; + test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY); + } + + igt_subtest_f("primary-rotation-90-with-test") { + igt_require(gen >= 9); + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_90; test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY); } igt_subtest_f("primary-rotation-270") { igt_require(gen >= 9); + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_270; + test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY); + } + + igt_subtest_f("primary-rotation-270-with-test") { + igt_require(gen >= 9); + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_270; test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY); } igt_subtest_f("sprite-rotation-90") { igt_require(gen >= 9); + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_90; + test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); + } + + igt_subtest_f("sprite-rotation-90-with-test") { + igt_require(gen >= 9); + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_90; test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); } igt_subtest_f("sprite-rotation-270") { igt_require(gen >= 9); + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_270; + test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); + } + + igt_subtest_f("sprite-rotation-270-with-test") { + igt_require(gen >= 9); + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_270; test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); } igt_subtest_f("sprite-rotation-90-pos-100-0") { igt_require(gen >= 9); + data.display.force_test_atomic = false; + data.rotation = IGT_ROTATION_90; + data.pos_x = 100, + data.pos_y = 0; + test_plane_rotation(&data, DRM_PLANE_TYPE_OVERLAY); + } + + igt_subtest_f("sprite-rotation-90-pos-100-0-with-test") { + igt_require(gen >= 9); + data.display.force_test_atomic = true; data.rotation = IGT_ROTATION_90; data.pos_x = 100, data.pos_y = 0; @@ -574,6 +637,17 @@ igt_main data.pos_y = 0; data.rotation = IGT_ROTATION_90; data.override_fmt = DRM_FORMAT_RGB565; + data.display.force_test_atomic = false; + test_plane
[Intel-gfx] [PATCH i-g-t v4 2/7] tests/kms_plane_multiple: Add TEST_ONLY flag Cc: maarten.lankho...@linux.intel.com
Add TEST_ONLY flag to test atomic modesetting commits without actual real-life commit. v2: Use flag to indicate to test with TEST_ONLY flag with atomic commit v3: Moved force_test_atomic flag set to 'test_plane_position()' Signed-off-by: Mika Kahola --- tests/kms_plane_multiple.c | 48 -- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c index 93dce6b..84fd411 100644 --- a/tests/kms_plane_multiple.c +++ b/tests/kms_plane_multiple.c @@ -365,7 +365,8 @@ test_legacy_plane_position_with_output(data_t *data, enum pipe pipe, } static void -test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling) +test_plane_position(data_t *data, enum pipe pipe, bool atomic, + bool force_test_atomic, uint64_t tiling) { igt_output_t *output; int connected_outs; @@ -379,6 +380,8 @@ test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling) tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED)) igt_require(AT_LEAST_GEN(devid, 9)); + data->display.force_test_atomic = force_test_atomic; + if (!opt.user_seed) opt.seed = time(NULL); @@ -421,46 +424,71 @@ run_tests_for_pipe(data_t *data, enum pipe pipe) igt_require(data->display.pipes[pipe].n_planes > 0); } - igt_subtest_f("legacy-pipe-%s-tiling-none", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_DRM_FORMAT_MOD_NONE); + test_plane_position(data, pipe, false, false, + LOCAL_DRM_FORMAT_MOD_NONE); igt_subtest_f("atomic-pipe-%s-tiling-none", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("legacy-pipe-%s-tiling-x", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("atomic-pipe-%s-tiling-x", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("legacy-pipe-%s-tiling-y", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_Y_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_Y_TILED); igt_subtest_f("atomic-pipe-%s-tiling-y", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_Y_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_Y_TILED); igt_subtest_f("legacy-pipe-%s-tiling-yf", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_Yf_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_Yf_TILED); igt_subtest_f("atomic-pipe-%s-tiling-yf", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_Yf_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_Yf_TILED); + + igt_subtest_f("atomic-pipe-%s-tiling-x-with-test", + kmstest_pipe_name(pipe)) + for_each_valid_output_on_pipe(&data->display, pipe, output) + test_plane_position(data, pipe, true,
[Intel-gfx] [PATCH i-g-t v4 2/3] tests/kms_plane_multiple: Add TEST_ONLY flag Cc: maarten.lankho...@linux.intel.com
Add TEST_ONLY flag to test atomic modesetting commits without actual real-life commit. v2: Use flag to indicate to test with TEST_ONLY flag with atomic commit v3: Moved force_test_atomic flag set to 'test_plane_position()' Signed-off-by: Mika Kahola --- tests/kms_plane_multiple.c | 48 -- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c index 93dce6b..84fd411 100644 --- a/tests/kms_plane_multiple.c +++ b/tests/kms_plane_multiple.c @@ -365,7 +365,8 @@ test_legacy_plane_position_with_output(data_t *data, enum pipe pipe, } static void -test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling) +test_plane_position(data_t *data, enum pipe pipe, bool atomic, + bool force_test_atomic, uint64_t tiling) { igt_output_t *output; int connected_outs; @@ -379,6 +380,8 @@ test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling) tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED)) igt_require(AT_LEAST_GEN(devid, 9)); + data->display.force_test_atomic = force_test_atomic; + if (!opt.user_seed) opt.seed = time(NULL); @@ -421,46 +424,71 @@ run_tests_for_pipe(data_t *data, enum pipe pipe) igt_require(data->display.pipes[pipe].n_planes > 0); } - igt_subtest_f("legacy-pipe-%s-tiling-none", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_DRM_FORMAT_MOD_NONE); + test_plane_position(data, pipe, false, false, + LOCAL_DRM_FORMAT_MOD_NONE); igt_subtest_f("atomic-pipe-%s-tiling-none", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("legacy-pipe-%s-tiling-x", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("atomic-pipe-%s-tiling-x", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("legacy-pipe-%s-tiling-y", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_Y_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_Y_TILED); igt_subtest_f("atomic-pipe-%s-tiling-y", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_Y_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_Y_TILED); igt_subtest_f("legacy-pipe-%s-tiling-yf", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_Yf_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_Yf_TILED); igt_subtest_f("atomic-pipe-%s-tiling-yf", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_Yf_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_Yf_TILED); + + igt_subtest_f("atomic-pipe-%s-tiling-x-with-test", + kmstest_pipe_name(pipe)) + for_each_valid_output_on_pipe(&data->display, pipe, output) + test_plane_position(data, pipe, true,
[Intel-gfx] [PATCH i-g-t v4 4/7] tests/kms_plane_scaling: Add TEST_ONLY flag
Add TEST_ONLY flag to test atomic scaling without actually committing the changes. v2: Create subtests with TEST_ONLY flag and one without v3: Rename subtest 'force-atomic-test' as 'with-atomic-test' Signed-off-by: Mika Kahola --- tests/kms_plane_scaling.c | 28 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c index 1457894..8db91c3 100644 --- a/tests/kms_plane_scaling.c +++ b/tests/kms_plane_scaling.c @@ -310,21 +310,33 @@ static void test_plane_scaling(data_t *d) igt_require_f(valid_tests, "no valid crtc/connector combinations found\n"); } -igt_simple_main +igt_main { data_t data = {}; igt_skip_on_simulation(); + igt_fixture { + data.drm_fd = drm_open_driver(DRIVER_INTEL); + igt_require_pipe_crc(data.drm_fd); + igt_display_init(&data.display, data.drm_fd); + data.devid = intel_get_drm_devid(data.drm_fd); + data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0; + } - data.drm_fd = drm_open_driver(DRIVER_INTEL); - igt_require_pipe_crc(data.drm_fd); - igt_display_init(&data.display, data.drm_fd); - data.devid = intel_get_drm_devid(data.drm_fd); + igt_subtest("with-atomic-test") { + data.display.force_test_atomic = true; + test_plane_scaling(&data); + } - data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0; + igt_subtest("normal") { + data.display.force_test_atomic = false; + test_plane_scaling(&data); + } - test_plane_scaling(&data); + igt_fixture { + igt_display_fini(&data.display); + } - igt_display_fini(&data.display); + igt_exit(); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 1/7] lib/igt_kms: Add forcing TEST_ONLY for atomic commits
Add an option to force atomic commits to do commits with TEST_ONLY flag first before doing the actual commit. v2: Clear force_test_atomic flag if atomic commit with TEST_ONLY flag fails (Maarten) Signed-off-by: Mika Kahola --- lib/igt_kms.c | 22 +- lib/igt_kms.h | 1 + 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index d9f9672..a687939 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -2436,7 +2436,27 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_ igt_atomic_prepare_connector_commit(output, req); } - ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data); + if (display->force_test_atomic && + !(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { + unsigned int test_flags = flags & ~DRM_MODE_PAGE_FLIP_EVENT; + int test_ret; + + test_flags |= DRM_MODE_ATOMIC_TEST_ONLY; + + test_ret = drmModeAtomicCommit(display->drm_fd, req, test_flags, user_data); + ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data); + + if (test_ret) { + if (test_ret != ret) + display->force_test_atomic = false; + + igt_assert_eq(test_ret, ret); + } else { + igt_assert(ret != -EINVAL); + } + } else + ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data); + if (!ret) { for_each_pipe(display, pipe) { diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 595832d..2972e94 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -368,6 +368,7 @@ struct igt_display { igt_pipe_t *pipes; bool has_cursor_plane; bool is_atomic; + bool force_test_atomic; }; void igt_display_init(igt_display_t *display, int drm_fd); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 3/7] tests/kms_atomic_transition: Add TEST_ONLY flag
Add TEST_ONLY flag to test atomic transition display commits without actual real-life commit. v2: use flag to force atomic commit with TEST_ONLY flag v3: Rebase Signed-off-by: Mika Kahola --- tests/kms_atomic_transition.c | 72 +++ 1 file changed, 59 insertions(+), 13 deletions(-) diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c index 70bff20..466280e 100644 --- a/tests/kms_atomic_transition.c +++ b/tests/kms_atomic_transition.c @@ -397,7 +397,8 @@ static void atomic_commit(igt_display_t *display, enum pipe pipe, unsigned int f */ static void run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output, - enum transition_type type, bool nonblocking, bool fencing) + enum transition_type type, bool nonblocking, bool fencing, + bool force_test_atomic) { struct igt_fb fb, argb_fb, sprite_fb; drmModeModeInfo *mode, override_mode; @@ -408,6 +409,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output unsigned flags = DRM_MODE_PAGE_FLIP_EVENT; int ret; + display->force_test_atomic = force_test_atomic; + if (fencing) prepare_fencing(display, pipe); @@ -782,12 +785,15 @@ cleanup: } -static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking, bool fencing) +static void run_modeset_transition(igt_display_t *display, int requested_outputs, + bool nonblocking, bool fencing, bool force_test_atomic) { igt_output_t *outputs[I915_MAX_PIPES] = {}; int num_outputs = 0; enum pipe pipe; + display->force_test_atomic = force_test_atomic; + for_each_pipe(display, pipe) { igt_output_t *output; @@ -845,44 +851,84 @@ igt_main igt_subtest("plane-all-transition") for_each_pipe_with_valid_output(&display, pipe, output) - run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false); + run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false, false); + + igt_subtest("plane-all-transition-with-test") + for_each_pipe_with_valid_output(&display, pipe, output) + run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false, true); igt_subtest("plane-all-transition-fencing") for_each_pipe_with_valid_output(&display, pipe, output) - run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true); + run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true, false); + + igt_subtest("plane-all-transition-fencing-with-test") + for_each_pipe_with_valid_output(&display, pipe, output) + run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true, true); igt_subtest("plane-all-transition-nonblocking") for_each_pipe_with_valid_output(&display, pipe, output) - run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false); + run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false, false); + + igt_subtest("plane-all-transition-nonblocking-with-test") + for_each_pipe_with_valid_output(&display, pipe, output) + run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false, true); igt_subtest("plane-all-transition-nonblocking-fencing") for_each_pipe_with_valid_output(&display, pipe, output) - run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, true); + run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, true, false); + + igt_subtest("plane-all-transition-nonblocking-fencing-with-test") + for_each_pipe_with_valid_output(&display, pipe, output) + run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, true, true); igt_subtest("plane-all-modeset-transition") for_each_pipe_with_valid_output(&display, pipe, output) - run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false); + run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false, false); + + igt_subtest("plane-all-modeset-transition-with-test") + for_each_pipe_with_valid_output(&display, pipe, output) + run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false, true); igt_subtest("plane-all-modeset-transition-fencing") for_each_pipe_with_valid_output(&display, pipe, output) - run_
[Intel-gfx] [PATCH i-g-t v4 7/7] tests/kms_cursor_legacy: Add TEST_ONLY flag
Add TEST_ONLY flag to test atomic modesetting commits without actual real-life commit. Signed-off-by: Mika Kahola --- tests/kms_cursor_legacy.c | 230 -- 1 file changed, 204 insertions(+), 26 deletions(-) diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c index 8a8c71b..6f79ad8 100644 --- a/tests/kms_cursor_legacy.c +++ b/tests/kms_cursor_legacy.c @@ -1441,53 +1441,165 @@ igt_main igt_subtest("all-pipes-torture-move") stress(&display, -1, -ncpus, DRM_MODE_CURSOR_MOVE, 20); - igt_subtest("nonblocking-modeset-vs-cursor-atomic") + igt_subtest("nonblocking-modeset-vs-cursor-atomic") { + display.force_test_atomic = false; nonblocking_modeset_vs_cursor(&display, 1); + } - igt_subtest("long-nonblocking-modeset-vs-cursor-atomic") + igt_subtest("nonblocking-modeset-vs-cursor-atomic-with-test") { + display.force_test_atomic = true; + nonblocking_modeset_vs_cursor(&display, 1); + } + + igt_subtest("long-nonblocking-modeset-vs-cursor-atomic") { + display.force_test_atomic = false; + nonblocking_modeset_vs_cursor(&display, 16); + } + + igt_subtest("long-nonblocking-modeset-vs-cursor-atomic-with-test") { + display.force_test_atomic = true; nonblocking_modeset_vs_cursor(&display, 16); + } - igt_subtest("2x-flip-vs-cursor-legacy") + igt_subtest("2x-flip-vs-cursor-legacy") { + display.force_test_atomic = false; two_screens_flip_vs_cursor(&display, 8, false, false); + } + + igt_subtest("2x-flip-vs-cursor-legacy-with-test") { + display.force_test_atomic = true; + two_screens_flip_vs_cursor(&display, 8, false, false); + } + + igt_subtest("2x-flip-vs-cursor-atomic") { + display.force_test_atomic = false; + two_screens_flip_vs_cursor(&display, 4, false, true); + } - igt_subtest("2x-flip-vs-cursor-atomic") + igt_subtest("2x-flip-vs-cursor-atomic-with-test") { + display.force_test_atomic = true; two_screens_flip_vs_cursor(&display, 4, false, true); + } - igt_subtest("2x-cursor-vs-flip-legacy") + igt_subtest("2x-cursor-vs-flip-legacy") { + display.force_test_atomic = false; two_screens_cursor_vs_flip(&display, 8, false); + } - igt_subtest("2x-long-flip-vs-cursor-legacy") + igt_subtest("2x-cursor-vs-flip-legacy-with-test") { + display.force_test_atomic = true; + two_screens_cursor_vs_flip(&display, 8, false); + } + + igt_subtest("2x-long-flip-vs-cursor-legacy") { + display.force_test_atomic = false; + two_screens_flip_vs_cursor(&display, 150, false, false); + } + + igt_subtest("2x-long-flip-vs-cursor-legacy-with-test") { + display.force_test_atomic = true; two_screens_flip_vs_cursor(&display, 150, false, false); + } - igt_subtest("2x-long-flip-vs-cursor-atomic") + igt_subtest("2x-long-flip-vs-cursor-atomic") { + display.force_test_atomic = false; two_screens_flip_vs_cursor(&display, 150, false, true); + } + + igt_subtest("2x-long-flip-vs-cursor-atomic-with-test") { + display.force_test_atomic = true; + two_screens_flip_vs_cursor(&display, 150, false, true); + } + + igt_subtest("2x-long-cursor-vs-flip-legacy") { + display.force_test_atomic = false; + two_screens_cursor_vs_flip(&display, 50, false); + } - igt_subtest("2x-long-cursor-vs-flip-legacy") + igt_subtest("2x-long-cursor-vs-flip-legacy-with-test") { + display.force_test_atomic = true; two_screens_cursor_vs_flip(&display, 50, false); + } - igt_subtest("2x-nonblocking-modeset-vs-cursor-atomic") + igt_subtest("2x-nonblocking-modeset-vs-cursor-atomic") { + display.force_test_atomic = false; two_screens_flip_vs_cursor(&display, 8, true, true); + } - igt_subtest("2x-cursor-vs-flip-atomic") + igt_subtest("2x-nonblocking-modeset-vs-cursor-atomic-with-test") { + display.force_test_atomic = true; + two_screens_flip_vs_cursor(&display, 8, true, true); + } + + igt_subtest("2x-cursor-vs-flip-atomic") { + display.force_test_atomic = false; + two_screens_cursor_vs_flip(&display, 8, true); + } + + igt_subtest("2x-cursor-vs-flip-atomic-with-test") { + display.force_test_atomic = true; two_screens_cursor_vs_flip(&display, 8, true); + } - igt_subtest("2x-long-nonblocking-modeset-vs-cursor-atomic") + igt_subtest(
Re: [Intel-gfx] [PATCH 06/19] drm/vmwgfx: Drop the cursor locking hack
On Thu, Mar 23, 2017 at 11:32:49AM +0100, Thomas Hellstrom wrote: > On 03/23/2017 11:10 AM, Daniel Vetter wrote: > > On Thu, Mar 23, 2017 at 09:35:25AM +0100, Thomas Hellstrom wrote: > >> Hi, Daniel, > >> > >> On 03/23/2017 08:31 AM, Daniel Vetter wrote: > >>> On Thu, Mar 23, 2017 at 08:28:32AM +0100, Daniel Vetter wrote: > On Thu, Mar 23, 2017 at 07:22:31AM +0100, Thomas Hellstrom wrote: > > On 03/22/2017 10:50 PM, Daniel Vetter wrote: > >> It's been around forever, no one bothered to address the FIXME, so I > >> presume it's all fine. > >> > >> Cc: Sinclair Yeh > >> Cc: Thomas Hellstrom > >> Signed-off-by: Daniel Vetter > > NAK. We need to properly address this. Probably as part of the atomic > > update. > So could someone with vmwgfx understanding explain this? Note that the > FIXME was originally added by me years ago, because I wasn't sure (only > about 90%) that this is safe, and was essentially pleading for a vmwgfx > expert to review this? > > Since it didn't happen I presume it's not that terribly and probably safe > ... > > I'm still 90% sure that this is correct, but I'd love for a vmwgfx to > audit it. Replying with a NAK is kinda not the response I was hoping for > (and yes I guess I should have explained what's going on here better, but > it's just a git blame of the FIXME comment away). > >> So the code has been left in place because it works. Altering it now > >> will create unnecessary merge conflicts with the atomic code, and the > >> change isn't tested and audited which means we need to drop focus from > >> what we're doing and audit and test code that isn't going to be used > >> anyway for not apparent reason? But otoh put in the below context there > >> indeed is a reason. > >> > >> From a quick audit of the existing code it seems like at least > >> vmw_cursor_update_position is touching global device state so I think at > >> a minimum we need to take a spinlock in that function. Otherwise it > >> seems to be safe. > > Note that you're holding the crtc lock already, which gives you exclusion > > against concurrent page_flips, mode_sets and property changes. Note also > > that page_flips themselves also only hold the crtc lock, so you can run > > multiple page_flips in parallel on different crtc (iirc vmwgfx has > > multiple crtc, if not this discussion is entirely moot). > > > > tbh I'd be surprised if my patch really breaks something that hasn't been > > a pre-existing issue for a long time. The original commit which added this > > FIXME comment is from 2012. Note also that because it's a hack, you > > already have a pretty a real race with the core drm state keeping, and no > > one seems to have hit that either. > > > > I mean I can dig through vmwgfx code and do the audit, but it'll take a > > few hours and vmwgfx is it's own world, so much harder to understand (for > > me). > > > > I'm thinking of the situation when someone would call a cursor_set ioctl > in parallell > for two crtcs at the same time and race writing the position registers? > Note that the device has only a single global cursor. > Admittedly the effects of a race would probably be small, but I'd rather > see it being > properly protected. Hm, didn't realize you only have 1 cursor for everything together. In that case you indeed have a problem. Not sure why that didn't come up 4 years ago with the original patch, would be pretty easy to add a quite mutex in v2 ... Since read-only global state is perfectly fine, having the crtc lock gives you a read-only global state lock (for legacy drivers at least, not for atomic). > > >> But I prefer if we can do that as part of the atomic update? > > When does that vmwgfx atomic happen? > > We're targeting 4.12, which means the code that is currently under > testing will need to be sent out for review pretty soon. > It's already in our standalone testing repo at > > git://git.freedesktop.org/git/mesa/vmwgfx Deadline is in 2 weeks for 4.12 feature work, per the discussion we've had after the 4.11 merge window fallout with Linus. You pretty much have to submit the patches now to have a reasonable chance of them landing in time. Since vmwgfx has traditionally been the odd kms driver out I'd really like to give the new atomic code at least a quick read-through, to make sure it's aligned as much as possible with the other 20+ atomic drivers. > but the cursor code hasn't been fixed in that repo yet. Well if you switched to universal planes it's pretty easy to fix with the acquire ctx and grabbing mode_config.connection_mutex. Without that you can just add a global cursor mutex (equally few lines) to patch it up. > > BTW is this blocking some other core drm work you're doing? Just removing lock_crtc and preventing abuse from spreading. Somehow both tegra and tilcdc starting using it in places it was definitely not meant for. vmwgfx (with this FIXME here) was the only legit
Re: [Intel-gfx] [PATCH] drm/scdc: declare drm_scdc_get_scrambling_status
On Wed, 22 Mar 2017, "Sharma, Shashank" wrote: > Thanks for this patch, Jani. > > Reviewed-by: Shashank Sharma And pushed to drm-misc-next, thanks for the review. BR, Jani. > > > Regards > Shashank > On 3/22/2017 4:33 PM, Jani Nikula wrote: >> Fix sparse warning: >> >> drivers/gpu/drm/drm_scdc_helper.c:138:6: warning: symbol >> 'drm_scdc_get_scrambling_status' was not declared. Should it be static? >> >> Fixes: 62c58af32c93 ("drm/edid: detect SCDC support in HF-VSDB") >> Cc: Shashank Sharma >> Signed-off-by: Jani Nikula >> --- >> include/drm/drm_scdc_helper.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/include/drm/drm_scdc_helper.h b/include/drm/drm_scdc_helper.h >> index ab6bcfbceba9..c25122bb490a 100644 >> --- a/include/drm/drm_scdc_helper.h >> +++ b/include/drm/drm_scdc_helper.h >> @@ -129,6 +129,8 @@ static inline int drm_scdc_writeb(struct i2c_adapter >> *adapter, u8 offset, >> return drm_scdc_write(adapter, offset, &value, sizeof(value)); >> } >> >> +bool drm_scdc_get_scrambling_status(struct i2c_adapter *adapter); >> + >> /** >>* drm_scdc_set_scrambling - enable scrambling >>* @adapter: I2C adapter for DDC channel > -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 2/7] tests/kms_plane_multiple: Add TEST_ONLY flag
Add TEST_ONLY flag to test atomic modesetting commits without actual real-life commit. v2: Use flag to indicate to test with TEST_ONLY flag with atomic commit v3: Moved force_test_atomic flag set to 'test_plane_position()' v4: Fix typo in subject field Signed-off-by: Mika Kahola --- tests/kms_plane_multiple.c | 48 -- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c index 93dce6b..84fd411 100644 --- a/tests/kms_plane_multiple.c +++ b/tests/kms_plane_multiple.c @@ -365,7 +365,8 @@ test_legacy_plane_position_with_output(data_t *data, enum pipe pipe, } static void -test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling) +test_plane_position(data_t *data, enum pipe pipe, bool atomic, + bool force_test_atomic, uint64_t tiling) { igt_output_t *output; int connected_outs; @@ -379,6 +380,8 @@ test_plane_position(data_t *data, enum pipe pipe, bool atomic, uint64_t tiling) tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED)) igt_require(AT_LEAST_GEN(devid, 9)); + data->display.force_test_atomic = force_test_atomic; + if (!opt.user_seed) opt.seed = time(NULL); @@ -421,46 +424,71 @@ run_tests_for_pipe(data_t *data, enum pipe pipe) igt_require(data->display.pipes[pipe].n_planes > 0); } - igt_subtest_f("legacy-pipe-%s-tiling-none", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_DRM_FORMAT_MOD_NONE); + test_plane_position(data, pipe, false, false, + LOCAL_DRM_FORMAT_MOD_NONE); igt_subtest_f("atomic-pipe-%s-tiling-none", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("legacy-pipe-%s-tiling-x", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("atomic-pipe-%s-tiling-x", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_X_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_X_TILED); igt_subtest_f("legacy-pipe-%s-tiling-y", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_Y_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_Y_TILED); igt_subtest_f("atomic-pipe-%s-tiling-y", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_Y_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_Y_TILED); igt_subtest_f("legacy-pipe-%s-tiling-yf", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, false, LOCAL_I915_FORMAT_MOD_Yf_TILED); + test_plane_position(data, pipe, false, false, + LOCAL_I915_FORMAT_MOD_Yf_TILED); igt_subtest_f("atomic-pipe-%s-tiling-yf", kmstest_pipe_name(pipe)) for_each_valid_output_on_pipe(&data->display, pipe, output) - test_plane_position(data, pipe, true, LOCAL_I915_FORMAT_MOD_Yf_TILED); + test_plane_position(data, pipe, true, false, + LOCAL_I915_FORMAT_MOD_Yf_TILED); + + igt_subtest_f("atomic-pipe-%s-tiling-x-with-test", + kmstest_pipe_name(pipe)) + for_each_valid_output_on_pipe(&data->display, pipe, output) + test_pl
[Intel-gfx] [PATCH] drm/i915: Pass vma to relocate entry
We can simplify our tracking of pending writes in an execbuf to the single bit in the vma->exec_entry->flags, but that requires the relocation function knowing the object's vma. Pass it along. Note we have only been using a single bit to track flushing since commit cc889e0f6ce6a63c62db17d702ecfed86d58083f Author: Daniel Vetter Date: Wed Jun 13 20:45:19 2012 +0200 drm/i915: disable flushing_list/gpu_write_list unconditionally flushed all render caches before the breadcrumb and commit 6ac42f4148bc27e5ffd18a9ab0eac57f58822af4 Author: Daniel Vetter Date: Sat Jul 21 12:25:01 2012 +0200 drm/i915: Replace the complex flushing logic with simple invalidate/flush all did away with the explicit GPU domain tracking. This was then codified into the ABI with NO_RELOC in commit ed5982e6ce5f106abcbf071f80730db344a6da42 Author: Daniel Vetter # Oi! Patch stealer! Date: Thu Jan 17 22:23:36 2013 +0100 drm/i915: Allow userspace to hint that the relocations were known Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 101 - 1 file changed, 41 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 27868798eb85..a07375bad449 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -622,42 +622,25 @@ relocate_entry(struct drm_i915_gem_object *obj, } static int -eb_relocate_entry(struct drm_i915_gem_object *obj, +eb_relocate_entry(struct i915_vma *vma, struct i915_execbuffer *eb, struct drm_i915_gem_relocation_entry *reloc) { - struct drm_gem_object *target_obj; - struct drm_i915_gem_object *target_i915_obj; - struct i915_vma *target_vma; - uint64_t target_offset; + struct i915_vma *target; + u64 target_offset; int ret; /* we've already hold a reference to all valid objects */ - target_vma = eb_get_vma(eb, reloc->target_handle); - if (unlikely(target_vma == NULL)) + target = eb_get_vma(eb, reloc->target_handle); + if (unlikely(!target)) return -ENOENT; - target_i915_obj = target_vma->obj; - target_obj = &target_vma->obj->base; - - target_offset = gen8_canonical_addr(target_vma->node.start); - - /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and -* pipe_control writes because the gpu doesn't properly redirect them -* through the ppgtt for non_secure batchbuffers. */ - if (unlikely(IS_GEN6(eb->i915) && -reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION)) { - ret = i915_vma_bind(target_vma, target_i915_obj->cache_level, - PIN_GLOBAL); - if (WARN_ONCE(ret, "Unexpected failure to bind target VMA!")) - return ret; - } /* Validate that the target is in a valid r/w GPU domain */ if (unlikely(reloc->write_domain & (reloc->write_domain - 1))) { DRM_DEBUG("reloc with multiple write domains: " - "obj %p target %d offset %d " + "target %d offset %d " "read %08x write %08x", - obj, reloc->target_handle, + reloc->target_handle, (int) reloc->offset, reloc->read_domains, reloc->write_domain); @@ -666,43 +649,56 @@ eb_relocate_entry(struct drm_i915_gem_object *obj, if (unlikely((reloc->write_domain | reloc->read_domains) & ~I915_GEM_GPU_DOMAINS)) { DRM_DEBUG("reloc with read/write non-GPU domains: " - "obj %p target %d offset %d " + "target %d offset %d " "read %08x write %08x", - obj, reloc->target_handle, + reloc->target_handle, (int) reloc->offset, reloc->read_domains, reloc->write_domain); return -EINVAL; } - target_obj->pending_read_domains |= reloc->read_domains; - target_obj->pending_write_domain |= reloc->write_domain; + if (reloc->write_domain) + target->exec_entry->flags |= EXEC_OBJECT_WRITE; + + /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and +* pipe_control writes because the gpu doesn't properly redirect them +* through the ppgtt for non_secure batchbuffers. +*/ + if (unlikely(IS_GEN6(eb->i915) && +reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION)) { + ret = i915_vma_bind(target, target->obj->cache_level, + PIN_GLOBAL); +
Re: [Intel-gfx] [PATCH 02/19] drm: Add acquire ctx parameter to ->update_plane
On Wed, Mar 22, 2017 at 10:50:41PM +0100, Daniel Vetter wrote: > diff --git a/drivers/gpu/drm/armada/armada_overlay.c > b/drivers/gpu/drm/armada/armada_overlay.c > index 34cb73d0db77..b54fd8cbd3a6 100644 > --- a/drivers/gpu/drm/armada/armada_overlay.c > +++ b/drivers/gpu/drm/armada/armada_overlay.c > @@ -94,7 +94,8 @@ static int > armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, > struct drm_framebuffer *fb, > int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h, > - uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h) > + uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h, > + struct drm_modeset_acquire_ctx *ctx) I'm rather unhappy that we're ending up with a function taking soo many arguments. Most of these have to be stacked on ARM, and I'm guessing most architectures end up doing something similar. Is there a reason why we don't pass pointers to drm_rect's or maybe even consider passing the drm_plane_state structure in? I've found that, when cleaning up these code paths in armada, that storing all the parameters into a drm_plane_state and then validating it with drm_plane_helper_check_state() is by way the simplest solution, and of course, it's forward-compatible with atomic modeset. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
== Series Details == Series: drm/i915/kvmgt: avoid dereferencing a potentially null info pointer URL : https://patchwork.freedesktop.org/series/21762/ State : warning == Summary == Series 21762v1 drm/i915/kvmgt: avoid dereferencing a potentially null info pointer https://patchwork.freedesktop.org/api/1.0/series/21762/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-snb-2600) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 455s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 456s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 582s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 547s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 508s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 507s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 441s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 434s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 441s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 521s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 496s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 485s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 486s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 603s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 484s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 523s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 465s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 546s fi-snb-2600 total:278 pass:248 dwarn:1 dfail:0 fail:0 skip:29 time: 420s f99a2e3df860d56c83a726a1183162cb467b2ad4 drm-tip: 2017y-03m-23d-12h-06m-41s UTC integration manifest eee6d67 drm/i915/kvmgt: avoid dereferencing a potentially null info pointer == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4277/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for Various improvements around the GuC topic
Merged this series except the HAX patch (also, reordered the S-o-b, R-b and Cc lines to canonical form), so do rebase your work. Regards, Joonas On to, 2017-03-23 at 11:06 +, Patchwork wrote: > == Series Details == > > Series: Various improvements around the GuC topic > URL : https://patchwork.freedesktop.org/series/21726/ > State : success > > == Summary == > > Series 21726v1 Various improvements around the GuC topic > https://patchwork.freedesktop.org/api/1.0/series/21726/revisions/1/mbox/ > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 > time: 460s > fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 > time: 461s > fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 > time: 593s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 > time: 523s > fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 > time: 511s > fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 > time: 502s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time: 441s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time: 431s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 > time: 436s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time: 509s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time: 496s > fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time: 478s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time: 468s > fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 > time: 588s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 > time: 480s > fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time: 513s > fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 > time: 446s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time: 558s > fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 > time: 416s > > 8229a8c712c22ff8e94e3244d4fd942a7dcd89af drm-tip: 2017y-03m-23d-09h-57m-34s > UTC integration manifest > f003ba2 HAX Enable GuC loading & submission > 43d51b6 drm/i915/guc: Move guc_interrupts_release next to > guc_interrupts_capture > 19b8f8a drm/i915/guc: Split out the mmio_white_list struct > 22675ac drm/i915/guc: Refactor the concept "GuC context descriptor" into "GuC > stage descriptor" > 98c3280 drm/i915/guc: A little bit more of doorbell sanitization > a5d1810 drm/i915/guc: Wait for doorbell to be inactive before deallocating > cf3850d drm/i915/guc: Improve the GuC documentation & comments about proxy > submissions > 7112b1b drm/i915/guc: Make intel_guc_send a function pointer > cdbbc12 drm/i915/guc: Break out the GuC log extras into their own "runtime" > struct > 0009b27 drm/i915/guc: The Additional Data Struct (ADS) should get enabled > together with GuC submission > c11c121 drm/i915/guc: Add onion teardown to the GuC setup > d1a1ced drm/i915/guc: Keep the ctx_pool_vaddr mapped, for easy access > 5956a8b drm/i915/guc: Sanitize GuC client initialization > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4273/ > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Make the decision to keep vblank irq enabled earlier
On Thu, Mar 23, 2017 at 07:51:06AM +, Chris Wilson wrote: > We want to provide the vblank irq shadow for pageflip events as well as > vblank queries. Such events are completed within the vblank interrupt > handler, and so the current check for disabling the irq will disable it > from with the same interrupt as the last pageflip event. If we move the > decision on whether to disable the irq (based on there no being no > remaining vblank events, i.e. vblank->refcount == 0) to before we signal > the events, we will only disable the irq on the interrupt after the last > event was signaled. In the normal course of events, this will keep the > vblank irq enabled for the entire flip sequence whereas before it would > flip-flop around every interrupt. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Michel Dänzer > Cc: Laurent Pinchart > Cc: Dave Airlie , > Cc: Mario Kleiner > --- > drivers/gpu/drm/drm_irq.c | 18 +++--- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 5b77057e91ca..1d6bcee3708f 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -1741,6 +1741,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned > int pipe) > { > struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; > unsigned long irqflags; > + bool disable_irq; > > if (WARN_ON_ONCE(!dev->num_crtcs)) > return false; > @@ -1768,16 +1769,19 @@ bool drm_handle_vblank(struct drm_device *dev, > unsigned int pipe) > spin_unlock(&dev->vblank_time_lock); > > wake_up(&vblank->queue); > - drm_handle_vblank_events(dev, pipe); > > /* With instant-off, we defer disabling the interrupt until after > - * we finish processing the following vblank. The disable has to > - * be last (after drm_handle_vblank_events) so that the timestamp > - * is always accurate. > + * we finish processing the following vblank after all events have > + * been signaled. The disable has to be last (after > + * drm_handle_vblank_events) so that the timestamp is always accurate. We wouldn't actually do the disable as long there's a reference still held, so the timestamp should be fine in that case. And if there aren't any references the timestamp shouldn't matter... I think. But it's probably more clear to keep to the order you propose here anyway. Reviewed-by: Ville Syrjälä Oh, and now that I think about this stuff again, I start to wonder why I made the disable actually update the seq/ts. If the interrupt is currently enabled the seq/ts should be reasonably uptodate already when we do disable the interrupt. Perhaps I was only thinking about drm_vblank_off() when I made that change, or I decided that I didn't want two different disable codepaths. Anyways, just an idea that we might be able to make the vblank irq disable a little cheaper. >*/ > - if (dev->vblank_disable_immediate && > - drm_vblank_offdelay > 0 && > - !atomic_read(&vblank->refcount)) > + disable_irq = (dev->vblank_disable_immediate && > +drm_vblank_offdelay > 0 && > +!atomic_read(&vblank->refcount)); > + > + drm_handle_vblank_events(dev, pipe); > + > + if (disable_irq) > vblank_disable_fn((unsigned long)vblank); > > spin_unlock_irqrestore(&dev->event_lock, irqflags); > -- > 2.11.0 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
> -Original Message- > From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com] > Sent: Thursday, March 23, 2017 5:38 PM > To: Dong, Chuanxiao; intel-gfx@lists.freedesktop.org > Cc: intel-gvt-...@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification > for guc submission > > On ke, 2017-03-22 at 14:34 +0800, Chuanxiao Dong wrote: > > GVT request needs a manual mmio load/restore. Before GuC submit a > > request, send notification to gvt for mmio loading. And after the GuC > > finished this GVT request, notify gvt again for mmio restore. This > > follows the usage when using execlists submission. > > > > v2: use context_status_change instead of > > execlists_context_status_change > > for better understanding (ZhengXiao) > > v3: remove the comment as it is obvious and not friendly to > > the caller (Kevin) > > > > Cc: xiao.zh...@intel.com > > Cc: kevin.t...@intel.com > > Signed-off-by: Chuanxiao Dong > > > > > @@ -350,7 +335,7 @@ static void execlists_submit_ports(struct > > intel_engine_cs *engine) > > > > GEM_BUG_ON(port[0].count > 1); > > if (!port[0].count) > > - execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > INTEL_CONTEXT_SCHEDULE_IN); > > Fix indent. > > > desc[0] = execlists_update_context(port[0].request); > > GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); > @@ > > -358,7 +343,7 @@ static void execlists_submit_ports(struct > > intel_engine_cs *engine) > > > > if (port[1].request) { > > GEM_BUG_ON(port[1].count); > > - execlists_context_status_change(port[1].request, > > + context_status_change(port[1].request, > > > INTEL_CONTEXT_SCHEDULE_IN); > > Ditto. > > > @@ -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data) > > if (--port[0].count == 0) { > > GEM_BUG_ON(status & > GEN8_CTX_STATUS_PREEMPTED); > > > GEM_BUG_ON(!i915_gem_request_completed(port[0].request)); > > - > execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > INTEL_CONTEXT_SCHEDULE_OUT); > > Ditto. > > > @@ -87,5 +87,18 @@ uint64_t intel_lr_context_descriptor(struct > > i915_gem_context *ctx, > > /* Execlists */ > > int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv, > > int enable_execlists); > > +static inline void > > +context_status_change(struct drm_i915_gem_request *rq, unsigned long > > +status) > > This needs intel_lr_ prefix now that it's in .h file. > > With those changes (make sure context_status_change doesn't become > over character 80 line), this is; Sure. Will fix in the next version. Thanks Chuanxiao > > Reviewed-by: Joonas Lahtinen > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/execlists: Relax the locked clear_bit(IRQ_EXECLIST)
We only need to care about the ordering of the clearing of the bit with the uncached CSB read in order to correctly detect a new interrupt before the read completes. The uncached read itself acts as a full memory barrier, so we do not to enforce another in the form of a locked clear_bit. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_lrc.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 59acdd3b7a12..c9010fa881b4 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -540,7 +540,14 @@ static void intel_lrc_irq_handler(unsigned long data) dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)); unsigned int csb, head, tail; - clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); + /* The write will be ordered by the uncached read (itself +* a memory barrier), we do need another in the form a +* locked instruction. That is the clear of IRQ_EXECLIST bit +* will be visible to another cpu prior to the completion +* of the CSB read. If the other cpu receives an interrupt +* before then, we will redo the CSB read. +*/ + __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); csb = readl(csb_mmio); head = GEN8_CSB_READ_PTR(csb); tail = GEN8_CSB_WRITE_PTR(csb); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
> -Original Message- > From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On > Behalf Of Chris Wilson > Sent: Thursday, March 23, 2017 5:43 PM > To: Joonas Lahtinen > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org; > Dong, Chuanxiao > Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification > for guc submission > > On Thu, Mar 23, 2017 at 11:37:32AM +0200, Joonas Lahtinen wrote: > > On ke, 2017-03-22 at 14:34 +0800, Chuanxiao Dong wrote: > > > @@ -87,5 +87,18 @@ uint64_t intel_lr_context_descriptor(struct > > > i915_gem_context *ctx, > > > /* Execlists */ > > > int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv, > > > int enable_execlists); > > > +static inline void > > > +context_status_change(struct drm_i915_gem_request *rq, unsigned > > > +long status) > > > > This needs intel_lr_ prefix now that it's in .h file. > > But not in this header. The event is not strictly tied to execlists, it > consumes > request for starters - but on the other hand it is gvt specific. > > I'm tempted to say intel_gvt_notify_context_status(). It makes sense. Will change the name in the next version. Thanks Chuanxiao > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > intel-gvt-dev mailing list > intel-gvt-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
> -Original Message- > From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] > Sent: Thursday, March 23, 2017 5:52 PM > To: Dong, Chuanxiao; intel-gfx@lists.freedesktop.org > Cc: intel-gvt-...@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification > for guc submission > > > On 22/03/2017 06:34, Chuanxiao Dong wrote: > > GVT request needs a manual mmio load/restore. Before GuC submit a > > request, send notification to gvt for mmio loading. And after the GuC > > finished this GVT request, notify gvt again for mmio restore. This > > follows the usage when using execlists submission. > > > > v2: use context_status_change instead of > execlists_context_status_change > > for better understanding (ZhengXiao) > > v3: remove the comment as it is obvious and not friendly to > > the caller (Kevin) > > > > Cc: xiao.zh...@intel.com > > Cc: kevin.t...@intel.com > > Signed-off-by: Chuanxiao Dong > > --- > > drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++ > > drivers/gpu/drm/i915/intel_lrc.c | 21 +++-- > > drivers/gpu/drm/i915/intel_lrc.h | 13 + > > 3 files changed, 19 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > > b/drivers/gpu/drm/i915/i915_guc_submission.c > > index 055467a..0195547 100644 > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > > @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct > drm_i915_gem_request *rq) > > unsigned long flags; > > int b_ret; > > > > + context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); > > + > > /* WA to flush out the pending GMADR writes to ring buffer. */ > > if (i915_vma_is_map_and_fenceable(rq->ring->vma)) > > POSTING_READ_FW(GUC_STATUS); > > @@ -634,6 +636,7 @@ static void i915_guc_irq_handler(unsigned long data) > > rq = port[0].request; > > while (rq && i915_gem_request_completed(rq)) { > > trace_i915_gem_request_out(rq); > > + context_status_change(rq, > INTEL_CONTEXT_SCHEDULE_OUT); > > Does GVT care that the context will still be active on the GPU for a small > window after this notification? (User interrupt happens before context > complete, which GuC hides from the driver.) Actually GVT cares. GVT driver will check the context status register to make sure the status is ACTIVE_IDLE in this notification before manually doing the context switch out for the GuC submission case. Thanks Chuanxiao > > Regards, > > Tvrtko > > > i915_gem_request_put(rq); > > port[0].request = port[1].request; > > port[1].request = NULL; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index eec1e71..24c69b5 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct > i915_gem_context *ctx, > > return ctx->engine[engine->id].lrc_desc; } > > > > -static inline void > > -execlists_context_status_change(struct drm_i915_gem_request *rq, > > - unsigned long status) > > -{ > > - /* > > -* Only used when GVT-g is enabled now. When GVT-g is disabled, > > -* The compiler should eliminate this function as dead-code. > > -*/ > > - if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) > > - return; > > - > > - atomic_notifier_call_chain(&rq->engine->context_status_notifier, > > - status, rq); > > -} > > - > > static void > > execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 > > *reg_state) { @@ -350,7 +335,7 @@ static void > > execlists_submit_ports(struct intel_engine_cs *engine) > > > > GEM_BUG_ON(port[0].count > 1); > > if (!port[0].count) > > - execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > INTEL_CONTEXT_SCHEDULE_IN); > > desc[0] = execlists_update_context(port[0].request); > > GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); > @@ > > -358,7 +343,7 @@ static void execlists_submit_ports(struct > > intel_engine_cs *engine) > > > > if (port[1].request) { > > GEM_BUG_ON(port[1].count); > > - execlists_context_status_change(port[1].request, > > + context_status_change(port[1].request, > > > INTEL_CONTEXT_SCHEDULE_IN); > > desc[1] = execlists_update_context(port[1].request); > > GEM_DEBUG_EXEC(port[1].context_id = > upper_32_bits(desc[1])); @@ > > -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data) > > if (--port[0].count == 0) { > > GEM_BUG_ON(status & > GEN8_CTX_STATUS_PREEMPTED); > > > GEM_BUG_ON(!i915_gem_request_completed(port[0].request
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [01/15] drm/i915: Copy user requested buffers into the error state (rev4)
== Series Details == Series: series starting with [01/15] drm/i915: Copy user requested buffers into the error state (rev4) URL : https://patchwork.freedesktop.org/series/21377/ State : failure == Summary == drivers/gpu/drm/i915/i915_gem_execbuffer.c:1891:1: error: expected expression before ‘<<’ token <<< b63fa3700fd66618174ee4d422c0867a0fc30e0f ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:2184:1: error: expected declaration or statement at end of input } ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1889:3: error: label ‘err_vma’ used but not defined goto err_vma; ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1873:3: error: label ‘err_unlock’ used but not defined goto err_unlock; ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1869:3: error: label ‘err_rpm’ used but not defined goto err_rpm; ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1853:4: error: label ‘err_in_fence’ used but not defined goto err_in_fence; ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1793:20: error: unused variable ‘out_fence’ [-Werror=unused-variable] struct sync_file *out_fence = NULL; ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c: At top level: drivers/gpu/drm/i915/i915_gem_execbuffer.c:442:1: error: ‘eb_reserve_vma’ defined but not used [-Werror=unused-function] eb_reserve_vma(struct i915_execbuffer *eb, struct i915_vma *vma) ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:773:13: error: ‘eb_release_vma’ defined but not used [-Werror=unused-function] static void eb_release_vma(const struct i915_execbuffer *eb) ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:796:13: error: ‘eb_destroy’ defined but not used [-Werror=unused-function] static void eb_destroy(const struct i915_execbuffer *eb) ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1546:1: error: ‘i915_gem_check_execbuffer’ defined but not used [-Werror=unused-function] i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec) ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1630:25: error: ‘eb_parse’ defined but not used [-Werror=unused-function] static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master) ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1672:1: error: ‘add_to_client’ defined but not used [-Werror=unused-function] add_to_client(struct drm_i915_gem_request *req, ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1680:1: error: ‘eb_submit’ defined but not used [-Werror=unused-function] eb_submit(struct i915_execbuffer *eb) ^ drivers/gpu/drm/i915/i915_gem_execbuffer.c:1786:1: error: ‘i915_gem_do_execbuffer’ defined but not used [-Werror=unused-function] i915_gem_do_execbuffer(struct drm_device *dev, ^ cc1: all warnings being treated as errors scripts/Makefile.build:294: recipe for target 'drivers/gpu/drm/i915/i915_gem_execbuffer.o' failed make[4]: *** [drivers/gpu/drm/i915/i915_gem_execbuffer.o] Error 1 make[4]: *** Waiting for unfinished jobs LD drivers/usb/gadget/udc/udc-core.o LD drivers/video/console/built-in.o LD drivers/usb/gadget/udc/built-in.o LD drivers/video/built-in.o LD drivers/usb/gadget/built-in.o LD [M] drivers/net/ethernet/intel/e1000/e1000.o LD net/ipv6/ipv6.o LD net/ipv6/built-in.o LD drivers/tty/serial/8250/8250_base.o LD drivers/scsi/sd_mod.o LD drivers/tty/serial/8250/built-in.o LD drivers/scsi/built-in.o AR lib/lib.a LD drivers/tty/serial/built-in.o LD [M] drivers/net/ethernet/intel/igb/igb.o EXPORTS lib/lib-ksyms.o LD lib/built-in.o LD net/xfrm/built-in.o LD drivers/usb/core/usbcore.o LD drivers/usb/core/built-in.o LD drivers/md/md-mod.o LD drivers/md/built-in.o CC arch/x86/kernel/cpu/capflags.o LD arch/x86/kernel/cpu/built-in.o LD arch/x86/kernel/built-in.o LD drivers/tty/vt/built-in.o LD drivers/usb/host/xhci-hcd.o LD drivers/tty/built-in.o LD fs/btrfs/btrfs.o LD fs/ext4/ext4.o LD arch/x86/built-in.o LD drivers/usb/host/built-in.o LD [M] drivers/net/ethernet/intel/e1000e/e1000e.o LD drivers/usb/built-in.o LD fs/ext4/built-in.o LD fs/btrfs/built-in.o LD net/core/built-in.o LD fs/built-in.o LD net/ipv4/built-in.o LD net/built-in.o LD drivers/net/ethernet/built-in.o LD drivers/net/built-in.o scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2 scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:553: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:1002: recipe for target 'drivers' failed make: *** [drivers] Error 2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman
Re: [Intel-gfx] [PATCH 00/14] drm/i915: Moar plane update optimizations
On Fri, Mar 17, 2017 at 11:17:54PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > The plane updates are still taking far too long, at least with > heavy kernel debug knobs turned on. Using kms_atomic_transitions > I'm currently seeing numbers as high as 170 usec on my VLV. > > To combat lockdep the most beneficial thing is taking the > uncore.lock around the entire pipe update. Combined with all > the other optimizations here I was able to push the max > duration below 60 usec with my debug kernel. > > The pre-compute stuff isn't supremely important with lockdep/etc. > turned on, but once those are turned off the few usec saved by > the other optimizations start to matter. With all the optimization > and a less debug heavy kernel I was able to get the max duration > to around 15 usec. It was around 25 usec with the current code. > > Mika was saying that he's still seeing huge numbers (as high > as 400 usec) with the current drm-tip, and that wasn't even with > a particularly debug heavy kernel. One theory is that there's > contention on the uncore.lock. So I'm thinking we may want to > split the lock into two; one for gt, the other for display. Not > starving the GPU by hogging the shared lock for display stuff > might also be a good thing. I've not tried playing with more > GPU heavy scenarios yet > > Anyways, running out of time to play with this more today so > I figured I'd post what I have now. > > Series available here: > git://github.com/vsyrjala/linux.git plane_update_optimization > > Ville Syrjälä (14): > drm/i915: Extract skl_plane_ctl() > drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes > drm/i915: Extract vlv_sprite_ctl() > drm/i915: Extract ivb_sprite_ctl() > drm/i915: Extract ilk_sprite_ctl() > drm/i915: Extract i845_cursor_ctl() and i9xx_cursor_ctl() I've pushed these to dinq. Thanks for the review. > drm/i915: Extract i9xx_plane_ctl() > drm/i915: Pre-compute plane control register value > drm/i915: Introduce i9xx_check_plane_surface() > drm/i915: Eliminate ironlake_update_primary_plane() > drm/i915: Use i9xx_check_plane_surface() for sprite planes as well I'll repost these as new series. > drm/i915: Keep vblanks enabled during the entire pipe update > WIP/drm/i915: Protect the entire pipe update with uncore.lock > WIP/drm/i915: Nuke posting reads from plane updates And these I'll probably leave out from the next series, and repost separately later. > > drivers/gpu/drm/i915/i915_irq.c | 66 -- > drivers/gpu/drm/i915/i915_trace.h| 28 +-- > drivers/gpu/drm/i915/intel_display.c | 436 > +-- > drivers/gpu/drm/i915/intel_drv.h | 16 +- > drivers/gpu/drm/i915/intel_pm.c | 11 +- > drivers/gpu/drm/i915/intel_sprite.c | 355 > 6 files changed, 440 insertions(+), 472 deletions(-) > > -- > 2.10.2 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/execlists: Relax the locked clear_bit(IRQ_EXECLIST)
== Series Details == Series: drm/i915/execlists: Relax the locked clear_bit(IRQ_EXECLIST) URL : https://patchwork.freedesktop.org/series/21767/ State : failure == Summary == Series 21767v1 drm/i915/execlists: Relax the locked clear_bit(IRQ_EXECLIST) https://patchwork.freedesktop.org/api/1.0/series/21767/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload: pass -> INCOMPLETE (fi-byt-j1900) Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-byt-j1900) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> FAIL (fi-byt-j1900) Subgroup basic-rte: pass -> FAIL (fi-byt-j1900) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 468s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 462s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 577s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 539s fi-byt-j1900 total:274 pass:244 dwarn:1 dfail:0 fail:2 skip:26 time: 0s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 502s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 444s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 435s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 516s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 488s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 480s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 480s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 598s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 489s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 522s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 457s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 549s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 419s f05fca81afc25c60190fae715e09c3554b16a012 drm-tip: 2017y-03m-23d-13h-08m-10s UTC integration manifest 8149945 drm/i915/execlists: Relax the locked clear_bit(IRQ_EXECLIST) == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4279/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
Dropping the irrelevant Cc's. On to, 2017-03-23 at 12:39 +, Chris Wilson wrote: > On Thu, Mar 23, 2017 at 12:22:30PM +, Colin King wrote: > > > > From: Colin Ian King > > > > info is being checked to see if it is a null pointer, however, vpgu is > > dereferencing info before this check, leading to a potential null > > pointer dereference. If info is null, then the error message being > > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > > use a null vpgu as the macro has a sanity check to see if vpgu is null, > > so this is OK. > > It is never NULL, it gets checked by its only caller. Took me a while to make any sense of the code as gvt_vgpu_err depends on a vgpu variable being declared in the scope without taking it as a parameter and that is a one big no-no: https://01.org/linuxgraphics/gfx-docs/drm/process/coding-style.html#macros-enums-and-rtl Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Store a direct lookup from object handle to vma
On Wed, Mar 22, 2017 at 04:22:38PM +0200, Joonas Lahtinen wrote: > + Daniel for the rsvd2 I'd go with a shadow struct definition which matches the uapi one exactly, except for having a proper name and type for this ... Or we do the anynomous union thing again. -Daniel > > On ke, 2017-03-22 at 09:33 +, Chris Wilson wrote: > > The advent of full-ppgtt lead to an extra indirection between the object > > and its binding. That extra indirection has a noticeable impact on how > > fast we can convert from the user handles to our internal vma for > > execbuffer. In order to bypass the extra indirection, we use a > > resizable hashtable to jump from the object to the per-ctx vma. > > rhashtable was considered but we don't need the online resizing feature > > and the extra complexity proved to undermine its usefulness. Instead, we > > simply reallocate the hastable on demand in a background task and > > serialize it before iterating. > > > > In non-full-ppgtt modes, multiple files and multiple contexts can share > > the same vma. This leads to having multiple possible handle->vma links, > > so we only use the first to establish the fast path. The majority of > > buffers are not shared and so we should still be able to realise > > speedups with multiple clients. > > > > v2: Prettier names, more magic. > > > > Signed-off-by: Chris Wilson > > > > > +static void resize_vma_ht(struct work_struct *work) > > +{ > > + struct i915_gem_context_vma_lut *lut = > > + container_of(work, typeof(*lut), resize); > > + unsigned int size, bits, new_bits, i; > > + struct hlist_head *new_ht; > > + > > + bits = 1 + ilog2(4*lut->ht_count/3); > > + new_bits = min_t(unsigned int, > > + max(bits, VMA_HT_BITS), > > + sizeof(unsigned int)*8); > > * BITS_PER_BYTE for extra clarity. > > > + if (new_bits == lut->ht_bits) > > + goto out; > > + > > + new_ht = kzalloc(sizeof(*new_ht)< > + if (!new_ht) > > + new_ht = vzalloc(sizeof(*new_ht)< > No vcalloc :( Otherwise would've suggested > > vzalloc(BIT(new_bits), sizeof(*new_ht), ...); > > but > > kzalloc(BIT(new_bits)*sizeof(*new_ht), ...) > > might still be clearer. > > > @@ -266,6 +331,16 @@ __create_hw_context(struct drm_i915_private *dev_priv, > > list_add_tail(&ctx->link, &dev_priv->context_list); > > ctx->i915 = dev_priv; > > > > + ctx->vma_lut.ht_bits = VMA_HT_BITS; > > + ctx->vma_lut.ht_size = BIT(VMA_HT_BITS); > > + ctx->vma_lut.ht = kcalloc(ctx->vma_lut.ht_size, > > + sizeof(*ctx->vma_lut.ht), > > + GFP_KERNEL); > > + if (!ctx->vma_lut.ht) > > + goto err_out; > > + > > Errors after this point will leak lut. Need err_lut label and call it > from further error gotos. > > > @@ -143,6 +143,31 @@ struct i915_gem_context { > > /** ggtt_offset_bias: placement restriction for context objects */ > > u32 ggtt_offset_bias; > > > > + struct i915_gem_context_vma_lut { > > + /** ht_size: last request size to allocate the hashtable for. */ > > + unsigned int ht_size; > > +#define RESIZE_IN_PROGRESS BIT(0) > > Easily conflicting name? Forward declare and hide in .c file? By making > ht[0] at the end, and maybe pull out work. Or maybe just prefix :P > > > +#define to_ptr(T, x) ((T *)(uintptr_t)(x)) > > i915_utils.h so we some day push to core. from_uintptr might make more > sense, though. > > The remainder is somewhat hard to review due to combined code motion > and changes becoming mess, but didn't find anything else but I still > dislike rsvd2 usage, and the magic bit it has. > > We could at least #define proper_name rsvd2 in the .c file... Jani > would be glad, I guess. I think if somebody touches a reserved field by > name, they deserve to have their build broken. > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation -- 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 v3 0/5] drm/i915: SKL+ render decompression support
Hi Ville, On 21 March 2017 at 18:12, wrote: > Another iteration of the i915 CCS support. Main change is lifting the > fb dimensions hsub/vsub alignment restrictions from the core. Without that > userspace would have to align the fb size be a multiple of 8x16 pixels > which isn't something they are interested in doing. With a rebase of Ben's GBM branch[0], and my last Weston atomic series, for Y_CCS this series is: Tested-by: Daniel Stone I had to disable Yf_CCS due to complaints about the kernel's idea of the tiling mode not matching the modifier. Not sure if this is something which has subsequently been fixed, or ... Cheers, Daniel [0]: https://git.collabora.com/cgit/user/daniels/mesa.git/log/?h=wip/2017-03/modifiers-v9-rebased ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [dim PATCH 1/7] dim: don't fail on grep not matching
On Thu, Mar 23, 2017 at 12:06:16PM +0200, Jani Nikula wrote: > Oops, the comment told us to watch out for this. > > Fixes: 56e53a49e28f ("dim: declare and assign separately") I just hacked around this with a || true :-) Ack. -Daniel > Signed-off-by: Jani Nikula > --- > dim | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/dim b/dim > index c1ac9e546ea9..ddcc18f17f0d 100755 > --- a/dim > +++ b/dim > @@ -1091,10 +1091,7 @@ function checkpatch_commit > git --no-pager log --oneline -1 $commit > $cmd | scripts/checkpatch.pl -q --emacs --strict - || true > > - # FIXME: this relies on local assignment not failing on command > - # substitution failures > - bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') > - if test "$bug_lines" -eq 1; then > + if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c > '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then > warn_or_fail "New BUG macro added" > fi > > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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] [dim PATCH 7/7] dim: propagate errors from check_maintainer
On Thu, Mar 23, 2017 at 12:06:22PM +0200, Jani Nikula wrote: > Signed-off-by: Jani Nikula Ack on the entire pile. -Daniel > --- > dim | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/dim b/dim > index 7b6275e7796f..989674ab7a91 100755 > --- a/dim > +++ b/dim > @@ -701,7 +701,9 @@ function dim_apply_branch > if ! checkpatch_commit HEAD; then > rv=1 > fi > - check_maintainer $branch HEAD > + if ! check_maintainer $branch HEAD; then > + rv=1 > + fi > > eval $DRY $DIM_POST_APPLY_ACTION > > @@ -1090,7 +1092,7 @@ function dim_conf > # $2 commit > function check_maintainer > { > - local branch commit > + local branch commit rv > > branch=$1 > commit=$2 > @@ -1101,8 +1103,11 @@ function check_maintainer > echo -e "The following files are outside of i915 > maintenance scope:\n" > echo "$non_i915_files" > echo -e "\nConfirm you have appropriate Acked-by and > Reviewed-by for above files." > + rv=1 > fi > fi > + > + return $rv > } > > # $1 is the git sha1 to check > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 v2] drm/i915: Store a direct lookup from object handle to vma
On Thu, Mar 23, 2017 at 03:23:28PM +0100, Daniel Vetter wrote: > On Wed, Mar 22, 2017 at 04:22:38PM +0200, Joonas Lahtinen wrote: > > + Daniel for the rsvd2 > > I'd go with a shadow struct definition which matches the uapi one exactly, > except for having a proper name and type for this ... Or we do the > anynomous union thing again. Definitely not an anon union since this is not part of the ABI, just taking advantage of the overallocation in the kernel copy of the user structs. I've plonked it behind a macro so that we have only one point of failure, though realistically expanding the struct or using a lut[] will take more massaging than just renaming the field. It's not a big job so I don't feel like the code is hamstringing our future selves. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Check we have an wake device before flushing GTT writes
We can assume that if the device is asleep then all pending GTT writes will have been posted, and so we can defer the flush from i915_gem_object_flush_gtt_write_domain() [ 1957.462568] WARNING: CPU: 0 PID: 6132 at drivers/gpu/drm/i915/intel_drv.h:1742 fwtable_read32+0x123/0x150 [i915] [ 1957.462582] RPM wakelock ref not held during HW access [ 1957.462583] Modules linked in: i915 intel_gtt drm_kms_helper prime_numbers [ 1957.462607] CPU: 0 PID: 6132 Comm: gem_concurrent_ Tainted: G U 4.11.0-rc1+ #464 [ 1957.462619] Hardware name: /, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 1957.462630] Call Trace: [ 1957.462646] dump_stack+0x4d/0x6f [ 1957.462657] __warn+0xc1/0xe0 [ 1957.462667] warn_slowpath_fmt+0x4a/0x50 [ 1957.462709] fwtable_read32+0x123/0x150 [i915] [ 1957.462750] i915_gem_object_flush_gtt_write_domain+0x43/0x70 [i915] [ 1957.462791] i915_gem_object_set_to_cpu_domain+0x46/0xa0 [i915] [ 1957.462831] i915_gem_set_domain_ioctl+0x15d/0x220 [i915] [ 1957.462843] drm_ioctl+0x1d7/0x440 [ 1957.462885] ? i915_gem_obj_prepare_shmem_write+0x1d0/0x1d0 [i915] [ 1957.462896] ? pick_next_task_fair+0x436/0x440 [ 1957.462906] ? mntput+0x1f/0x30 [ 1957.462915] do_vfs_ioctl+0x8f/0x5c0 [ 1957.462925] ? __schedule+0x16f/0x5f0 [ 1957.462935] ? fput+0x9/0x10 [ 1957.462943] SyS_ioctl+0x3c/0x70 [ 1957.462952] entry_SYSCALL_64_fastpath+0x17/0x98 [ 1957.462961] RIP: 0033:0x7fc542179ca7 [ 1957.462968] RSP: 002b:7ffeef12ff98 EFLAGS: 0246 ORIG_RAX: 0010 [ 1957.462982] RAX: ffda RBX: 7ffeef1301d0 RCX: 7fc542179ca7 [ 1957.462990] RDX: 7ffeef12ffd0 RSI: 400c645f RDI: 0003 [ 1957.462999] RBP: 0003 R08: 55f433bc7c40 R09: 002c [ 1957.463006] R10: 0073 R11: 0246 R12: 0018 [ 1957.463015] R13: 55f432c89d20 R14: 55f432c87690 R15: Fixes: 3b5724d702ef ("drm/i915: Wait for writes through the GTT to land before reading back") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: # v4.9 --- drivers/gpu/drm/i915/i915_gem.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 4e66daa12bc9..39d5b849367e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3104,9 +3104,12 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj) */ wmb(); if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) { - spin_lock_irq(&dev_priv->uncore.lock); - POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base)); - spin_unlock_irq(&dev_priv->uncore.lock); + if (intel_runtime_pm_get_if_in_use(dev_priv)) { + spin_lock_irq(&dev_priv->uncore.lock); + POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base)); + spin_unlock_irq(&dev_priv->uncore.lock); + intel_runtime_pm_put(dev_priv); + } } intel_fb_obj_flush(obj, write_origin(obj, I915_GEM_DOMAIN_GTT)); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/2] drm/edid: Complete CEA modedb(VIC 1-107)
CEA-861-F specs defines new video modes to be used with HDMI 2.0 EDIDs. The VIC range has been extended from 1-64 to 1-107. Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now to be able to parse new CEA modes using the existing methods, we have to complete the modedb (VIC=65 onwards). This patch adds: - Timings for existing CEA video modes (from VIC=65 till VIC=92) - Newly added 4k modes (from VIC=93 to VIC=107). Cc: Ville Syrjala Cc: Jose Abreu Cc: Andrzej Hajda Cc: Alex Deucher V2: Addressed review comments from Jose: - fix the timings for VIC 83, 90 and 91 - fix formatting for VIC 93-107 V3: Rebase on drm-tip, added R-B from Jose, Alex. V4: Addressed review comments from Andrzej not to modify the VIC filed for HDMI 1.4b sinks (by adding another patch). Reviewed-by: Jose Abreu Reviewed-by: Alex Deucher Signed-off-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 215 + 1 file changed, 215 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e66397a..3b494c2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1001,6 +1001,221 @@ static const struct drm_display_mode edid_cea_modes[] = { 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, + /* 65 - 1280x720@24Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 59400, 1280, 3040, + 3080, 3300, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 66 - 1280x720@25Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3700, + 3740, 3960, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 67 - 1280x720@30Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3040, + 3080, 3300, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 68 - 1280x720@50Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1720, + 1760, 1980, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 69 - 1280x720@60Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390, + 1430, 1650, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 70 - 1280x720@100Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1720, + 1760, 1980, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 71 - 1280x720@120Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1390, + 1430, 1650, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 72 - 1920x1080@24Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2558, + 2602, 2750, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 73 - 1920x1080@25Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2448, + 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 74 - 1920x1080@30Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008, + 2052, 2200, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 75 - 1920x1080@50Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2448, + 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 76 - 1920x1080@60Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2008, + 2052, 22
[Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). For any other mode, the VIC filed in AVI infoframes should be 0. HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is extended to (VIC 1-107). This patch adds a bool input variable, which indicates if the connected sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a HDMI 2.0 VIC to a HDMI 1.4 sink. This patch touches all drm drivers, who are callers of this function drm_hdmi_avi_infoframe_from_display_mode but to make sure there is no change in current behavior, is_hdmi2 is kept as false. In case of I915 driver, this patch checks the connector->display_info to check if the connected display is HDMI 2.0. Cc: Ville Syrjala Cc: Jose Abreu Cc: Andrzej Hajda Cc: Alex Deucher Cc: Daniel Vetter PS: This patch touches a few lines in few files, which were already above 80 char, so checkpatch gives 80 char warning again. - gpu/drm/omapdrm/omap_encoder.c - gpu/drm/i915/intel_sdvo.c Signed-off-by: Shashank Sharma --- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/drm_edid.c| 12 +++- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- drivers/gpu/drm/i915/intel_hdmi.c | 5 - drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- drivers/gpu/drm/radeon/radeon_audio.c | 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c| 2 +- drivers/gpu/drm/tegra/hdmi.c | 2 +- drivers/gpu/drm/tegra/sor.c | 2 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- drivers/gpu/drm/zte/zx_hdmi.c | 2 +- include/drm/drm_edid.h| 3 ++- 21 files changed, 38 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index d4452d8..ab7a399 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder, dce_v10_0_audio_write_sad_regs(encoder); dce_v10_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 5b24e89..3c5fd83 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder, dce_v11_0_audio_write_sad_regs(encoder); dce_v11_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index d2590d7..c564dca 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder, dce_v8_0_audio_write_sad_regs(encoder); dce_v8_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c index a2a8236..f9b77b8 100644 --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge, mutex_lock(&anx78xx->lock); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode, + false); if (err) { DRM_ERROR("Failed to setup AVI infoframe: %d\n", err); goto unlock; diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index
Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma wrote: > HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). > For any other mode, the VIC filed in AVI infoframes should be 0. > HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is > extended to (VIC 1-107). > > This patch adds a bool input variable, which indicates if the connected > sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a > HDMI 2.0 VIC to a HDMI 1.4 sink. Should this patch come before the patch which recognizes VICs 65+? Otherwise if only the first patch is applied but not this one, you could end up with the bad scenario. (As can happen in bisections, for example.) > > This patch touches all drm drivers, who are callers of this function > drm_hdmi_avi_infoframe_from_display_mode but to make sure there is > no change in current behavior, is_hdmi2 is kept as false. > > In case of I915 driver, this patch checks the connector->display_info > to check if the connected display is HDMI 2.0. > > Cc: Ville Syrjala > Cc: Jose Abreu > Cc: Andrzej Hajda > Cc: Alex Deucher > Cc: Daniel Vetter > > PS: This patch touches a few lines in few files, which were > already above 80 char, so checkpatch gives 80 char warning again. > - gpu/drm/omapdrm/omap_encoder.c > - gpu/drm/i915/intel_sdvo.c > > Signed-off-by: Shashank Sharma > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- > drivers/gpu/drm/bridge/sii902x.c | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > drivers/gpu/drm/drm_edid.c| 12 +++- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > drivers/gpu/drm/i915/intel_hdmi.c | 5 - > drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- > drivers/gpu/drm/sti/sti_hdmi.c| 2 +- > drivers/gpu/drm/tegra/hdmi.c | 2 +- > drivers/gpu/drm/tegra/sor.c | 2 +- > drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- > drivers/gpu/drm/zte/zx_hdmi.c | 2 +- > include/drm/drm_edid.h| 3 ++- > 21 files changed, 38 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index d4452d8..ab7a399 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder > *encoder, > dce_v10_0_audio_write_sad_regs(encoder); > dce_v10_0_audio_write_latency_fields(encoder, mode); > > - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index 5b24e89..3c5fd83 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder > *encoder, > dce_v11_0_audio_write_sad_regs(encoder); > dce_v11_0_audio_write_latency_fields(encoder, mode); > > - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index d2590d7..c564dca 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder > *encoder, > dce_v8_0_audio_write_sad_regs(encoder); > dce_v8_0_audio_write_latency_fields(encoder, mode); > > - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c > b/drivers/gpu/drm/bridge/analogix-anx78xx.c > index a2a8236..f9b77b8 100644 > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c > @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge > *bridg
Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
Regards Shashank On 3/23/2017 5:17 PM, Ilia Mirkin wrote: On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma wrote: HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). For any other mode, the VIC filed in AVI infoframes should be 0. HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is extended to (VIC 1-107). This patch adds a bool input variable, which indicates if the connected sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a HDMI 2.0 VIC to a HDMI 1.4 sink. Should this patch come before the patch which recognizes VICs 65+? Otherwise if only the first patch is applied but not this one, you could end up with the bad scenario. (As can happen in bisections, for example.) I kindof agree, this could be the case, but also, for the correct functionality, you should have the whole series together. This patch touches all drm drivers, who are callers of this function drm_hdmi_avi_infoframe_from_display_mode but to make sure there is no change in current behavior, is_hdmi2 is kept as false. In case of I915 driver, this patch checks the connector->display_info to check if the connected display is HDMI 2.0. Cc: Ville Syrjala Cc: Jose Abreu Cc: Andrzej Hajda Cc: Alex Deucher Cc: Daniel Vetter PS: This patch touches a few lines in few files, which were already above 80 char, so checkpatch gives 80 char warning again. - gpu/drm/omapdrm/omap_encoder.c - gpu/drm/i915/intel_sdvo.c Signed-off-by: Shashank Sharma --- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/drm_edid.c| 12 +++- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- drivers/gpu/drm/i915/intel_hdmi.c | 5 - drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- drivers/gpu/drm/radeon/radeon_audio.c | 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c| 2 +- drivers/gpu/drm/tegra/hdmi.c | 2 +- drivers/gpu/drm/tegra/sor.c | 2 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- drivers/gpu/drm/zte/zx_hdmi.c | 2 +- include/drm/drm_edid.h| 3 ++- 21 files changed, 38 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index d4452d8..ab7a399 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder, dce_v10_0_audio_write_sad_regs(encoder); dce_v10_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 5b24e89..3c5fd83 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder, dce_v11_0_audio_write_sad_regs(encoder); dce_v11_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index d2590d7..c564dca 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder, dce_v8_0_audio_write_sad_regs(encoder); dce_v8_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c index a2a8236..f9b77b8 100644 --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_s
Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
On Thu, Mar 23, 2017 at 11:22 AM, Sharma, Shashank wrote: > On 3/23/2017 5:17 PM, Ilia Mirkin wrote: >> >> On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma >> wrote: >>> >>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC >>> 1-64). >>> For any other mode, the VIC filed in AVI infoframes should be 0. >>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is >>> extended to (VIC 1-107). >>> >>> This patch adds a bool input variable, which indicates if the connected >>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a >>> HDMI 2.0 VIC to a HDMI 1.4 sink. >> >> Should this patch come before the patch which recognizes VICs 65+? >> Otherwise if only the first patch is applied but not this one, you >> could end up with the bad scenario. (As can happen in bisections, for >> example.) > > I kindof agree, this could be the case, but also, for the correct > functionality, you should have the whole series > together. OK, so ... I have a bug in my filesystem, and I'm bisecting it and rebooting my box at each step. I happen to hit this commit in my bisect and my monitor doesn't turn on? Seems unpleasant, no? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
Hi Shashank, On 23-03-2017 15:14, Shashank Sharma wrote: > HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). > For any other mode, the VIC filed in AVI infoframes should be 0. > HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is > extended to (VIC 1-107). > > This patch adds a bool input variable, which indicates if the connected > sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a > HDMI 2.0 VIC to a HDMI 1.4 sink. > > This patch touches all drm drivers, who are callers of this function > drm_hdmi_avi_infoframe_from_display_mode but to make sure there is > no change in current behavior, is_hdmi2 is kept as false. > > In case of I915 driver, this patch checks the connector->display_info > to check if the connected display is HDMI 2.0. > > Cc: Ville Syrjala > Cc: Jose Abreu > Cc: Andrzej Hajda > Cc: Alex Deucher > Cc: Daniel Vetter > > PS: This patch touches a few lines in few files, which were > already above 80 char, so checkpatch gives 80 char warning again. > - gpu/drm/omapdrm/omap_encoder.c > - gpu/drm/i915/intel_sdvo.c > > Signed-off-by: Shashank Sharma > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- > drivers/gpu/drm/bridge/sii902x.c | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > drivers/gpu/drm/drm_edid.c| 12 +++- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > drivers/gpu/drm/i915/intel_hdmi.c | 5 - > drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- > drivers/gpu/drm/sti/sti_hdmi.c| 2 +- > drivers/gpu/drm/tegra/hdmi.c | 2 +- > drivers/gpu/drm/tegra/sor.c | 2 +- > drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- > drivers/gpu/drm/zte/zx_hdmi.c | 2 +- > include/drm/drm_edid.h| 3 ++- > 21 files changed, 38 insertions(+), 21 deletions(-) > [snip] > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index af93f7a..5ff2886 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, > struct drm_display_mode *mode) > u8 val; > > /* Initialise info frame from DRM mode */ > - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > > if (hdmi->hdmi_data.enc_out_format == YCBCR444) > frame.colorspace = HDMI_COLORSPACE_YUV444; > dw-hdmi controller has full support for HDMI 2.0 features. It all depends on the platform it is integrated. I think adding a parameter to drm_hdmi_avi_infoframe_from_display_mode is not the best idea because of this case: A bridge can have support for HDMI 2.0 features but the platform may limit this support. I guess it can happen in other drivers too. Best regards, Jose Miguel Abreu ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
On Thu, Mar 23, 2017 at 05:14:19PM +0200, Shashank Sharma wrote: > HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). > For any other mode, the VIC filed in AVI infoframes should be 0. > HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is > extended to (VIC 1-107). > > This patch adds a bool input variable, which indicates if the connected > sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a > HDMI 2.0 VIC to a HDMI 1.4 sink. The spec is unfortunately vague when it comes to the CEA-861-F VIC transmission when there is a corresponding HDMI VIC for the same mode. I'm not sure if it's telling us to set both or just one depending on whether we're transmitting 3D video or not. Or at least I can't parse that information from the spec. Anyone have a better crystal ball in their possession? > > This patch touches all drm drivers, who are callers of this function > drm_hdmi_avi_infoframe_from_display_mode but to make sure there is > no change in current behavior, is_hdmi2 is kept as false. > > In case of I915 driver, this patch checks the connector->display_info > to check if the connected display is HDMI 2.0. > > Cc: Ville Syrjala > Cc: Jose Abreu > Cc: Andrzej Hajda > Cc: Alex Deucher > Cc: Daniel Vetter > > PS: This patch touches a few lines in few files, which were > already above 80 char, so checkpatch gives 80 char warning again. > - gpu/drm/omapdrm/omap_encoder.c > - gpu/drm/i915/intel_sdvo.c > > Signed-off-by: Shashank Sharma > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- > drivers/gpu/drm/bridge/sii902x.c | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > drivers/gpu/drm/drm_edid.c| 12 +++- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > drivers/gpu/drm/i915/intel_hdmi.c | 5 - > drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- > drivers/gpu/drm/sti/sti_hdmi.c| 2 +- > drivers/gpu/drm/tegra/hdmi.c | 2 +- > drivers/gpu/drm/tegra/sor.c | 2 +- > drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- > drivers/gpu/drm/zte/zx_hdmi.c | 2 +- > include/drm/drm_edid.h| 3 ++- > 21 files changed, 38 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index d4452d8..ab7a399 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder > *encoder, > dce_v10_0_audio_write_sad_regs(encoder); > dce_v10_0_audio_write_latency_fields(encoder, mode); > > - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index 5b24e89..3c5fd83 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder > *encoder, > dce_v11_0_audio_write_sad_regs(encoder); > dce_v11_0_audio_write_latency_fields(encoder, mode); > > - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index d2590d7..c564dca 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder > *encoder, > dce_v8_0_audio_write_sad_regs(encoder); > dce_v8_0_audio_write_latency_fields(encoder, mode); > > - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c > b/drivers/gpu/drm/bridge/analogix-anx78xx.c > index a2a8236..f9b77b8 100644 > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c > +++ b/d
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Check we have an wake device before flushing GTT writes (rev2)
== Series Details == Series: drm/i915: Check we have an wake device before flushing GTT writes (rev2) URL : https://patchwork.freedesktop.org/series/20899/ State : success == Summary == Series 20899v2 drm/i915: Check we have an wake device before flushing GTT writes https://patchwork.freedesktop.org/api/1.0/series/20899/revisions/2/mbox/ Test gem_sync: Subgroup basic-many-each: incomplete -> PASS (fi-ilk-650) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 459s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 461s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 589s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 544s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 506s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 499s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 438s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 445s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 519s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 494s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 485s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 480s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 601s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 486s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 512s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 468s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 552s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 427s 0b29bac4adfc0329c9235f8e496eaefb2bcc41e9 drm-tip: 2017y-03m-23d-14h-20m-03s UTC integration manifest 26ecce0 drm/i915: Check we have an wake device before flushing GTT writes == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4280/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
On Thu, Mar 23, 2017 at 03:28:29PM +, Jose Abreu wrote: > Hi Shashank, > > > On 23-03-2017 15:14, Shashank Sharma wrote: > > HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). > > For any other mode, the VIC filed in AVI infoframes should be 0. > > HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is > > extended to (VIC 1-107). > > > > This patch adds a bool input variable, which indicates if the connected > > sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a > > HDMI 2.0 VIC to a HDMI 1.4 sink. > > > > This patch touches all drm drivers, who are callers of this function > > drm_hdmi_avi_infoframe_from_display_mode but to make sure there is > > no change in current behavior, is_hdmi2 is kept as false. > > > > In case of I915 driver, this patch checks the connector->display_info > > to check if the connected display is HDMI 2.0. > > > > Cc: Ville Syrjala > > Cc: Jose Abreu > > Cc: Andrzej Hajda > > Cc: Alex Deucher > > Cc: Daniel Vetter > > > > PS: This patch touches a few lines in few files, which were > > already above 80 char, so checkpatch gives 80 char warning again. > > - gpu/drm/omapdrm/omap_encoder.c > > - gpu/drm/i915/intel_sdvo.c > > > > Signed-off-by: Shashank Sharma > > --- > > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- > > drivers/gpu/drm/bridge/sii902x.c | 2 +- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > > drivers/gpu/drm/drm_edid.c| 12 +++- > > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > > drivers/gpu/drm/i915/intel_hdmi.c | 5 - > > drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- > > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- > > drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- > > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- > > drivers/gpu/drm/sti/sti_hdmi.c| 2 +- > > drivers/gpu/drm/tegra/hdmi.c | 2 +- > > drivers/gpu/drm/tegra/sor.c | 2 +- > > drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- > > drivers/gpu/drm/zte/zx_hdmi.c | 2 +- > > include/drm/drm_edid.h| 3 ++- > > 21 files changed, 38 insertions(+), 21 deletions(-) > > > > [snip] > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > index af93f7a..5ff2886 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, > > struct drm_display_mode *mode) > > u8 val; > > > > /* Initialise info frame from DRM mode */ > > - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > > + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); > > > > if (hdmi->hdmi_data.enc_out_format == YCBCR444) > > frame.colorspace = HDMI_COLORSPACE_YUV444; > > > > dw-hdmi controller has full support for HDMI 2.0 features. It all > depends on the platform it is integrated. > > I think adding a parameter to > drm_hdmi_avi_infoframe_from_display_mode is not the best idea > because of this case: A bridge can have support for HDMI 2.0 > features but the platform may limit this support. I guess it can > happen in other drivers too. Your driver is in full control of what gets passed here. So I don't see why that would be a problem. Also this doesn't really have anything to do with the capabilities of the source. All we want to make sure is that we don't send a VIC the sink will not understand. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-misc-fixes
Hi Dave, drm-misc-fixes-2017-03-23: One fbdev regression fix from Michel Cheers, Daniel The following changes since commit 97da3854c526d3a6ee05c849c96e48d21527606c: Linux 4.11-rc3 (2017-03-19 19:09:39 -0700) are available in the git repository at: git://anongit.freedesktop.org/git/drm-misc tags/drm-misc-fixes-2017-03-23 for you to fetch changes up to 12ffed96d4369f086261ba2ee734fa8c932d7f55: drm/fb-helper: Allow var->x/yres(_virtual) < fb->width/height again (2017-03-23 15:12:07 +0100) One fbdev regression fix from Michel Michel Dänzer (1): drm/fb-helper: Allow var->x/yres(_virtual) < fb->width/height again drivers/gpu/drm/drm_fb_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 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 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
Hi Ville, On 23-03-2017 15:36, Ville Syrjälä wrote: > On Thu, Mar 23, 2017 at 05:14:19PM +0200, Shashank Sharma wrote: >> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64). >> For any other mode, the VIC filed in AVI infoframes should be 0. >> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is >> extended to (VIC 1-107). >> >> This patch adds a bool input variable, which indicates if the connected >> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a >> HDMI 2.0 VIC to a HDMI 1.4 sink. > The spec is unfortunately vague when it comes to the CEA-861-F VIC > transmission when there is a corresponding HDMI VIC for the same mode. > I'm not sure if it's telling us to set both or just one depending on > whether we're transmitting 3D video or not. Or at least I can't parse > that information from the spec. Anyone have a better crystal ball > in their possession? > I've been working in HDMI receivers and this is what I've got in a comment: 1282 /* 1283 * Update current VIC: When transmitting any extended video format 1284 * indicated through use of the HDMI_VIC field in the HDMI Vendor 1285 * Specific InfoFrame or any other format which is not described in 1286 * the above cases, an HDMI Source shall set the AVI InfoFrame VIC 1287 * field to zero. 1288 */ This was directly taken from the spec, can't remember exactly were though. So, the VIC in AVIIF must be set to 0 and the HDMI_VIC field in VSIF shall be set to the HDMI 4k VIC. Best regards, Jose Miguel Abreu ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx