[Intel-gfx] linux-next: manual merge of the drm-misc tree with the drm-intel tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/i915_drv.c

between commit:

  99c539bef538 ("drm/i915: unregister interfaces first in unload")

from the drm-intel tree and commit:

  baf54385af78 ("drm/i915: Drop drm_vblank_cleanup")

from the drm-misc tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/i915_drv.c
index f406aec8a499,04d9bd84ee43..
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@@ -1386,8 -1367,8 +1384,6 @@@ void i915_driver_unload(struct drm_devi
  
intel_gvt_cleanup(dev_priv);
  
-   drm_vblank_cleanup(dev);
 -  i915_driver_unregister(dev_priv);
--
intel_modeset_cleanup(dev);
  
/*
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq

[Pandiyan, Dhinakaran]

On Mon, 2017-06-26 at 15:32 +0300, Paul Kocialkowski wrote:
> After detecting an IRQ storm, hotplug detection will switch from
> irq-based detection to poll-based detection. After a short delay or
> when resetting storm detection from debugfs, detection will switch
> back to being irq-based.
> 
> However, it may occur that polling does not have enough time to detect
> the current connector state when that second switch takes place. 

Some questions so that I understand this better. How short would this
have to be for detect to not complete? Is there a way I can easily test
this scenario?

> Thus,
> this sets the appropriate hotplug event bits for the concerned
> connectors and schedules the hotplug work, that will ensure the
> connectors states are in sync when switching back to irq.
> 

Not sure I understand this correctly, looks like you are setting the
event_bits even if there was no irq during the polling interval. Is that
right?


> Without this, no irq will be triggered and the hpd change will be lost.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index f1200272a699..29f55480b0bb 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -218,9 +218,13 @@ static void intel_hpd_irq_storm_reenable_work(struct 
> work_struct *work)
>   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>  
>   if (intel_connector->encoder->hpd_pin == i) {
> - if (connector->polled != 
> intel_connector->polled)
> + if (connector->polled != 
> intel_connector->polled) {
>   DRM_DEBUG_DRIVER("Reenabling HPD on 
> connector %s\n",
>connector->name);
> +
> + dev_priv->hotplug.event_bits |= (1 << 
> i);
> + }
> +
>   connector->polled = intel_connector->polled;
>   if (!connector->polled)
>   connector->polled = 
> DRM_CONNECTOR_POLL_HPD;
> @@ -232,6 +236,8 @@ static void intel_hpd_irq_storm_reenable_work(struct 
> work_struct *work)
>   dev_priv->display.hpd_irq_setup(dev_priv);
>   spin_unlock_irq(_priv->irq_lock);
>  
> + schedule_work(_priv->hotplug.hotplug_work);

How does this work with DP connectors? From what I understand, the
event_bits are set for DP based on the return value from
intel_dp_hpd_pulse(). But, doesn't this patch set the bits
unconditionally?

> +
>   intel_runtime_pm_put(dev_priv);
>  }
>  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Fix scaler init during CRTC HW 
state readout
URL   : https://patchwork.freedesktop.org/series/27607/
State : success

== Summary ==

Series 27607v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/27607/revisions/1/mbox/

Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
warn   -> FAIL   (fi-snb-2600) fdo#100215
Test kms_flip:
Subgroup basic-flip-vs-modeset:
warn   -> SKIP   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
warn   -> DMESG-FAIL (fi-pnv-d510) fdo#101597 +1

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:11  
time:454s
fi-bdw-gvtdvmtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:14  
time:439s
fi-blb-e6850 total:279  pass:0dwarn:0   dfail:1   fail:0   skip:54  
time:359s
fi-bsw-n3050 total:279  pass:0dwarn:0   dfail:0   fail:0   skip:36  
time:539s
fi-bxt-j4205 total:279  pass:0dwarn:0   dfail:0   fail:0   skip:19  
time:518s
fi-byt-j1900 total:279  pass:0dwarn:0   dfail:1   fail:0   skip:24  
time:490s
fi-byt-n2820 total:279  pass:0dwarn:0   dfail:1   fail:0   skip:28  
time:493s
fi-glk-2atotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:19  
time:599s
fi-hsw-4770  total:279  pass:0dwarn:0   dfail:0   fail:0   skip:16  
time:435s
fi-hsw-4770r total:279  pass:0dwarn:0   dfail:0   fail:0   skip:16  
time:416s
fi-ilk-650   total:279  pass:0dwarn:0   dfail:0   fail:0   skip:50  
time:414s
fi-ivb-3520m total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:504s
fi-ivb-3770  total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:473s
fi-kbl-7500u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:467s
fi-kbl-7560u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:10  
time:575s
fi-kbl-r total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:581s
fi-pnv-d510  total:279  pass:0dwarn:0   dfail:3   fail:0   skip:55  
time:559s
fi-skl-6260u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:10  
time:467s
fi-skl-6700hqtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:17  
time:594s
fi-skl-6700k total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:467s
fi-skl-6770hqtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:10  
time:473s
fi-skl-gvtdvmtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:13  
time:436s
fi-skl-x1585ltotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:11  
time:470s
fi-snb-2520m total:279  pass:0dwarn:0   dfail:0   fail:0   skip:28  
time:545s
fi-snb-2600  total:279  pass:0dwarn:0   dfail:0   fail:1   skip:29  
time:404s

f7e80ae2a77f233ffca8fcf5739b26e2bd9a80a6 drm-tip: 2017y-07m-19d-18h-16m-25s UTC 
integration manifest
e51c5fc drm/i915: Simplify scaler init during CRTC HW readout
c296387 drm/i915: Fix scaler init during CRTC HW state readout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5239/
___
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/selftests: Fix an error handling path in 'mock_gem_device()'

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Fix an error handling path in 'mock_gem_device()'
URL   : https://patchwork.freedesktop.org/series/27604/
State : success

== Summary ==

Series 27604v1 drm/i915/selftests: Fix an error handling path in 
'mock_gem_device()'
https://patchwork.freedesktop.org/api/1.0/series/27604/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
warn   -> FAIL   (fi-snb-2600) fdo#17
Test kms_flip:
Subgroup basic-flip-vs-modeset:
warn   -> SKIP   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
warn   -> DMESG-FAIL (fi-pnv-d510) fdo#101597

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:11  
time:448s
fi-bdw-gvtdvmtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:14  
time:427s
fi-blb-e6850 total:279  pass:0dwarn:0   dfail:1   fail:0   skip:54  
time:355s
fi-bsw-n3050 total:279  pass:0dwarn:0   dfail:0   fail:0   skip:36  
time:536s
fi-bxt-j4205 total:279  pass:0dwarn:0   dfail:0   fail:0   skip:19  
time:512s
fi-byt-j1900 total:279  pass:0dwarn:0   dfail:1   fail:0   skip:24  
time:495s
fi-byt-n2820 total:279  pass:0dwarn:0   dfail:1   fail:0   skip:28  
time:488s
fi-glk-2atotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:19  
time:595s
fi-hsw-4770  total:279  pass:0dwarn:0   dfail:0   fail:0   skip:16  
time:435s
fi-hsw-4770r total:279  pass:0dwarn:0   dfail:0   fail:0   skip:16  
time:418s
fi-ilk-650   total:279  pass:0dwarn:0   dfail:0   fail:0   skip:50  
time:411s
fi-ivb-3520m total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:502s
fi-ivb-3770  total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:467s
fi-kbl-7500u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-kbl-7560u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:10  
time:585s
fi-kbl-r total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:584s
fi-pnv-d510  total:279  pass:0dwarn:0   dfail:2   fail:0   skip:55  
time:561s
fi-skl-6260u total:279  pass:0dwarn:0   dfail:0   fail:0   skip:10  
time:471s
fi-skl-6700hqtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:17  
time:586s
fi-skl-6700k total:279  pass:0dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-skl-gvtdvmtotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:13  
time:437s
fi-skl-x1585ltotal:279  pass:0dwarn:0   dfail:0   fail:0   skip:11  
time:493s
fi-snb-2520m total:279  pass:0dwarn:0   dfail:0   fail:0   skip:28  
time:555s
fi-snb-2600  total:279  pass:0dwarn:0   dfail:0   fail:1   skip:29  
time:411s
fi-skl-6770hq failed to collect. IGT log at Patchwork_5238/fi-skl-6770hq/igt.log

f7e80ae2a77f233ffca8fcf5739b26e2bd9a80a6 drm-tip: 2017y-07m-19d-18h-16m-25s UTC 
integration manifest
2d76463 drm/i915/selftests: Fix an error handling path in 'mock_gem_device()'

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5238/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout

[Imre Deak]

The scaler allocation code depends on a non-zero default value for the
crtc scaler_id, so make sure we initialize the scaler state accordingly
even if the crtc is off. This fixes at least an initial YUV420 modeset
(added in a follow-up patchset by Shashank) when booting with the screen
off: after the initial HW readout and modeset which enables the scaler a
subsequent modeset will disable the scaler which isn't properly
allocated. This results in a funky HW state where the pipe scaler HW
registers can't be modified and the normally black screen is grey and
shifted to the right or jitters.

The problem was revealed by Shashank's YUV420 patchset and first
reported by Ville.

Cc: Shashank Sharma 
Cc: Ville Syrjälä 
Cc: Chandra Konduru 
Cc: Matt Roper 
Cc:  # 4.11.x
Reported-by: Ville Syrjälä 
Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
Signed-off-by: Imre Deak 

---

[ Older stable versions need backporting, so that's for a follow-up ]
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7774f3465fbc..8a38e64b1931 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
u64 power_domain_mask;
bool active;
 
+   if (INTEL_GEN(dev_priv) >= 9) {
+   intel_crtc_init_scalers(crtc, pipe_config);
+
+   pipe_config->scaler_state.scaler_id = -1;
+   pipe_config->scaler_state.scaler_users &= ~(1 << 
SKL_CRTC_INDEX);
+   }
+
power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
return false;
@@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
pipe_config->gamma_mode =
I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
-   if (INTEL_GEN(dev_priv) >= 9) {
-   intel_crtc_init_scalers(crtc, pipe_config);
-
-   pipe_config->scaler_state.scaler_id = -1;
-   pipe_config->scaler_state.scaler_users &= ~(1 << 
SKL_CRTC_INDEX);
-   }
-
power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
power_domain_mask |= BIT_ULL(power_domain);
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout

[Imre Deak]

The crtc state starts out being bzero'd, so no need to clear
scaler_users. Also intel_crtc_init_scalers() knows already which
platforms have scalers, so no need for the platform check here.
Similarly intel_crtc_init_scalers() will init scaler_id as required,
so no need to do it here separately.

Cc: Chandra Konduru 
Cc: Matt Roper 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_display.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8a38e64b1931..7210f418e9c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc 
*crtc,
u64 power_domain_mask;
bool active;
 
-   if (INTEL_GEN(dev_priv) >= 9) {
-   intel_crtc_init_scalers(crtc, pipe_config);
-
-   pipe_config->scaler_state.scaler_id = -1;
-   pipe_config->scaler_state.scaler_users &= ~(1 << 
SKL_CRTC_INDEX);
-   }
+   intel_crtc_init_scalers(crtc, pipe_config);
 
power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/selftests: Fix an error handling path in 'mock_gem_device()'

[Christophe JAILLET]

Goto the right label in case of error, otherwise there is a leak.
This has been introduced by c5cf9a9147ff. In this patch a goto has not been
updated.

Fixes: c5cf9a9147ff ("drm/i915: Create a kmem_cache to allocate struct 
i915_priolist from")
Signed-off-by: Christophe JAILLET 
---
 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c 
b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 627e2aa09766..8cdec455cf7d 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -206,7 +206,7 @@ struct drm_i915_private *mock_gem_device(void)
mkwrite_device_info(i915)->ring_mask = BIT(0);
i915->engine[RCS] = mock_engine(i915, "mock");
if (!i915->engine[RCS])
-   goto err_dependencies;
+   goto err_priorities;
 
i915->kernel_context = mock_context(i915, NULL);
if (!i915->kernel_context)
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Avoid the gpu reset vs. modeset deadlock

[Daniel Vetter]

On Wed, Jul 19, 2017 at 3:44 PM, Daniel Vetter  wrote:
> On Wed, Jul 19, 2017 at 3:32 PM, Chris Wilson  
> wrote:
>> Quoting Daniel Vetter (2017-07-19 13:54:56)
>>> ... using the biggest hammer we have. This is essentially a weaponized
>>> version of the timeout-based wedging Chris added in
>>>
>>> commit 36703e79a982c8ce5a8e43833291f2719e92d0d1
>>> Author: Chris Wilson 
>>> Date:   Thu Jun 22 11:56:25 2017 +0100
>>>
>>> drm/i915: Break modeset deadlocks on reset
>>>
>>> Because defense-in-depth is good it's good to still have both. Also
>>> note that with the locking change we can now restrict this a lot (old
>>> gpus and special testing only), so this doesn't kill the TDR benefits
>>> on at least anything remotely modern.
>>>
>>> And futuremore with a few tricks it should be possible to make a much
>>> more educated guess about whether an atomic commit is stuck waiting on
>>> the gpu (atomic_t counting the pending i915_sw_fence used by the
>>> atomic modeset code should do it), so we can improve this.
>>>
>>> But for now just start with something that is guaranteed to recover
>>> faster, for much better CI througput.
>>>
>>> This defacto reverts TDR on these platforms, but there's not really a
>>> single commit to specify as the sole offender.
>>>
>>> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting 
>>> for request completion")
>>> Fixes: 221fe7994554 ("drm/i915: Perform a direct reset of the GPU from the 
>>> waiter")
>>> Cc: Chris Wilson 
>>> Cc: Mika Kuoppala 
>>> Cc: Joonas Lahtinen 
>>> Signed-off-by: Daniel Vetter 
>>> ---
>>>  drivers/gpu/drm/i915/intel_display.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 9ffa1566..010a1f3e000c 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -3471,6 +3471,11 @@ void intel_prepare_reset(struct drm_i915_private 
>>> *dev_priv)
>>> !gpu_reset_clobbers_display(dev_priv))
>>> return;
>>>
>>> +   /* We have a modeset vs reset deadlock, defensively unbreak it.
>>> +*
>>> +* FIXME: We can do a _lot_ better, this is just a first 
>>> iteration.*/
>>
>> You should keep the error message for wedging the device. If all goes
>> well it is removed in a few patches, so shouldn't be an eyesore for
>> long.
>
> Yeah makes sense, just in case the next patches need to be reverted
> for some reasons. That's why I split it ou. Something like
>
> DRM_ERROR("Wedging gpu to unblock modesets\n");
>
> and then remove that again 2 patches later?

After thinking about this a bit more, s/DRM_ERROR/DRM_DEBUG_KMS/ or
so. We know we can deadlock here, complaining about that only spams
dmesg and results in noise in CI, hiding worse stuff. The timeout
based wedging as fallback should have a DRM_ERROR since it's
unexpected, this one here imo sholdn't. And with the follow-up patches
it won't be unconditional anymore (if we don't have to revert them
again).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Use AUX for backlight only if eDP 1.4 or later

[Pandiyan, Dhinakaran]

On Wed, 2017-07-19 at 15:59 +0530, Jenny TC wrote:
> With older panels, AUX pin for backlight doesn't work. On some
> panels, this causes backlight issues on S3 resume.

What is it that we are missing in the resume path?

>  Enable the
> feature only for eDP1.4 or later.

I can't find the eDP 1.4 requirement in the spec. Regional brightness
control was added in eDP 1.4, but we don't enable that. My concern is we
might be missing a real fix by ignoring < eDP 1.4 panels. 


> 
> Fix-suggested-by: Puthikorn Voravootivat 

1. Please use the "Fixes" tag to refer to the commit that introduced the
code you are fixing.
2. The "Suggested-by" tag is more common to give credits to the person
who suggested the fix.
3. Please use the "Bugzilla" tag to point to the bug that David
reported.


> Signed-off-by: Jenny TC 
> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c 
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index b25cd88..421f31f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -292,7 +292,7 @@ static int intel_dp_aux_setup_backlight(struct 
> intel_connector *connector,
>   * via PWM pin or using AUX is better than using PWM pin.
>   *
>   * The heuristic to determine that using AUX pin is better than using PWM 
> pin is
> - * that the panel support any of the feature list here.
> + * that the panel is eDP 1.4 or later and support any of the feature list 
> here
>   * - Regional backlight brightness adjustment
>   * - Backlight PWM frequency set
>   * - More than 8 bits resolution of brightness level
> @@ -310,6 +310,10 @@ static int intel_dp_aux_setup_backlight(struct 
> intel_connector *connector,
>   if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
>   return true;
>  
> + /* Enable this for eDP 1.4 panel or later. */
> + if (intel_dp->edp_dpcd[0] < DP_EDP_14)
> + return false;
> +
>   /* Panel supports regional backlight brightness adjustment */
>   if (drm_dp_dpcd_readb(_dp->aux, DP_EDP_GENERAL_CAP_3,
> _val) != 1) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/selftests: Mark contexts as lost during freeing of mock device

[Chris Wilson]

Quoting Matthew Auld (2017-07-19 18:40:05)
> On 19 July 2017 at 14:59, Chris Wilson  wrote:
> > We need to unpin the last retired context early in the shutdown sequence
> > so that its RCU free is done before we try to free the context ida. I
> > included this in a later patch ("drm/i915: Keep a recent cache of freed
> > contexts objects for reuse") and so missed that the selftests were broken
> > in the meantime.
> >
> > Reported-by: Matthew Auld 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101627
> > Fixes: 5f09a9c8ab6b ("drm/i915: Allow contexts to be unreferenced 
> > locklessly")
> > Signed-off-by: Chris Wilson 
> > Cc: Joonas Lahtinen 
> > Cc: Tvrtko Ursulin 
> > Cc: Mika Kuoppala 
> > Cc: Matthew Auld 
> 
> Makes sense.
> 
> Tested-by: Matthew Auld 
> Reviewed-by: Matthew Auld 

Obvious in hindsight, pushed. Thanks,
-Chris
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Pass enum pipe to intel_set_pch_fifo_underrun_reporting()

[Patchwork]

== Series Details ==

Series: drm/i915: Pass enum pipe to intel_set_pch_fifo_underrun_reporting()
URL   : https://patchwork.freedesktop.org/series/27591/
State : success

== Summary ==

Series 27591v1 drm/i915: Pass enum pipe to 
intel_set_pch_fifo_underrun_reporting()
https://patchwork.freedesktop.org/api/1.0/series/27591/revisions/1/mbox/

Test gem_sync:
Subgroup basic-store-all:
fail   -> PASS   (fi-ivb-3520m) fdo#17
Test kms_busy:
Subgroup basic-flip-default-a:
pass   -> DMESG-WARN (fi-skl-6700hq) fdo#101144
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-byt-n2820) fdo#101705

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:447s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:433s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:355s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:533s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:509s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:497s
fi-byt-n2820 total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:486s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:597s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:437s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:417s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:422s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:505s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:467s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:661s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:586s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:460s
fi-skl-6700hqtotal:279  pass:261  dwarn:1   dfail:0   fail:0   skip:17  
time:595s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:468s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:482s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:435s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:472s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:541s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:404s
fi-pnv-d510 failed to collect. IGT log at Patchwork_5237/fi-pnv-d510/igt.log

f1c32d67a1e143cba1b0ad042448f39d75d9ccaa drm-tip: 2017y-07m-19d-13h-38m-55s UTC 
integration manifest
406ffba drm/i915: Pass enum pipe to intel_set_pch_fifo_underrun_reporting()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5237/
___
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 0/2] Analogue/VGA frame comparison support

[Lyude Paul]

For both patches once you squash the configure.ac changes into the
first one:

Reviewed-by: Lyude 

Once you post the new versions I'll just go ahead and push them

On Wed, 2017-07-19 at 16:48 +0300, Paul Kocialkowski wrote:
> Changes since v2:
> * Changed analogue in favor of analog
> * Integrated analog frame match fixup in the original commit
> * Rebased on top of the new revisions of the series this depends on
> 
> Changes since v1:
> * Split analogue frame comparison to igt_frame, using cairo surfaces
> * Added a chamelium helper for analogue frame checking and frame
> dumping
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 3/3] configure.ac: Make gsl dependency and dependent code optional

[Lyude Paul]

R-B for the first two, I've already pushed them. For this one I'd
prefer it if you just squashed it into the series where you add analog
frame comparison support. Otherwise, looks good.

On Wed, 2017-07-19 at 17:59 +0300, Paul Kocialkowski wrote:
> This adds automake instructions, with matching autoconf macros, to
> make
> the dependency on gsl and the code that depends on it optional.
> 
> This should allow preserving the ability to build IGT on Android,
> where
> gsl support may be lacking.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  configure.ac | 6 +-
>  lib/Makefile.am  | 7 +++
>  lib/Makefile.sources | 2 --
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 20e5cf96..de0e85dd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -181,7 +181,8 @@ PKG_CHECK_MODULES(GLIB, [glib-2.0], [glib=yes],
> [glib=no])
>  if test x"$glib" = xyes; then
>   AC_DEFINE(HAVE_GLIB,1,[Enable glib support])
>  fi
> -PKG_CHECK_MODULES(GSL, gsl)
> +PKG_CHECK_MODULES(GSL, [gsl], [gsl=yes], [gsl=no])
> +AM_CONDITIONAL(HAVE_GSL, [test "x$gsl" = xyes])
>  
>  # for chamelium
>  AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium],
> @@ -196,6 +197,9 @@ if test "x$enable_chamelium" = xyes; then
>   if test x"$glib" != xyes; then
>   AC_MSG_ERROR([Failed to find glib, required by
> chamelium. Use --disable-chamelium to disable chamelium support.])
>   fi
> + if test x"$gsl" != xyes; then
> + AC_MSG_ERROR([Failed to find gsl, required by
> chamelium. Use --disable-chamelium to disable chamelium support.])
> + fi
>   AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support])
>  fi
>  
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index fb922ced..9b506f69 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -29,6 +29,13 @@ lib_source_list += \
>   $(NULL)
>  endif
>  
> +if HAVE_GSL
> +lib_source_list +=   \
> + igt_frame.c \
> + igt_frame.h \
> + $(NULL)
> +endif
> +
>  AM_CPPFLAGS = -I$(top_srcdir)
>  AM_CFLAGS = \
>   $(CWARNFLAGS) \
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index c2e58809..53fdb54c 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -83,8 +83,6 @@ lib_source_list =   \
>   uwildmat/uwildmat.c \
>   igt_kmod.c  \
>   igt_kmod.h  \
> - igt_frame.c \
> - igt_frame.h \
>   $(NULL)
>  
>  .PHONY: version.h.tmp
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/selftests: Mark contexts as lost during freeing of mock device

[Matthew Auld]

On 19 July 2017 at 14:59, Chris Wilson  wrote:
> We need to unpin the last retired context early in the shutdown sequence
> so that its RCU free is done before we try to free the context ida. I
> included this in a later patch ("drm/i915: Keep a recent cache of freed
> contexts objects for reuse") and so missed that the selftests were broken
> in the meantime.
>
> Reported-by: Matthew Auld 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101627
> Fixes: 5f09a9c8ab6b ("drm/i915: Allow contexts to be unreferenced locklessly")
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Tvrtko Ursulin 
> Cc: Mika Kuoppala 
> Cc: Matthew Auld 

Makes sense.

Tested-by: Matthew Auld 
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Pass enum pipe to intel_set_pch_fifo_underrun_reporting()

[Matthias Kaehlcke]

Commit a21960339c8c ("drm/i915: Consistently use enum pipe for PCH
transcoders") misses some pieces, due to a problem with the patch
format, this patch adds the remaining bits.

Fixes: a21960339c8c ("drm/i915: Consistently use enum pipe for PCH
transcoders")

Signed-off-by: Matthias Kaehlcke 
---
 drivers/gpu/drm/i915/intel_display.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a89d0fd1c2e1..5c7054c3be0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state 
*pipe_config,
return;
 
if (intel_crtc->config->has_pch_encoder)
-   intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
- false);
+   intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
@@ -5433,8 +5432,7 @@ static void haswell_crtc_enable(struct intel_crtc_state 
*pipe_config,
intel_wait_for_vblank(dev_priv, pipe);
intel_wait_for_vblank(dev_priv, pipe);
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
-   intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
- true);
+   intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
}
 
/* If we change the relative order between pipe/planes enabling, we need
@@ -5531,8 +5529,7 @@ static void haswell_crtc_disable(struct intel_crtc_state 
*old_crtc_state,
enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 
if (intel_crtc->config->has_pch_encoder)
-   intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
- false);
+   intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
intel_encoders_disable(crtc, old_crtc_state, old_state);
 
@@ -5560,8 +5557,7 @@ static void haswell_crtc_disable(struct intel_crtc_state 
*old_crtc_state,
intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
if (old_crtc_state->has_pch_encoder)
-   intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
- true);
+   intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
-- 
2.14.0.rc0.284.gd933b75aa4-goog

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders

[Matthias Kaehlcke]

El Wed, Jul 19, 2017 at 08:30:36AM +0200 Daniel Vetter ha dit:

> On Tue, Jul 18, 2017 at 01:48:53PM -0700, Matthias Kaehlcke wrote:
> > Hi Daniel,
> > 
> > El Tue, Jul 18, 2017 at 08:39:50AM +0200 Daniel Vetter ha dit:
> > 
> > > On Mon, Jul 17, 2017 at 11:14:03AM -0700, Matthias Kaehlcke wrote:
> > > > The current code uses in some instances enum transcoder for PCH
> > > > transcoders and enum pipe in others. This is error prone and clang
> > > > raises warnings like this:
> > > > 
> > > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion
> > > >   from enumeration type 'enum pipe' to different enumeration type
> > > >   'enum transcoder' [-Wenum-conversion]
> > > > intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > > > 
> > > > Consistently use the type enum pipe for PCH transcoders.
> > > > 
> > > > Signed-off-by: Matthias Kaehlcke 
> > > 
> > > Somehow git apply-mbox could parse it, but manually applying using patch
> > > worked. Not sure what's going on, maybe double-check it's all right.
> > 
> > Not sure what happened, one of the patch fragments only has one '@'
> > instead of two, with that fixed the patch applies.
> > 
> > Unfortunately the manual application missed some fragments:
> > 
> > drivers/gpu/drm/i915/intel_display.c:5350:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> > intel_set_pch_fifo_underrun_reporting(dev_priv, 
> > TRANSCODER_A,
> > ~   ^~~~
> > drivers/gpu/drm/i915/intel_display.c:5436:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> > intel_set_pch_fifo_underrun_reporting(dev_priv, 
> > TRANSCODER_A,
> > ~   ^~~~
> > drivers/gpu/drm/i915/intel_display.c:5534:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> > intel_set_pch_fifo_underrun_reporting(dev_priv, 
> > TRANSCODER_A,
> > ~   ^~~~
> > drivers/gpu/drm/i915/intel_display.c:5563:51: warning: implicit
> >   conversion from enumeration type 'enum transcoder' to different
> >   enumeration type 'enum pipe' [-Wenum-conversion]
> > intel_set_pch_fifo_underrun_reporting(dev_priv, 
> > TRANSCODER_A,
> > 
> > 
> > What would be the best way forward from here? Revert the manual
> > application and apply again, or a fixup patch?
> 
> Drat I screwed up :-( drm-intel-next-queued is non-rebasing, that means I
> need a fixup patch. I should have checked more carefully that I have all
> the hunks, but patch -p1 seemed happy ...

Ok, I will send a fixup patch shortly

Matthias
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v5 0/7] CRC testing with Chamelium improvements

[Lyude Paul]

Thank you for all of the great work you're doing! This looks perfect,
so for the whole series

Reviewed-by: Lyude 

I've pushed everything upstream, congrats!

On Wed, 2017-07-19 at 16:46 +0300, Paul Kocialkowski wrote:
> Changes since v4:
> * Moved igt_get_cairo_surface out of the thread function to properly
>   handle assert failure
> * Rebased on top of current master
> 
> Changes since v3:
> * Renamed structure used by async crc calculation for more clarity
> * Used const qualifier for untouched buffer when hashing
> * Split actual calculation to a dedicated function
> * Reworked async functions names for more clarity
> * Reworked descriptions for better accuracy
> * Exported framebuffer cairo surface and use it directly instead of
>   (unused) png dumping
> * Fix how the framebuffer cairo surface is obtained to avoid
> destroying
>   it too early
> 
> * Made CRC checking logic common
> * Added a check for the same number of words
> * Made frame dumping configuration and helpers common
> * Added an extended crc to string helper
> * Added a chamelium helper for crc checking and frame dumping
> * Split the merging of crc functions to a separate patch
> * Added support for frame dump path global variable
> * Added listing the dumped images in a file, possibly identified with
>   an id global variable
> 
> The latter allows having one "dump report" file per run, possibly
> identified with the id global variable, that indicates which files
> are the output, while keeping the possibility to have the same files
> for different runs. This allows saving significant disk space when
> the images are identified with e.g. their crc, so that duplicate
> results
> only lead to duplicate dump reports and not duplicate images.
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/2] tests/chamelium: Skip suspend/resume test with unreliable hotplug event

[Lyude Paul]

On Wed, 2017-07-19 at 11:31 +0300, Paul Kocialkowski wrote:
> On Tue, 2017-07-18 at 22:21 +0100, Chris Wilson wrote:
> > Quoting Paul Kocialkowski (2017-07-18 16:16:26)
> > > It may occur that a hotplug uevent is detected at resume, even
> > > though it
> > > does not indicate that an actual hotplug happened. This is the
> > > case
> > > when
> > > link training fails on any other connector.
> > > 
> > > There is currently no way to distinguish what connector caused a
> > > hotplug
> > > uevent, nor what the reason for that uevent really is. This makes
> > > it
> > > impossible to find out whether the test actually passed or not.
> > 
> > And you may get more than one and then this skips even though the
> > test
> > passed. Looks like the patch is overcompensating. What you can do
> > is
> > repeat the test a few times, and then look at all the different
> > errors
> > you get. If the connector remains (no mst disappareance) once it
> > goes
> > bad, it should remain bad and so not generate any new uevent. Or
> > you
> > only repeat the test whilst link_status[old] != link_status[new].
> 
> I am not sure it is really desirable to repeat the test until we are
> fairly certain it succeeds. This involves suspend/resume, that is
> already long enough as it is.
> 
> Also, a uevent will be generated everytime link training fails,
> regardless of whether it was already failing before (I just tested
> that
> to make sure). In my case, it's due to a DP-VGA bridge that will
> consistently fail link training in the first seconds after resume.
> 
> So this is actually even worse that I thought, because there is no
> way
> to find out that this is why a uevent was generated if the link
> status
> was already bad before.
> 
> So I don't see how we can manage with the current information at
> disposal.
> 
> My main point here is that we need more information about what's
> going
> on than simply "HOTPLUG=1". These patches demonstrate that working
> around the lack of information is a pain for testing purposes and can
> only leads to semi-working hackish workarounds.
> 
> Do you agree that this is what the problem really is?
Yes, I agree we need more debugging information for when hotplugs fail.
This being said though, the fact that i915 is unconditionally sending
hotplugs on resume (this appears to be a hack that they did add to stop
from missign hotplug events between suspend/resume) is really what's
causing this problem specifically.

We really need the debugging stuff me and martin suggested for the
kernel, and also more drm helpers to actually do edid checks and that
sort of stuff so that we don't have to deal with dirty hacks like this
:\.
> 
___
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/atomic: Remove deprecated atomic iterator macros, v2.

[Patchwork]

== Series Details ==

Series: drm/atomic: Remove deprecated atomic iterator macros, v2.
URL   : https://patchwork.freedesktop.org/series/27582/
State : success

== Summary ==

Series 27582v1 drm/atomic: Remove deprecated atomic iterator macros, v2.
https://patchwork.freedesktop.org/api/1.0/series/27582/revisions/1/mbox/

Test gem_sync:
Subgroup basic-store-all:
fail   -> PASS   (fi-ivb-3520m) fdo#17
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l) fdo#101781

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:442s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:431s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:352s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:540s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:517s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:487s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:491s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:600s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:423s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:417s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:420s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:503s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:493s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:466s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:574s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:587s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:567s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:459s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:594s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:474s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:476s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:435s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:494s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:535s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:405s

f1c32d67a1e143cba1b0ad042448f39d75d9ccaa drm-tip: 2017y-07m-19d-13h-38m-55s UTC 
integration manifest
db13c8a drm/atomic: Remove deprecated accessor macros
026 drm/nouveau: Convert nouveau to use new iterator macros, v2.
82bf6c3 drm/msm: Convert to use new iterator macros, v2.
d377fe4 drm/omapdrm: Fix omap_atomic_wait_for_completion
2ddcf93 drm/rcar-du: Use new iterator macros, v2.
f83fb92 drm/atomic: Clean up drm_atomic_helper_async_check
2c07ba5 drm/atomic: Use new iterator macros in 
drm_atomic_helper_wait_for_flip_done, again.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5236/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 3/3] configure.ac: Make gsl dependency and dependent code optional

[Paul Kocialkowski]

This adds automake instructions, with matching autoconf macros, to make
the dependency on gsl and the code that depends on it optional.

This should allow preserving the ability to build IGT on Android, where
gsl support may be lacking.

Signed-off-by: Paul Kocialkowski 
---
 configure.ac | 6 +-
 lib/Makefile.am  | 7 +++
 lib/Makefile.sources | 2 --
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 20e5cf96..de0e85dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -181,7 +181,8 @@ PKG_CHECK_MODULES(GLIB, [glib-2.0], [glib=yes], [glib=no])
 if test x"$glib" = xyes; then
AC_DEFINE(HAVE_GLIB,1,[Enable glib support])
 fi
-PKG_CHECK_MODULES(GSL, gsl)
+PKG_CHECK_MODULES(GSL, [gsl], [gsl=yes], [gsl=no])
+AM_CONDITIONAL(HAVE_GSL, [test "x$gsl" = xyes])
 
 # for chamelium
 AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium],
@@ -196,6 +197,9 @@ if test "x$enable_chamelium" = xyes; then
if test x"$glib" != xyes; then
AC_MSG_ERROR([Failed to find glib, required by chamelium. Use 
--disable-chamelium to disable chamelium support.])
fi
+   if test x"$gsl" != xyes; then
+   AC_MSG_ERROR([Failed to find gsl, required by chamelium. Use 
--disable-chamelium to disable chamelium support.])
+   fi
AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support])
 fi
 
diff --git a/lib/Makefile.am b/lib/Makefile.am
index fb922ced..9b506f69 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -29,6 +29,13 @@ lib_source_list +=   \
$(NULL)
 endif
 
+if HAVE_GSL
+lib_source_list += \
+   igt_frame.c \
+   igt_frame.h \
+   $(NULL)
+endif
+
 AM_CPPFLAGS = -I$(top_srcdir)
 AM_CFLAGS = \
$(CWARNFLAGS) \
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index c2e58809..53fdb54c 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -83,8 +83,6 @@ lib_source_list = \
uwildmat/uwildmat.c \
igt_kmod.c  \
igt_kmod.h  \
-   igt_frame.c \
-   igt_frame.h \
$(NULL)
 
 .PHONY: version.h.tmp
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 2/3] configure.ac: Make glib dependency optional to preserve Android build

[Paul Kocialkowski]

This adds ifdef wrappers, with matching autoconf macros, to make the
dependency on glib (used for parsing configuration) optional.

This allows preserving the ability to build IGT on Android, where glib
support is lacking.

Signed-off-by: Paul Kocialkowski 
---
 configure.ac   | 8 +++-
 lib/igt_core.c | 9 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 690f73ef..20e5cf96 100644
--- a/configure.ac
+++ b/configure.ac
@@ -177,7 +177,10 @@ AM_CONDITIONAL(HAVE_UDEV, [test "x$udev" = xyes])
 if test x"$udev" = xyes; then
AC_DEFINE(HAVE_UDEV,1,[Enable udev-based monitor hotplug detection])
 fi
-PKG_CHECK_MODULES(GLIB, glib-2.0)
+PKG_CHECK_MODULES(GLIB, [glib-2.0], [glib=yes], [glib=no])
+if test x"$glib" = xyes; then
+   AC_DEFINE(HAVE_GLIB,1,[Enable glib support])
+fi
 PKG_CHECK_MODULES(GSL, gsl)
 
 # for chamelium
@@ -190,6 +193,9 @@ if test "x$enable_chamelium" = xyes; then
  [AC_MSG_ERROR([Failed to find xmlrpc, required by 
chamelium. Use --disable-chamelium to disable chamelium support.])])
PKG_CHECK_MODULES(PIXMAN, pixman-1, [],
  [AC_MSG_ERROR([Failed to find pixman, required by 
chamelium. Use --disable-chamelium to disable chamelium support.])])
+   if test x"$glib" != xyes; then
+   AC_MSG_ERROR([Failed to find glib, required by chamelium. Use 
--disable-chamelium to disable chamelium support.])
+   fi
AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support])
 fi
 
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 5a3b00e8..028ef6bd 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -293,7 +293,10 @@ static struct {
 } log_buffer;
 static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+#ifdef HAVE_GLIB
 GKeyFile *igt_key_file;
+#endif
+
 char *frame_dump_path;
 
 const char *igt_test_name(void)
@@ -618,6 +621,7 @@ static void oom_adjust_for_doom(void)
 
 }
 
+#ifdef HAVE_GLIB
 static int config_parse(void)
 {
GError *error = NULL;
@@ -643,6 +647,7 @@ static int config_parse(void)
 
return 0;
 }
+#endif
 
 static int common_init(int *argc, char **argv,
   const char *extra_short_opts,
@@ -799,6 +804,7 @@ static int common_init(int *argc, char **argv,
snprintf(key_file_loc, 100, "%s/.igtrc", g_get_home_dir());
}
 
+#ifdef HAVE_GLIB
igt_key_file = g_key_file_new();
ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
G_KEY_FILE_NONE, );
@@ -811,6 +817,7 @@ static int common_init(int *argc, char **argv,
}
 
ret = config_parse();
+#endif
 
 out:
if (!key_file_env && key_file_loc)
@@ -1423,8 +1430,10 @@ void igt_exit(void)
 {
igt_exit_called = true;
 
+#ifdef HAVE_GLIB
if (igt_key_file)
g_key_file_free(igt_key_file);
+#endif
 
if (run_single_subtest && !run_single_subtest_found) {
igt_warn("Unknown subtest: %s\n", run_single_subtest);
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 0/3] Various autoconf fixups

[Paul Kocialkowski]

These patches apply on top of:
tests/chamelium: Detect analogue bridges and handle EDID accordingly

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 1/3] configure.ac: Enable back chamelium build by default

[Paul Kocialkowski]

Introducing an option for chamelium build inadvertently disabled it by
default, according to the definition of the AC_ARG_ENABLE macro.

This enables it back chamelium by default.

Fixes: fd096fcc ("configure.ac: Make building chamelium an option")

Signed-off-by: Paul Kocialkowski 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index db0015e5..690f73ef 100644
--- a/configure.ac
+++ b/configure.ac
@@ -183,7 +183,7 @@ PKG_CHECK_MODULES(GSL, gsl)
 # for chamelium
 AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium],
  [Enable building of chamelium libraries and tests (default: 
yes)]),
- [enable_chamelium=yes], [enable_chamelium=no])
+ [enable_chamelium=no], [enable_chamelium=yes])
 AM_CONDITIONAL(HAVE_CHAMELIUM, [test "x$enable_chamelium" = xyes])
 if test "x$enable_chamelium" = xyes; then
PKG_CHECK_MODULES(XMLRPC, xmlrpc xmlrpc_util xmlrpc_client, [],
-- 
2.13.2

___
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/selftests: Mark contexts as lost during freeing of mock device

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Mark contexts as lost during freeing of mock device
URL   : https://patchwork.freedesktop.org/series/27580/
State : success

== Summary ==

Series 27580v1 drm/i915/selftests: Mark contexts as lost during freeing of mock 
device
https://patchwork.freedesktop.org/api/1.0/series/27580/revisions/1/mbox/

Test gem_sync:
Subgroup basic-store-all:
fail   -> PASS   (fi-ivb-3520m) fdo#17

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:448s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:430s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:358s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:538s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:514s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:494s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:486s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:602s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:422s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:416s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:507s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:476s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:584s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:586s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:566s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:470s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:586s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:486s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:435s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:480s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:547s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:411s

f1c32d67a1e143cba1b0ad042448f39d75d9ccaa drm-tip: 2017y-07m-19d-13h-38m-55s UTC 
integration manifest
a2a449a drm/i915/selftests: Mark contexts as lost during freeing of mock device

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5235/
___
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 gpu reset vs modeset fix, plus page_flip removal

[Chris Wilson]

Quoting Patchwork (2017-07-19 15:15:25)
> fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
> time:443s
> fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
> time:427s
> fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
> time:356s
> fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
> time:537s
> fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
> time:511s
> fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
> time:491s
> fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
> time:494s
> fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
> time:598s
> fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
> time:436s
> fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
> time:413s
> fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
> time:419s
> fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
> time:511s
> fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
> time:470s
> fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
> time:470s
> fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
> time:579s
> fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
> time:586s
> fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
> time:566s
> fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
> time:460s
> fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
> time:584s
> fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
> time:475s
> fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
> time:476s
> fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
> time:434s
> fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
> time:477s
> fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
> time:555s
> fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
> time:406s

This would be useful to run across the whole farm, for gen3/gen4
coverage. iirc some of the older ones are on trybot (but not BAT).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 6/7] drm/nouveau: Convert nouveau to use new iterator macros, v2.

[Maarten Lankhorst]

Use the new atomic iterator macros, the old ones are about to be
removed. With the new macros, it's more easy to get old and new state so
get them from the macros instead of from obj->state.

Changes since v1:
- Don't mix up old and new state. (danvet)
- Rebase on top of interruptible swap_state changes.

Signed-off-by: Maarten Lankhorst 
Cc: Ben Skeggs 
Cc: nouv...@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nv50_display.c | 72 +-
 1 file changed, 37 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index 747c99c1e474..7abfb561b00c 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2103,7 +2103,7 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct 
drm_crtc_state *state)
 
NV_ATOMIC(drm, "%s atomic_check %d\n", crtc->name, asyh->state.active);
if (asyh->state.active) {
-   for_each_connector_in_state(asyh->state.state, conn, conns, i) {
+   for_each_new_connector_in_state(asyh->state.state, conn, conns, 
i) {
if (conns->crtc == crtc) {
asyc = nouveau_conn_atom(conns);
break;
@@ -3905,9 +3905,9 @@ static void
 nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
 {
struct drm_device *dev = state->dev;
-   struct drm_crtc_state *crtc_state;
+   struct drm_crtc_state *new_crtc_state;
struct drm_crtc *crtc;
-   struct drm_plane_state *plane_state;
+   struct drm_plane_state *new_plane_state;
struct drm_plane *plane;
struct nouveau_drm *drm = nouveau_drm(dev);
struct nv50_disp *disp = nv50_disp(dev);
@@ -3926,8 +3926,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
mutex_lock(>mutex);
 
/* Disable head(s). */
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
struct nv50_head *head = nv50_head(crtc);
 
NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
@@ -3940,8 +3940,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
}
 
/* Disable plane(s). */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+   struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
 
NV_ATOMIC(drm, "%s: clr %02x (set %02x)\n", plane->name,
@@ -4006,8 +4006,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
}
 
/* Update head(s). */
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
struct nv50_head *head = nv50_head(crtc);
 
NV_ATOMIC(drm, "%s: set %04x (clr %04x)\n", crtc->name,
@@ -4019,14 +4019,14 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
}
}
 
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   if (crtc->state->event)
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   if (new_crtc_state->event)
drm_crtc_vblank_get(crtc);
}
 
/* Update plane(s). */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+   struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
 
NV_ATOMIC(drm, "%s: set %02x (clr %02x)\n", plane->name,
@@ -4056,23 +4056,23 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
mutex_unlock(>mutex);
 
/* Wait for HW to signal completion. */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+   for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+   struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
struct nv50_wndw *wndw = nv50_wndw(plane);
int ret = nv50_wndw_wait_armed(wndw, asyw);
if (ret)
NV_ERROR(drm, "%s: 

[Intel-gfx] [PATCH v2 5/7] drm/msm: Convert to use new iterator macros, v2.

[Maarten Lankhorst]

for_each_obj_in_state is about to be removed, so convert
to the new iterator macros.

Just like in omap, use crtc_state->active instead of
crtc_state->enable when waiting for completion.

Changes since v1:
- Fix compilation.

Signed-off-by: Maarten Lankhorst 
Cc: Rob Clark 
Cc: Archit Taneja 
Cc: Vincent Abriou 
Cc: Maarten Lankhorst 
Cc: Russell King 
Cc: Rob Herring 
Cc: Markus Elfring 
Cc: Sushmita Susheelendra 
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Reviewed-by: Daniel Vetter 
Tested-by: Archit Taneja 
---
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c |  4 ++--
 drivers/gpu/drm/msm/msm_atomic.c| 18 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c 
b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index bcd1f5cac72c..f7f087419ed8 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -114,7 +114,7 @@ static void mdp4_prepare_commit(struct msm_kms *kms, struct 
drm_atomic_state *st
mdp4_enable(mdp4_kms);
 
/* see 119ecb7fd */
-   for_each_crtc_in_state(state, crtc, crtc_state, i)
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i)
drm_crtc_vblank_get(crtc);
 }
 
@@ -126,7 +126,7 @@ static void mdp4_complete_commit(struct msm_kms *kms, 
struct drm_atomic_state *s
struct drm_crtc_state *crtc_state;
 
/* see 119ecb7fd */
-   for_each_crtc_in_state(state, crtc, crtc_state, i)
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i)
drm_crtc_vblank_put(crtc);
 
mdp4_disable(mdp4_kms);
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index badfa8717317..025d454163b0 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -84,13 +84,13 @@ static void msm_atomic_wait_for_commit_done(struct 
drm_device *dev,
struct drm_atomic_state *old_state)
 {
struct drm_crtc *crtc;
-   struct drm_crtc_state *crtc_state;
+   struct drm_crtc_state *new_crtc_state;
struct msm_drm_private *priv = old_state->dev->dev_private;
struct msm_kms *kms = priv->kms;
int i;
 
-   for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
-   if (!crtc->state->enable)
+   for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
+   if (!new_crtc_state->active)
continue;
 
kms->funcs->wait_for_crtc_commit_done(kms, crtc);
@@ -195,7 +195,7 @@ int msm_atomic_commit(struct drm_device *dev,
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_plane *plane;
-   struct drm_plane_state *plane_state;
+   struct drm_plane_state *old_plane_state, *new_plane_state;
int i, ret;
 
ret = drm_atomic_helper_prepare_planes(dev, state);
@@ -211,19 +211,19 @@ int msm_atomic_commit(struct drm_device *dev,
/*
 * Figure out what crtcs we have:
 */
-   for_each_crtc_in_state(state, crtc, crtc_state, i)
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i)
c->crtc_mask |= drm_crtc_mask(crtc);
 
/*
 * Figure out what fence to wait for:
 */
-   for_each_plane_in_state(state, plane, plane_state, i) {
-   if ((plane->state->fb != plane_state->fb) && plane_state->fb) {
-   struct drm_gem_object *obj = 
msm_framebuffer_bo(plane_state->fb, 0);
+   for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
new_plane_state, i) {
+   if ((new_plane_state->fb != old_plane_state->fb) && 
new_plane_state->fb) {
+   struct drm_gem_object *obj = 
msm_framebuffer_bo(new_plane_state->fb, 0);
struct msm_gem_object *msm_obj = to_msm_bo(obj);
struct dma_fence *fence = 
reservation_object_get_excl_rcu(msm_obj->resv);
 
-   drm_atomic_set_fence_for_plane(plane_state, fence);
+   drm_atomic_set_fence_for_plane(new_plane_state, fence);
}
}
 
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 2/7] drm/atomic: Clean up drm_atomic_helper_async_check

[Maarten Lankhorst]

Instead of assigning plane to __plane, iterate over plane
which is the same thing. Simplify the check for n_planes != 1,
at that point we know plane != NULL as well.

Rename obj_state to new_obj_state, and get rid of the bogus stuff.
crtc->state->state is NEVER non-null, if it is, there is a bug.
We should definitely try to prevent async updates if there are flips
queued, but this code will simply not be executed and needs to be
rethought.

Also remove the loop too, because we're trying to loop over the crtc until
we find plane_state->crtc == crtc, which we already know is non-zero.
It's dead code anyway. :)

Signed-off-by: Maarten Lankhorst 
Cc: Gustavo Padovan 
---
 drivers/gpu/drm/drm_atomic_helper.c | 56 ++---
 1 file changed, 15 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index a46b51291006..c538528a794a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1372,68 +1372,42 @@ int drm_atomic_helper_async_check(struct drm_device 
*dev,
   struct drm_atomic_state *state)
 {
struct drm_crtc *crtc;
-   struct drm_crtc_state *crtc_state;
-   struct drm_crtc_commit *commit;
-   struct drm_plane *__plane, *plane = NULL;
-   struct drm_plane_state *__plane_state, *plane_state = NULL;
+   struct drm_crtc_state *new_crtc_state;
+   struct drm_plane *plane;
+   struct drm_plane_state *new_plane_state;
const struct drm_plane_helper_funcs *funcs;
-   int i, j, n_planes = 0;
+   int i, n_planes = 0;
 
-   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
-   if (drm_atomic_crtc_needs_modeset(crtc_state))
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   if (drm_atomic_crtc_needs_modeset(new_crtc_state))
return -EINVAL;
}
 
-   for_each_new_plane_in_state(state, __plane, __plane_state, i) {
+   for_each_new_plane_in_state(state, plane, new_plane_state, i)
n_planes++;
-   plane = __plane;
-   plane_state = __plane_state;
-   }
 
/* FIXME: we support only single plane updates for now */
-   if (!plane || n_planes != 1)
+   if (n_planes != 1)
return -EINVAL;
 
-   if (!plane_state->crtc)
+   if (!new_plane_state->crtc)
return -EINVAL;
 
funcs = plane->helper_private;
if (!funcs->atomic_async_update)
return -EINVAL;
 
-   if (plane_state->fence)
+   if (new_plane_state->fence)
return -EINVAL;
 
/*
-* Don't do an async update if there is an outstanding commit modifying
-* the plane.  This prevents our async update's changes from getting
-* overridden by a previous synchronous update's state.
+* FIXME: We should prevent an async update if there is an outstanding
+* commit modifying the plane.  This prevents our async update's
+* changes from getting overwritten by a previous synchronous update's
+* state.
 */
-   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
-   if (plane->crtc != crtc)
-   continue;
-
-   spin_lock(>commit_lock);
-   commit = list_first_entry_or_null(>commit_list,
- struct drm_crtc_commit,
- commit_entry);
-   if (!commit) {
-   spin_unlock(>commit_lock);
-   continue;
-   }
-   spin_unlock(>commit_lock);
-
-   if (!crtc->state->state)
-   continue;
-
-   for_each_plane_in_state(crtc->state->state, __plane,
-   __plane_state, j) {
-   if (__plane == plane)
-   return -EINVAL;
-   }
-   }
 
-   return funcs->atomic_async_check(plane, plane_state);
+   return funcs->atomic_async_check(plane, new_plane_state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_async_check);
 
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 1/7] drm/atomic: Use new iterator macros in drm_atomic_helper_wait_for_flip_done, again.

[Maarten Lankhorst]

for_each_obj_in_state is about to be removed, so use the correct new
iterator macro.

I renamed the variable to 'unused', but forgot to convert
drm_atomic_helper_wait_for_flip_done to the new iterator macro,
so make it work this time.

Signed-off-by: Maarten Lankhorst 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: Sean Paul 
Cc: David Airlie 
---
 drivers/gpu/drm/drm_atomic_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 70146f809db5..a46b51291006 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1270,7 +1270,7 @@ void drm_atomic_helper_wait_for_flip_done(struct 
drm_device *dev,
struct drm_crtc *crtc;
int i;
 
-   for_each_crtc_in_state(old_state, crtc, unused, i) {
+   for_each_new_crtc_in_state(old_state, crtc, unused, i) {
struct drm_crtc_commit *commit = old_state->crtcs[i].commit;
int ret;
 
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 4/7] drm/omapdrm: Fix omap_atomic_wait_for_completion

[Maarten Lankhorst]

Use the new iterator macro and look for crtc_state->active instead of
enable, only crtc_state->enable implies that vblanks will happen.

Signed-off-by: Maarten Lankhorst 
Cc: Tomi Valkeinen 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 022029ea6972..66d3c6bfd6a8 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -57,13 +57,13 @@ static void omap_fb_output_poll_changed(struct drm_device 
*dev)
 static void omap_atomic_wait_for_completion(struct drm_device *dev,
struct drm_atomic_state *old_state)
 {
-   struct drm_crtc_state *old_crtc_state;
+   struct drm_crtc_state *new_crtc_state;
struct drm_crtc *crtc;
unsigned int i;
int ret;
 
-   for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
-   if (!crtc->state->enable)
+   for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
+   if (!new_crtc_state->active)
continue;
 
ret = omap_crtc_wait_pending(crtc);
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 7/7] drm/atomic: Remove deprecated accessor macros

[Maarten Lankhorst]

Now that the last users have been converted, we can finally get rid of
for_each_obj_in_state, we have better macros to replace them with.

Signed-off-by: Maarten Lankhorst 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: Sean Paul 
Cc: David Airlie 
---
 include/drm/drm_atomic.h| 75 -
 include/drm/drm_connector.h |  3 +-
 include/drm/drm_crtc.h  |  8 ++---
 include/drm/drm_plane.h |  8 ++---
 4 files changed, 9 insertions(+), 85 deletions(-)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 7cd0f303f5a3..679e746f0459 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -563,31 +563,6 @@ int __must_check drm_atomic_nonblocking_commit(struct 
drm_atomic_state *state);
 void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 
 /**
- * for_each_connector_in_state - iterate over all connectors in an atomic 
update
- * @__state:  drm_atomic_state pointer
- * @connector:  drm_connector iteration cursor
- * @connector_state:  drm_connector_state iteration cursor
- * @__i: int iteration cursor, for macro-internal use
- *
- * This iterates over all connectors in an atomic update. Note that before the
- * software state is committed (by calling drm_atomic_helper_swap_state(), this
- * points to the new state, while afterwards it points to the old state. Due to
- * this tricky confusion this macro is deprecated.
- *
- * FIXME:
- *
- * Replace all usage of this with one of the explicit iterators below and then
- * remove this macro.
- */
-#define for_each_connector_in_state(__state, connector, connector_state, __i) \
-   for ((__i) = 0; \
-(__i) < (__state)->num_connector &&
\
-((connector) = (__state)->connectors[__i].ptr, 
\
-(connector_state) = (__state)->connectors[__i].state, 1);  \
-(__i)++)   \
-   for_each_if (connector)
-
-/**
  * for_each_oldnew_connector_in_state - iterate over all connectors in an 
atomic update
  * @__state:  drm_atomic_state pointer
  * @connector:  drm_connector iteration cursor
@@ -651,31 +626,6 @@ void drm_state_dump(struct drm_device *dev, struct 
drm_printer *p);
for_each_if (connector)
 
 /**
- * for_each_crtc_in_state - iterate over all connectors in an atomic update
- * @__state:  drm_atomic_state pointer
- * @crtc:  drm_crtc iteration cursor
- * @crtc_state:  drm_crtc_state iteration cursor
- * @__i: int iteration cursor, for macro-internal use
- *
- * This iterates over all CRTCs in an atomic update. Note that before the
- * software state is committed (by calling drm_atomic_helper_swap_state(), this
- * points to the new state, while afterwards it points to the old state. Due to
- * this tricky confusion this macro is deprecated.
- *
- * FIXME:
- *
- * Replace all usage of this with one of the explicit iterators below and then
- * remove this macro.
- */
-#define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \
-   for ((__i) = 0; \
-(__i) < (__state)->dev->mode_config.num_crtc &&\
-((crtc) = (__state)->crtcs[__i].ptr,   \
-(crtc_state) = (__state)->crtcs[__i].state, 1);\
-(__i)++)   \
-   for_each_if (crtc_state)
-
-/**
  * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update
  * @__state:  drm_atomic_state pointer
  * @crtc:  drm_crtc iteration cursor
@@ -735,31 +685,6 @@ void drm_state_dump(struct drm_device *dev, struct 
drm_printer *p);
for_each_if (crtc)
 
 /**
- * for_each_plane_in_state - iterate over all planes in an atomic update
- * @__state:  drm_atomic_state pointer
- * @plane:  drm_plane iteration cursor
- * @plane_state:  drm_plane_state iteration cursor
- * @__i: int iteration cursor, for macro-internal use
- *
- * This iterates over all planes in an atomic update. Note that before the
- * software state is committed (by calling drm_atomic_helper_swap_state(), this
- * points to the new state, while afterwards it points to the old state. Due to
- * this tricky confusion this macro is deprecated.
- *
- * FIXME:
- *
- * Replace all usage of this with one of the explicit iterators below and then
- * remove this macro.
- */
-#define for_each_plane_in_state(__state, plane, plane_state, __i)  
\
-   for ((__i) = 0; \
-(__i) < (__state)->dev->mode_config.num_total_plane && \
-((plane) = (__state)->planes[__i].ptr, 
\
-(plane_state) = (__state)->planes[__i].state, 1);

[Intel-gfx] [PATCH v2 3/7] drm/rcar-du: Use new iterator macros, v2.

[Maarten Lankhorst]

for_each_obj_in_state is about to be removed, so use the correct new
iterator macros.

Also look at new_plane_state instead of plane->state when looking up
the hw planes in use. They should be the same except when reallocating,
(in which case this code is skipped) and we should really stop looking
at obj->state whenever possible.

Changes since v1:
- Actually compile correctly.

Signed-off-by: Maarten Lankhorst 
Cc: Laurent Pinchart 
Cc: linux-renesas-...@vger.kernel.org
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 57 -
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c 
b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index dcde6288da6c..50fd793c38d1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -51,12 +51,9 @@
  */
 
 static bool rcar_du_plane_needs_realloc(struct rcar_du_plane *plane,
+   const struct rcar_du_plane_state 
*cur_state,
struct rcar_du_plane_state *new_state)
 {
-   struct rcar_du_plane_state *cur_state;
-
-   cur_state = to_rcar_plane_state(plane->plane.state);
-
/* Lowering the number of planes doesn't strictly require reallocation
 * as the extra hardware plane will be freed when committing, but doing
 * so could lead to more fragmentation.
@@ -141,16 +138,17 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
unsigned int groups = 0;
unsigned int i;
struct drm_plane *drm_plane;
-   struct drm_plane_state *drm_plane_state;
+   struct drm_plane_state *old_drm_plane_state, *new_drm_plane_state;
 
/* Check if hardware planes need to be reallocated. */
-   for_each_plane_in_state(state, drm_plane, drm_plane_state, i) {
-   struct rcar_du_plane_state *plane_state;
+   for_each_oldnew_plane_in_state(state, drm_plane, old_drm_plane_state, 
new_drm_plane_state, i) {
+   struct rcar_du_plane_state *old_plane_state, *new_plane_state;
struct rcar_du_plane *plane;
unsigned int index;
 
plane = to_rcar_plane(drm_plane);
-   plane_state = to_rcar_plane_state(drm_plane_state);
+   old_plane_state = to_rcar_plane_state(old_drm_plane_state);
+   new_plane_state = to_rcar_plane_state(new_drm_plane_state);
 
dev_dbg(rcdu->dev, "%s: checking plane (%u,%tu)\n", __func__,
plane->group->index, plane - plane->group->planes);
@@ -159,19 +157,19 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
 * the full reallocation procedure. Just mark the hardware
 * plane(s) as freed.
 */
-   if (!plane_state->format) {
+   if (!new_plane_state->format) {
dev_dbg(rcdu->dev, "%s: plane is being disabled\n",
__func__);
index = plane - plane->group->planes;
group_freed_planes[plane->group->index] |= 1 << index;
-   plane_state->hwindex = -1;
+   new_plane_state->hwindex = -1;
continue;
}
 
/* If the plane needs to be reallocated mark it as such, and
 * mark the hardware plane(s) as free.
 */
-   if (rcar_du_plane_needs_realloc(plane, plane_state)) {
+   if (rcar_du_plane_needs_realloc(plane, old_plane_state, 
new_plane_state)) {
dev_dbg(rcdu->dev, "%s: plane needs reallocation\n",
__func__);
groups |= 1 << plane->group->index;
@@ -179,7 +177,7 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
 
index = plane - plane->group->planes;
group_freed_planes[plane->group->index] |= 1 << index;
-   plane_state->hwindex = -1;
+   new_plane_state->hwindex = -1;
}
}
 
@@ -204,7 +202,7 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
 
for (i = 0; i < group->num_planes; ++i) {
struct rcar_du_plane *plane = >planes[i];
-   struct rcar_du_plane_state *plane_state;
+   struct rcar_du_plane_state *new_plane_state;
struct drm_plane_state *s;
 
s = drm_atomic_get_plane_state(state, >plane);
@@ -226,16 +224,16 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
continue;
}
 
-   plane_state = to_rcar_plane_state(plane->plane.state);
-   used_planes |= 

[Intel-gfx] [PATCH v2 0/7] drm/atomic: Remove deprecated atomic iterator macros, v2.

[Maarten Lankhorst]

It's time to kill off for_each_obj_in_state, and convert
the remainder to for_each_(old/new/both)_obj_in_state.

Some patches have been upstreamed, these are the remaining few
with compile fixes.

Maarten Lankhorst (7):
  drm/atomic: Use new iterator macros in
  drm_atomic_helper_wait_for_flip_done, again.
  drm/atomic: Clean up drm_atomic_helper_async_check
  drm/rcar-du: Use new iterator macros, v2.
  drm/omapdrm: Fix omap_atomic_wait_for_completion
  drm/msm: Convert to use new iterator macros, v2.
  drm/nouveau: Convert nouveau to use new iterator macros, v2.
  drm/atomic: Remove deprecated accessor macros

 drivers/gpu/drm/drm_atomic_helper.c | 58 +++--
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c |  4 +-
 drivers/gpu/drm/msm/msm_atomic.c| 18 
 drivers/gpu/drm/nouveau/nv50_display.c  | 72 ---
 drivers/gpu/drm/omapdrm/omap_drv.c  |  6 +--
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 57 -
 include/drm/drm_atomic.h| 75 -
 include/drm/drm_connector.h |  3 +-
 include/drm/drm_crtc.h  |  8 ++--
 include/drm/drm_plane.h |  8 ++--
 10 files changed, 104 insertions(+), 205 deletions(-)

-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 3/4] drm/i915/execlists: Split insert_request()

[Chris Wilson]

Quoting Michał Winiarski (2017-07-19 15:23:47)
> On Mon, Jul 17, 2017 at 09:42:34AM +0100, Chris Wilson wrote:
> Matches what I have in my tree, except for "first" hidden in pointer.
> Can we #define the bit where we're keeping the "first" status? This way we can
> immediatelly see what's going on in the callers.

At least give me a name for that bit!

#define PRIOLIST_FLAGS_MASK 0x1
#define PRIOLIST_FIRST 0x1

Never going to fit in 80cols! :(
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 2/4] drm/i915/execlists: Keep request->priority for its lifetime

[Chris Wilson]

Quoting Michał Winiarski (2017-07-19 15:25:51)
> On Mon, Jul 17, 2017 at 09:42:33AM +0100, Chris Wilson wrote:
> > With preemption, we will want to "unsubmit" a request, taking it back
> > from the hw and returning it to the priority sorted execution list. In
> > order to know where to insert it into that list, we need to remember
> > its adjust priority (which may change even as it was being executed).
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Michal Winiarski 
> 
> We should also change GuC dequeue/irq_handler.

I wasn't sure if that was necessary as the current shortcut of sealing
the priority once submitted applies to guc dequeue as it currently
doesn't unsubmit. (Wasn't much point in penalising the guc until it was
ready.)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 1/4] drm/i915/execlists: Move insert_request()

[Michał Winiarski]

On Mon, Jul 17, 2017 at 09:42:32AM +0100, Chris Wilson wrote:
> Move insert_request() earlier to avoid a forward declaration in a later
> patch.
> 
> Signed-off-by: Chris Wilson 

Reviewed-by: Michał Winiarski 

-Michał

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 128 
> +++
>  1 file changed, 64 insertions(+), 64 deletions(-)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 2/4] drm/i915/execlists: Keep request->priority for its lifetime

[Michał Winiarski]

On Mon, Jul 17, 2017 at 09:42:33AM +0100, Chris Wilson wrote:
> With preemption, we will want to "unsubmit" a request, taking it back
> from the hw and returning it to the priority sorted execution list. In
> order to know where to insert it into that list, we need to remember
> its adjust priority (which may change even as it was being executed).
> 
> Signed-off-by: Chris Wilson 
> Cc: Michal Winiarski 

We should also change GuC dequeue/irq_handler.

With that:

Reviewed-by: Michał Winiarski 

-Michał

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 1b2f0e3d383a..8ab0c4b76c98 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -552,8 +552,6 @@ static void execlists_dequeue(struct intel_engine_cs 
> *engine)
>   }
>  
>   INIT_LIST_HEAD(>priotree.link);
> - rq->priotree.priority = INT_MAX;
> -
>   __i915_gem_request_submit(rq);
>   trace_i915_gem_request_in(rq, port_index(port, engine));
>   last = rq;
> @@ -687,6 +685,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>   execlists_context_status_change(rq, 
> INTEL_CONTEXT_SCHEDULE_OUT);
>  
>   trace_i915_gem_request_out(rq);
> + rq->priotree.priority = INT_MAX;
>   i915_gem_request_put(rq);
>  
>   port[0] = port[1];
> -- 
> 2.13.2
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 3/4] drm/i915/execlists: Split insert_request()

[Michał Winiarski]

On Mon, Jul 17, 2017 at 09:42:34AM +0100, Chris Wilson wrote:
> In the next patch we will want to reinsert a request not at the end of
> the priority queue, but at the front. Here we split insert_request()
> into two, the first function retrieves the priority list (for reuse for
> unsubmit later) and a wrapper function to insert at the end of that list
> and to schedule the tasklet if we were first.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 35 +++
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 8ab0c4b76c98..d227480b3a26 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -292,10 +292,10 @@ uint64_t intel_lr_context_descriptor(struct 
> i915_gem_context *ctx,
>   return ctx->engine[engine->id].lrc_desc;
>  }
>  
> -static bool
> -insert_request(struct intel_engine_cs *engine,
> -struct i915_priotree *pt,
> -int prio)
> +static struct i915_priolist *
> +lookup_priolist(struct intel_engine_cs *engine,
> + struct i915_priotree *pt,
> + int prio)
>  {
>   struct i915_priolist *p;
>   struct rb_node **parent, *rb;
> @@ -317,8 +317,7 @@ insert_request(struct intel_engine_cs *engine,
>   parent = >rb_right;
>   first = false;
>   } else {
> - list_add_tail(>link, >requests);
> - return false;
> + return p;
>   }
>   }
>  
> @@ -344,16 +343,14 @@ insert_request(struct intel_engine_cs *engine,
>   }
>  
>   p->priority = prio;
> + INIT_LIST_HEAD(>requests);
>   rb_link_node(>node, rb, parent);
>   rb_insert_color(>node, >execlist_queue);
>  
> - INIT_LIST_HEAD(>requests);
> - list_add_tail(>link, >requests);
> -
>   if (first)
>   engine->execlist_first = >node;
>  
> - return first;
> + return ptr_pack_bits(p, first, 1);

Matches what I have in my tree, except for "first" hidden in pointer.
Can we #define the bit where we're keeping the "first" status? This way we can
immediatelly see what's going on in the callers.
Or at least add a comment...

With that:

Reviewed-by: Michał Winiarski 

While this is an RFC, I think 1-3 make the code more clear, and could be pushed
as-is (perhaps this one with slightly amended commit message s/next 
patch/future)

-Michał

>  }
>  
>  static inline void
> @@ -712,6 +709,17 @@ static void intel_lrc_irq_handler(unsigned long data)
>   intel_uncore_forcewake_put(dev_priv, engine->fw_domains);
>  }
>  
> +static void insert_request(struct intel_engine_cs *engine,
> +struct i915_priotree *pt,
> +int prio)
> +{
> + struct i915_priolist *p = lookup_priolist(engine, pt, prio);
> +
> + list_add_tail(>link, _mask_bits(p, 1)->requests);
> + if (ptr_unmask_bits(p, 1) && execlists_elsp_ready(engine))
> + tasklet_hi_schedule(>irq_tasklet);
> +}
> +
>  static void execlists_submit_request(struct drm_i915_gem_request *request)
>  {
>   struct intel_engine_cs *engine = request->engine;
> @@ -720,12 +728,7 @@ static void execlists_submit_request(struct 
> drm_i915_gem_request *request)
>   /* Will be called from irq-context when using foreign fences. */
>   spin_lock_irqsave(>timeline->lock, flags);
>  
> - if (insert_request(engine,
> ->priotree,
> -request->priotree.priority)) {
> - if (execlists_elsp_ready(engine))
> - tasklet_hi_schedule(>irq_tasklet);
> - }
> + insert_request(engine, >priotree, request->priotree.priority);
>  
>   GEM_BUG_ON(!engine->execlist_first);
>   GEM_BUG_ON(list_empty(>priotree.link));
> -- 
> 2.13.2
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: Remove intel_flip_work infrastructure

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 14:24:20)
> On Wed, Jul 19, 2017 at 02:07:36PM +0100, Chris Wilson wrote:
> > Quoting Daniel Vetter (2017-07-19 13:55:01)
> > > This gets rid of all the interactions between the legacy flip code and
> > > the modeset code. Yay!
> > 
> > Including our missed vblank boosting. (That's been dead for a while.)
> 
> Hm right, but should be easy to add (and bonus, for every display update,
> not just flips) in the intel_sw_fence_wait function. Can you pls point me
> at what function I should call to reverse-boost an i915_sw_fence (and only
> that)?

You would have to walk the tree of input sw fences, detect those are
dma_fences and check if they are a request. Then for each request mark
it as boosted. That information is easier to keep during construction
rather than recovering it later, though honestly, I would just boost the
exclusive fences for the atomic state, i.e. use the reservation_object 
under the assumption that the snapshot hasn't drifted too much.

> Then we could queue up a vblank callback that fires on the next vblank for
> any of the CRTC in the update and boosts the i915_sw_fence. If we just
> boost the fence (instead of the context or something) it should also be
> easy to filter out boosting when the request has completed meanwhile.

Yup, boosting is now completely tied to requests.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for gpu reset vs modeset fix, plus page_flip removal

[Patchwork]

== Series Details ==

Series: gpu reset vs modeset fix, plus page_flip removal
URL   : https://patchwork.freedesktop.org/series/27576/
State : success

== Summary ==

Series 27576v1 gpu reset vs modeset fix, plus page_flip removal
https://patchwork.freedesktop.org/api/1.0/series/27576/revisions/1/mbox/

Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> SKIP   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-n2820) fdo#101705

fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:443s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:427s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:356s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:537s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:511s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:491s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:494s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:598s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:436s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:413s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:419s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:511s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:470s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:470s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:579s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:586s
fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
time:566s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:460s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:584s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:475s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:476s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:434s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:477s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:555s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:406s

64257e941fca5385627380375782bc4724436366 drm-tip: 2017y-07m-19d-12h-38m-49s UTC 
integration manifest
60eaaff drm/i915: Drop unpin stall in atomic_prepare_commit
a633438 drm/i915: Remove intel_flip_work infrastructure
ee84f0f drm/i915: adjust has_pending_fb_unpin to atomic
0004897 drm/i915: Rip out legacy page_flip completion/irq handling
33b5239 drm/i915: More surgically unbreak the modeset vs reset deadlock
d3325b0 drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit
e17d3a6 drm/i915: Avoid the gpu reset vs. modeset deadlock
887c8c2 drm/i915: Unbreak gpu reset vs. modeset locking
a10774a drm/i915: Nuke legacy flip queueing code

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5234/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/9] drm/i915: More surgically unbreak the modeset vs reset deadlock

[Daniel Vetter]

On Wed, Jul 19, 2017 at 4:05 PM, Daniel Vetter  wrote:
> On Wed, Jul 19, 2017 at 3:42 PM, Chris Wilson  
> wrote:
>> Quoting Daniel Vetter (2017-07-19 13:54:58)
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 5aa7ca1ab592..4762f158032d 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -3471,10 +3471,9 @@ void intel_prepare_reset(struct drm_i915_private 
>>> *dev_priv)
>>> !gpu_reset_clobbers_display(dev_priv))
>>> return;
>>>
>>> -   /* We have a modeset vs reset deadlock, defensively unbreak it.
>>> -*
>>> -* FIXME: We can do a _lot_ better, this is just a first 
>>> iteration.*/
>>> -   i915_gem_set_wedged(dev_priv);
>>> +   /* We have a modeset vs reset deadlock, defensively unbreak it. */
>>> +   set_bit(I915_RESET_MODESET, _priv->gpu_error.flags);
>>> +   wake_up_all(_priv->gpu_error.wait_queue);
>>
>> How are we breaking the
>>
>> modeset_lock -> struct_mutex -> wait_on_reset ?
>>
>> We wait the modeset_lock next which stops the reset from
>> proceeding, and so the deadlock persists until the wedge-me timeout?
>
> Hm indeed, I didn't check my logs carefully enough and there's still
> "i915_reset_device timed out" in it. But I also thought the only real
> wait we have left for the gpu is the one under i915_sw_fence. I think
> we could simply switch i915_mutex_lock_interruptible calls in atomic
> modeset over mutex_lock_interruptible? Or is there another can of
> worms I'm missing?

Obviously, because that's what we're doing already. But I don't have
the DRM_ERROR from i915_gem_wait_for_error anywhere in my logs either,
so not clear what exactly is going on ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/9] drm/i915: adjust has_pending_fb_unpin to atomic

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 14:15:44)
> On Wed, Jul 19, 2017 at 02:06:43PM +0100, Chris Wilson wrote:
> > Quoting Daniel Vetter (2017-07-19 13:55:00)
> > > A bit an oversight - the current code did nothing, since only
> > > legacy flips used the unpin_work_count and assorted logic.
> > > 
> > > Cc: Maarten Lankhorst 
> > > Cc: Ville Syrjälä 
> > > Signed-off-by: Daniel Vetter 
> > 
> > There's a fence deadlock testcase for this, kms_flip/fence?
> 
> Crunching through them, but since I tend to test my stuff all mixed into
> one pile I've hit a bug in a different patch series of mine. Given that
> we've run without this for a while, not sure it's that critical really.

It's only going to affect gen2 (realistically using 14 fences in a gen3
batch is unlikely) if we can have 3 fb in the pipeline as userspace
assumes 2 are reserved for the flip. Or at least if we may race while fb
holds 3 fences due to the wq.

EDEADLK is not something I've seen outside of buggy code, but it is
certainly possible and this patch should fix it.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/9] drm/i915: More surgically unbreak the modeset vs reset deadlock

[Daniel Vetter]

On Wed, Jul 19, 2017 at 3:42 PM, Chris Wilson  wrote:
> Quoting Daniel Vetter (2017-07-19 13:54:58)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 5aa7ca1ab592..4762f158032d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3471,10 +3471,9 @@ void intel_prepare_reset(struct drm_i915_private 
>> *dev_priv)
>> !gpu_reset_clobbers_display(dev_priv))
>> return;
>>
>> -   /* We have a modeset vs reset deadlock, defensively unbreak it.
>> -*
>> -* FIXME: We can do a _lot_ better, this is just a first iteration.*/
>> -   i915_gem_set_wedged(dev_priv);
>> +   /* We have a modeset vs reset deadlock, defensively unbreak it. */
>> +   set_bit(I915_RESET_MODESET, _priv->gpu_error.flags);
>> +   wake_up_all(_priv->gpu_error.wait_queue);
>
> How are we breaking the
>
> modeset_lock -> struct_mutex -> wait_on_reset ?
>
> We wait the modeset_lock next which stops the reset from
> proceeding, and so the deadlock persists until the wedge-me timeout?

Hm indeed, I didn't check my logs carefully enough and there's still
"i915_reset_device timed out" in it. But I also thought the only real
wait we have left for the gpu is the one under i915_sw_fence. I think
we could simply switch i915_mutex_lock_interruptible calls in atomic
modeset over mutex_lock_interruptible? Or is there another can of
worms I'm missing?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Fixes that failed to backport to v4.13-rc1

[Maarten Lankhorst]

Op 19-07-17 om 15:17 schreef Jani Nikula:
> Another kernel, another list of failed backports.
>
> The following commits have been marked as Cc: stable or fixing something
> in v4.13-rc1 or earlier, but failed to cherry-pick to
> drm-intel-fixes. Please see if they are worth backporting, and please do
> so if they are.
>
> BR,
> Jani.
>
> 54d20ed1fff2 ("drm/i915: Fix bad comparison in skl_compute_plane_wm, v2.")
>
v1 of this patch will apply. It was the version from before the renames that 
conflict here. :)

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/selftests: Mark contexts as lost during freeing of mock device

[Chris Wilson]

We need to unpin the last retired context early in the shutdown sequence
so that its RCU free is done before we try to free the context ida. I
included this in a later patch ("drm/i915: Keep a recent cache of freed
contexts objects for reuse") and so missed that the selftests were broken
in the meantime.

Reported-by: Matthew Auld 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101627
Fixes: 5f09a9c8ab6b ("drm/i915: Allow contexts to be unreferenced locklessly")
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Tvrtko Ursulin 
Cc: Mika Kuoppala 
Cc: Matthew Auld 
---
 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c 
b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index d451dfbe9bbb..dda413c95b89 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -54,6 +54,7 @@ static void mock_device_release(struct drm_device *dev)
 
mutex_lock(>drm.struct_mutex);
mock_device_flush(i915);
+   i915_gem_contexts_lost(i915);
mutex_unlock(>drm.struct_mutex);
 
cancel_delayed_work_sync(>gt.retire_work);
-- 
2.13.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 9/9] drm/i915: Drop unpin stall in atomic_prepare_commit

[Maarten Lankhorst]

Op 19-07-17 om 14:55 schreef Daniel Vetter:
> The core already does this in setup_commit(). With this we can also
> remove the unpin_work_count since it's the last user.
>
> Cc: Maarten Lankhorst 
> Cc: Ville Syrjälä 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +
>  drivers/gpu/drm/i915/intel_drv.h |  2 --
>  2 files changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e52a2adbaaa5..351208b7b1ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11825,18 +11825,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  static int intel_atomic_prepare_commit(struct drm_device *dev,
>  struct drm_atomic_state *state)
>  {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct drm_crtc_state *crtc_state;
> - struct drm_crtc *crtc;
> - int i, ret;
> -
> - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> - if (state->legacy_cursor_update)
> - continue;
> -
> - if (atomic_read(_intel_crtc(crtc)->unpin_work_count) >= 2)
> - flush_workqueue(dev_priv->wq);
> - }
> + int ret;
>  
>   ret = mutex_lock_interruptible(>struct_mutex);
>   if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 9cb7e781e863..96402c06e295 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -798,8 +798,6 @@ struct intel_crtc {
>   unsigned long long enabled_power_domains;
>   struct intel_overlay *overlay;
>  
> - atomic_t unpin_work_count;
> -
>   /* Display surface base address adjustement for pageflips. Note that on
>* gen4+ this only adjusts up to a tile, offsets within a tile are
>* handled in the hw itself (with the TILEOFF register). */

I like red diffs..

For patch 1, 4 (with updated commit message), 6-9:
Reviewed-by: Maarten Lankhorst 


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v3] tests/chamelium: Detect analog bridges and handle EDID accordingly

[Paul Kocialkowski]

Nowadays, many VGA connectors are not actually native VGA but use a
discrete bridge to a digital connector. These bridges usually enforce
their own EDID instead of the one supplied by the chamelium.

Thus, the EDID read test for VGA is not relevant in that case and
should be skipped. Reported modes may also go beyond what the chamelium
can support. Thus, only supported resolutions should be tested for the
frame dump test and others should be pruned.

Signed-off-by: Paul Kocialkowski 
---
 tests/chamelium.c | 78 +++
 1 file changed, 78 insertions(+)

diff --git a/tests/chamelium.c b/tests/chamelium.c
index 33ecc2e7..881b7fa9 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -130,6 +130,76 @@ wait_for_connector(data_t *data, struct chamelium_port 
*port,
igt_assert(finished);
 }
 
+static int chamelium_vga_modes[][2] = {
+   { 1600, 1200 },
+   { 1920, 1200 },
+   { 1920, 1080 },
+   { 1680, 1050 },
+   { 1280, 1024 },
+   { 1280, 960 },
+   { 1440, 900 },
+   { 1280, 800 },
+   { 1024, 768 },
+   { 1360, 768 },
+   { 1280, 720 },
+   { 800, 600 },
+   { 640, 480 },
+   { -1, -1 },
+};
+
+static bool
+prune_vga_mode(data_t *data, drmModeModeInfo *mode)
+{
+   int i = 0;
+
+   while (chamelium_vga_modes[i][0] != -1) {
+   if (mode->hdisplay == chamelium_vga_modes[i][0] &&
+   mode->vdisplay == chamelium_vga_modes[i][1])
+   return false;
+
+   i++;
+   }
+
+   return true;
+}
+
+static bool
+check_analog_bridge(data_t *data, struct chamelium_port *port)
+{
+   drmModePropertyBlobPtr edid_blob = NULL;
+   drmModeConnector *connector = chamelium_port_get_connector(
+   data->chamelium, port, false);
+   uint64_t edid_blob_id;
+   unsigned char *edid;
+   char edid_vendor[3];
+
+   if (chamelium_port_get_type(port) != DRM_MODE_CONNECTOR_VGA)
+   return false;
+
+   igt_assert(kmstest_get_property(data->drm_fd, connector->connector_id,
+   DRM_MODE_OBJECT_CONNECTOR, "EDID", NULL,
+   _blob_id, NULL));
+   igt_assert(edid_blob = drmModeGetPropertyBlob(data->drm_fd,
+ edid_blob_id));
+
+   edid = (unsigned char *) edid_blob->data;
+
+   edid_vendor[0] = ((edid[8] & 0x7c) >> 2) + '@';
+   edid_vendor[1] = (((edid[8] & 0x03) << 3) |
+ ((edid[9] & 0xe0) >> 5)) + '@';
+   edid_vendor[2] = (edid[9] & 0x1f) + '@';
+
+   /* Analog bridges provide their own EDID */
+   if (edid_vendor[0] != 'I' || edid_vendor[1] != 'G' ||
+   edid_vendor[0] != 'T')
+   return true;
+
+   drmModeFreePropertyBlob(edid_blob);
+   drmModeFreeConnector(connector);
+
+   return false;
+}
+
 static void
 reset_state(data_t *data, struct chamelium_port *port)
 {
@@ -193,6 +263,8 @@ test_edid_read(data_t *data, struct chamelium_port *port,
chamelium_plug(data->chamelium, port);
wait_for_connector(data, port, DRM_MODE_CONNECTED);
 
+   igt_skip_on(check_analog_bridge(data, port));
+
igt_assert(kmstest_get_property(data->drm_fd, connector->connector_id,
DRM_MODE_OBJECT_CONNECTOR, "EDID", NULL,
_blob_id, NULL));
@@ -547,15 +619,21 @@ test_analog_frame_dump(data_t *data, struct 
chamelium_port *port)
drmModeModeInfo *mode;
drmModeConnector *connector;
int fb_id, i;
+   bool bridge;
 
output = prepare_output(data, , port);
connector = chamelium_port_get_connector(data->chamelium, port, false);
primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
igt_assert(primary);
 
+   bridge = check_analog_bridge(data, port);
+
for (i = 0; i < connector->count_modes; i++) {
mode = >modes[i];
 
+   if (bridge && prune_vga_mode(data, mode))
+   continue;
+
fb_id = igt_create_color_pattern_fb(data->drm_fd,
mode->hdisplay, 
mode->vdisplay,
DRM_FORMAT_XRGB,
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v3 0/1] tests/chamelium: Detect analogue bridges and handle EDID accordingly

[Paul Kocialkowski]

This patch applies on top of: Analogue/VGA frame comparison support

Changes since v2:
* Changed analogue in favor of analog
* Rebased on top of the new revisions of the series this depends on

Changes since v1:
* Rebased on top of the new revisions of the series this depends on

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v3 2/2] chamelium: Add support for VGA frame comparison testing

[Paul Kocialkowski]

This adds support for VGA frame comparison testing with the reference
generated from cairo. The retrieved frame from the chamelium is first
cropped, as it contains the blanking intervals, through a dedicated
helper. Another helper function asserts that the analog frame
matches or dump it to png if not.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_chamelium.c | 164 ++--
 lib/igt_chamelium.h |   7 ++-
 tests/chamelium.c   |  57 ++
 3 files changed, 222 insertions(+), 6 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 348d2176..dcd8855f 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -937,6 +937,8 @@ static cairo_surface_t *convert_frame_dump_argb32(const 
struct chamelium_frame_d
int w = dump->width, h = dump->height;
uint32_t *bits_bgr = (uint32_t *) dump->bgr;
unsigned char *bits_argb;
+   unsigned char *bits_target;
+   int size;
 
image_bgr = pixman_image_create_bits(
PIXMAN_b8g8r8, w, h, bits_bgr,
@@ -946,9 +948,13 @@ static cairo_surface_t *convert_frame_dump_argb32(const 
struct chamelium_frame_d
 
bits_argb = (unsigned char *) pixman_image_get_data(image_argb);
 
-   dump_surface = cairo_image_surface_create_for_data(
-   bits_argb, CAIRO_FORMAT_ARGB32, w, h,
-   PIXMAN_FORMAT_BPP(PIXMAN_x8r8g8b8) / 8 * w);
+   dump_surface = cairo_image_surface_create(
+   CAIRO_FORMAT_ARGB32, w, h);
+
+   bits_target = cairo_image_surface_get_data(dump_surface);
+   size = cairo_image_surface_get_stride(dump_surface) * h;
+   memcpy(bits_target, bits_argb, size);
+   cairo_surface_mark_dirty(dump_surface);
 
pixman_image_unref(image_argb);
 
@@ -1054,6 +1060,154 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium 
*chamelium,
 }
 
 /**
+ * chamelium_assert_analog_frame_match_or_dump:
+ * @chamelium: The chamelium instance the frame dump belongs to
+ * @frame: The chamelium frame dump to match
+ * @fb: pointer to an #igt_fb structure
+ *
+ * Asserts that the provided captured frame matches the reference frame from
+ * the framebuffer. If they do not, this saves the reference and captured 
frames
+ * to a png file.
+ */
+void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
+struct chamelium_port *port,
+const struct 
chamelium_frame_dump *frame,
+struct igt_fb *fb)
+{
+   cairo_surface_t *reference;
+   cairo_surface_t *capture;
+   igt_crc_t *reference_crc;
+   igt_crc_t *capture_crc;
+   char *reference_suffix;
+   char *capture_suffix;
+   bool match;
+
+   /* Grab the reference frame from framebuffer */
+   reference = igt_get_cairo_surface(chamelium->drm_fd, fb);
+
+   /* Grab the captured frame from chamelium */
+   capture = convert_frame_dump_argb32(frame);
+
+   match = igt_check_analog_frame_match(reference, capture);
+   if (!match && igt_frame_dump_is_enabled()) {
+   reference_crc = chamelium_calculate_fb_crc(chamelium->drm_fd,
+  fb);
+   capture_crc = chamelium_get_crc_for_area(chamelium, port, 0, 0,
+0, 0);
+
+   reference_suffix = igt_crc_to_string_extended(reference_crc,
+ '-', 2);
+   capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
+   2);
+
+   /* Write reference and capture frames to png */
+   igt_write_compared_frames_to_png(reference, capture,
+reference_suffix,
+capture_suffix);
+
+   free(reference_suffix);
+   free(capture_suffix);
+   }
+
+   cairo_surface_destroy(capture);
+
+   igt_assert(match);
+}
+
+
+/**
+ * chamelium_analog_frame_crop:
+ * @chamelium: The Chamelium instance to use
+ * @dump: The chamelium frame dump to crop
+ * @width: The cropped frame width
+ * @height: The cropped frame height
+ *
+ * Detects the corners of a chamelium frame and crops it to the requested
+ * width/height. This is useful for VGA frame dumps that also contain the
+ * pixels dumped during the blanking intervals.
+ *
+ * The detection is done on a brightness-threshold-basis, that is adapted
+ * to the reference frame used by i-g-t. It may not be as relevant for other
+ * frames.
+ */
+void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump, int width,
+int height)
+{
+   unsigned char *bgr;
+   unsigned char *p;
+   unsigned char *q;

[Intel-gfx] [PATCH i-g-t v3 0/2] Analogue/VGA frame comparison support

[Paul Kocialkowski]

Changes since v2:
* Changed analogue in favor of analog
* Integrated analog frame match fixup in the original commit
* Rebased on top of the new revisions of the series this depends on

Changes since v1:
* Split analogue frame comparison to igt_frame, using cairo surfaces
* Added a chamelium helper for analogue frame checking and frame dumping

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v3 1/2] lib/igt_frame: Add support for analog frame comparison testing

[Paul Kocialkowski]

This adds support for analog frame comparison check, as used in VGA.
Since VGA uses a DAC-ADC chain, its data cannot be expected to be pixel
perfect. Thus, it is impossible to uses a CRC check and full frames have
to be analyzed instead. Such an analysis is implemented, based on both
an absolute error threshold and a correlation with the expected error
trend for a DAC-ADC chain. It was tested with a couple encoders and
provides reliable error detection with few false positives.

Signed-off-by: Paul Kocialkowski 
---
 configure.ac|   1 +
 lib/Makefile.am |   2 +
 lib/igt_frame.c | 131 
 lib/igt_frame.h |   2 +
 4 files changed, 136 insertions(+)

diff --git a/configure.ac b/configure.ac
index 5b41f333..db0015e5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -178,6 +178,7 @@ if test x"$udev" = xyes; then
AC_DEFINE(HAVE_UDEV,1,[Enable udev-based monitor hotplug detection])
 fi
 PKG_CHECK_MODULES(GLIB, glib-2.0)
+PKG_CHECK_MODULES(GSL, gsl)
 
 # for chamelium
 AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium],
diff --git a/lib/Makefile.am b/lib/Makefile.am
index d4f41128..fb922ced 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -35,6 +35,7 @@ AM_CFLAGS = \
$(DRM_CFLAGS) \
$(PCIACCESS_CFLAGS) \
$(LIBUNWIND_CFLAGS) \
+   $(GSL_CFLAGS) \
$(KMOD_CFLAGS) \
$(PROCPS_CFLAGS) \
$(DEBUG_CFLAGS) \
@@ -54,6 +55,7 @@ libintel_tools_la_LIBADD = \
$(DRM_LIBS) \
$(PCIACCESS_LIBS) \
$(PROCPS_LIBS) \
+   $(GSL_LIBS) \
$(KMOD_LIBS) \
$(CAIRO_LIBS) \
$(LIBUDEV_LIBS) \
diff --git a/lib/igt_frame.c b/lib/igt_frame.c
index dfafe53d..222a45f8 100644
--- a/lib/igt_frame.c
+++ b/lib/igt_frame.c
@@ -29,6 +29,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "igt.h"
 
@@ -135,3 +137,132 @@ void igt_write_compared_frames_to_png(cairo_surface_t 
*reference,
 
close(fd);
 }
+
+/**
+ * igt_check_analog_frame_match:
+ * @reference: The reference cairo surface
+ * @capture: The captured cairo surface
+ *
+ * Checks that the analog image contained in the chamelium frame dump matches
+ * the given framebuffer.
+ *
+ * In order to determine whether the frame matches the reference, the following
+ * reasoning is implemented:
+ * 1. The absolute error for each color value of the reference is collected.
+ * 2. The average absolute error is calculated for each color value of the
+ *reference and must not go above 60 (23.5 % of the total range).
+ * 3. A linear fit for the average absolute error from the pixel value is
+ *calculated, as a DAC-ADC chain is expected to have a linear error curve.
+ * 4. The linear fit is correlated with the actual average absolute error for
+ *the frame and the correlation coefficient is checked to be > 0.985,
+ *indicating a match with the expected error trend.
+ *
+ * Most errors (e.g. due to scaling, rotation, color space, etc) can be
+ * reliably detected this way, with a minimized number of false-positives.
+ * However, the brightest values (250 and up) are ignored as the error trend
+ * is often not linear there in practice due to clamping.
+ *
+ * Returns: a boolean indicating whether the frames match
+ */
+
+bool igt_check_analog_frame_match(cairo_surface_t *reference,
+ cairo_surface_t *capture)
+{
+   pixman_image_t *reference_src, *capture_src;
+   int w, h;
+   int error_count[3][256][2] = { 0 };
+   double error_average[4][250];
+   double error_trend[250];
+   double c0, c1, cov00, cov01, cov11, sumsq;
+   double correlation;
+   unsigned char *reference_pixels, *capture_pixels;
+   unsigned char *p;
+   unsigned char *q;
+   bool match = true;
+   int diff;
+   int x, y;
+   int i, j;
+
+   w = cairo_image_surface_get_width(reference);
+   h = cairo_image_surface_get_height(reference);
+
+   reference_src = pixman_image_create_bits(
+   PIXMAN_x8r8g8b8, w, h,
+   (void*)cairo_image_surface_get_data(reference),
+   cairo_image_surface_get_stride(reference));
+   reference_pixels = (unsigned char *) 
pixman_image_get_data(reference_src);
+
+   capture_src = pixman_image_create_bits(
+   PIXMAN_x8r8g8b8, w, h,
+   (void*)cairo_image_surface_get_data(capture),
+   cairo_image_surface_get_stride(capture));
+   capture_pixels = (unsigned char *) pixman_image_get_data(capture_src);
+
+   /* Collect the absolute error for each color value */
+   for (x = 0; x < w; x++) {
+   for (y = 0; y < h; y++) {
+   p = _pixels[(x + y * w) * 4];
+   q = _pixels[(x + y * w) * 4];
+
+   for (i = 0; i < 3; i++) {
+   diff = (int) p[i] - q[i];
+   

[Intel-gfx] [PATCH i-g-t v5 6/7] chamelium: Dump captured and reference frames to png on crc error

[Paul Kocialkowski]

This adds support for dumping both the frame capture from the chamelium
and the reference frame generated by cairo when the captured crc does
not match the crc calculated from the reference, using common helpers.

Getting a dump of the frames is quite useful in order to compare them.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_chamelium.c | 81 +
 lib/igt_chamelium.h |  4 +++
 tests/chamelium.c   | 14 ++---
 3 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index abcdc522..348d2176 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -929,6 +929,32 @@ static pixman_image_t *convert_frame_format(pixman_image_t 
*src,
return converted;
 }
 
+static cairo_surface_t *convert_frame_dump_argb32(const struct 
chamelium_frame_dump *dump)
+{
+   cairo_surface_t *dump_surface;
+   pixman_image_t *image_bgr;
+   pixman_image_t *image_argb;
+   int w = dump->width, h = dump->height;
+   uint32_t *bits_bgr = (uint32_t *) dump->bgr;
+   unsigned char *bits_argb;
+
+   image_bgr = pixman_image_create_bits(
+   PIXMAN_b8g8r8, w, h, bits_bgr,
+   PIXMAN_FORMAT_BPP(PIXMAN_b8g8r8) / 8 * w);
+   image_argb = convert_frame_format(image_bgr, PIXMAN_x8r8g8b8);
+   pixman_image_unref(image_bgr);
+
+   bits_argb = (unsigned char *) pixman_image_get_data(image_argb);
+
+   dump_surface = cairo_image_surface_create_for_data(
+   bits_argb, CAIRO_FORMAT_ARGB32, w, h,
+   PIXMAN_FORMAT_BPP(PIXMAN_x8r8g8b8) / 8 * w);
+
+   pixman_image_unref(image_argb);
+
+   return dump_surface;
+}
+
 /**
  * chamelium_assert_frame_eq:
  * @chamelium: The chamelium instance the frame dump belongs to
@@ -973,6 +999,61 @@ void chamelium_assert_frame_eq(const struct chamelium 
*chamelium,
 }
 
 /**
+ * chamelium_assert_crc_eq_or_dump:
+ * @chamelium: The chamelium instance the frame dump belongs to
+ * @reference_crc: The CRC for the reference frame
+ * @capture_crc: The CRC for the captured frame
+ * @fb: pointer to an #igt_fb structure
+ *
+ * Asserts that the CRC provided for both the reference and the captured frame
+ * are identical. If they are not, this grabs the captured frame and saves it
+ * along with the reference to a png file.
+ */
+void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
+igt_crc_t *reference_crc,
+igt_crc_t *capture_crc, struct igt_fb *fb,
+int index)
+{
+   struct chamelium_frame_dump *frame;
+   cairo_surface_t *reference;
+   cairo_surface_t *capture;
+   char *reference_suffix;
+   char *capture_suffix;
+   bool eq;
+
+   eq = igt_check_crc_equal(reference_crc, capture_crc);
+   if (!eq && igt_frame_dump_is_enabled()) {
+   /* Grab the reference frame from framebuffer */
+   reference = igt_get_cairo_surface(chamelium->drm_fd, fb);
+
+   /* Grab the captured frame from chamelium */
+   frame = chamelium_read_captured_frame(chamelium, index);
+   igt_assert(frame);
+
+   capture = convert_frame_dump_argb32(frame);
+
+   reference_suffix = igt_crc_to_string_extended(reference_crc,
+ '-', 2);
+   capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
+   2);
+
+   /* Write reference and capture frames to png */
+   igt_write_compared_frames_to_png(reference, capture,
+reference_suffix,
+capture_suffix);
+
+   free(reference_suffix);
+   free(capture_suffix);
+
+   chamelium_destroy_frame_dump(frame);
+
+   cairo_surface_destroy(capture);
+   }
+
+   igt_assert(eq);
+}
+
+/**
  * chamelium_get_frame_limit:
  * @chamelium: The Chamelium instance to use
  * @port: The port to check the frame limit on
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index 2bfbfdc7..80afcafa 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -105,6 +105,10 @@ int chamelium_get_frame_limit(struct chamelium *chamelium,
 void chamelium_assert_frame_eq(const struct chamelium *chamelium,
   const struct chamelium_frame_dump *dump,
   struct igt_fb *fb);
+void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
+igt_crc_t *reference_crc,
+igt_crc_t *capture_crc, struct igt_fb *fb,
+int index);
 void chamelium_destroy_frame_dump(struct chamelium_frame_dump *dump);
 
 #endif /* 

[Intel-gfx] [PATCH i-g-t v5 3/7] lib/igt_debugfs: Introduce CRC check function, with logic made common

[Paul Kocialkowski]

This introduces an igt_check_crc_equal function in addition to
igt_assert_crc_equal and makes the CRC comparison logic from the latter
common. In particular, an igt_find_crc_mismatch function indicates
whether there is a mistmatch and at what index, so that the calling
functions can print the diverging values.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_debugfs.c | 53 ++---
 lib/igt_debugfs.h |  1 +
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 2702686a..ef05dc77 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -281,6 +281,26 @@ bool igt_debugfs_search(int device, const char *filename, 
const char *substring)
  * Pipe CRC
  */
 
+static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b,
+ int *index)
+{
+   int i;
+
+   if (a->n_words != b->n_words)
+   return true;
+
+   for (i = 0; i < a->n_words; i++) {
+   if (a->crc[i] != b->crc[i]) {
+   if (index)
+   *index = i;
+
+   return true;
+   }
+   }
+
+   return false;
+}
+
 /**
  * igt_assert_crc_equal:
  * @a: first pipe CRC value
@@ -294,10 +314,37 @@ bool igt_debugfs_search(int device, const char *filename, 
const char *substring)
  */
 void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b)
 {
-   int i;
+   int index;
+   bool mismatch;
+
+   mismatch = igt_find_crc_mismatch(a, b, );
+   if (mismatch)
+   igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index,
+ a->crc[index], b->crc[index]);
+
+   igt_assert(!mismatch);
+}
+
+/**
+ * igt_check_crc_equal:
+ * @a: first pipe CRC value
+ * @b: second pipe CRC value
+ *
+ * Compares two CRC values and return whether they match.
+ *
+ * Returns: A boolean indicating whether the CRC values match
+ */
+bool igt_check_crc_equal(const igt_crc_t *a, const igt_crc_t *b)
+{
+   int index;
+   bool mismatch;
+
+   mismatch = igt_find_crc_mismatch(a, b, );
+   if (mismatch)
+   igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index,
+ a->crc[index], b->crc[index]);
 
-   for (i = 0; i < a->n_words; i++)
-   igt_assert_eq_u32(a->crc[i], b->crc[i]);
+   return !mismatch;
 }
 
 /**
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 7b846a83..fe355919 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -114,6 +114,7 @@ enum intel_pipe_crc_source {
 };
 
 void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
+bool igt_check_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
 char *igt_crc_to_string(igt_crc_t *crc);
 
 void igt_require_pipe_crc(int fd);
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v5 7/7] tests/chamelium: Merge the crc testing functions into one

[Paul Kocialkowski]

This merges the two test_display_crc_single and
test_display_crc_multiple functions into one, with a variable number of
frames to capture. This reduces code duplication.

Signed-off-by: Paul Kocialkowski 
---
 tests/chamelium.c | 72 +++
 1 file changed, 8 insertions(+), 64 deletions(-)

diff --git a/tests/chamelium.c b/tests/chamelium.c
index 1567386e..34448152 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -420,65 +420,7 @@ disable_output(data_t *data,
 }
 
 static void
-test_display_crc_single(data_t *data, struct chamelium_port *port)
-{
-   igt_display_t display;
-   igt_output_t *output;
-   igt_plane_t *primary;
-   igt_crc_t *crc;
-   igt_crc_t *expected_crc;
-   struct chamelium_fb_crc_async_data *fb_crc;
-   struct igt_fb fb;
-   drmModeModeInfo *mode;
-   drmModeConnector *connector;
-   int fb_id, i, captured_frame_count;
-
-   reset_state(data, port);
-
-   output = prepare_output(data, , port);
-   connector = chamelium_port_get_connector(data->chamelium, port, false);
-   primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-   igt_assert(primary);
-
-   for (i = 0; i < connector->count_modes; i++) {
-   mode = >modes[i];
-   fb_id = igt_create_color_pattern_fb(data->drm_fd,
-   mode->hdisplay,
-   mode->vdisplay,
-   DRM_FORMAT_XRGB,
-   LOCAL_DRM_FORMAT_MOD_NONE,
-   0, 0, 0, );
-   igt_assert(fb_id > 0);
-
-   fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
-   );
-   enable_output(data, port, output, mode, );
-
-   igt_debug("Testing single CRC fetch\n");
-
-   chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
-   crc = chamelium_read_captured_crcs(data->chamelium,
-  _frame_count);
-
-   expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
-
-   chamelium_assert_crc_eq_or_dump(data->chamelium, expected_crc,
-   crc, , 0);
-
-   igt_assert_crc_equal(crc, expected_crc);
-   free(expected_crc);
-   free(crc);
-
-   disable_output(data, port, output);
-   igt_remove_fb(data->drm_fd, );
-   }
-
-   drmModeFreeConnector(connector);
-   igt_display_fini();
-}
-
-static void
-test_display_crc_multiple(data_t *data, struct chamelium_port *port)
+test_display_crc(data_t *data, struct chamelium_port *port, int count)
 {
igt_display_t display;
igt_output_t *output;
@@ -517,10 +459,12 @@ test_display_crc_multiple(data_t *data, struct 
chamelium_port *port)
 * there's always the potential the driver isn't able to keep
 * the display running properly for very long
 */
-   chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 3);
+   chamelium_capture(data->chamelium, port, 0, 0, 0, 0, count);
crc = chamelium_read_captured_crcs(data->chamelium,
   _frame_count);
 
+   igt_assert(captured_frame_count == count);
+
igt_debug("Captured %d frames\n", captured_frame_count);
 
expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
@@ -737,10 +681,10 @@ igt_main
edid_id, alt_edid_id);
 
connector_subtest("dp-crc-single", DisplayPort)
-   test_display_crc_single(, port);
+   test_display_crc(, port, 1);
 
connector_subtest("dp-crc-multiple", DisplayPort)
-   test_display_crc_multiple(, port);
+   test_display_crc(, port, 3);
 
connector_subtest("dp-frame-dump", DisplayPort)
test_display_frame_dump(, port);
@@ -794,10 +738,10 @@ igt_main
edid_id, alt_edid_id);
 
connector_subtest("hdmi-crc-single", HDMIA)
-   test_display_crc_single(, port);
+   test_display_crc(, port, 1);
 
connector_subtest("hdmi-crc-multiple", HDMIA)
-   test_display_crc_multiple(, port);
+   test_display_crc(, port, 3);
 
connector_subtest("hdmi-frame-dump", HDMIA)
test_display_frame_dump(, port);
-- 
2.13.2

[Intel-gfx] [PATCH i-g-t v5 1/7] lib/igt_fb: Export the cairo surface instead of writing to a png

[Paul Kocialkowski]

This removes the igt_write_fb_to_png function (that was unused thus far)
and exports the igt_get_cairo_surface function to grab the matching
cairo surface. Writing to a png is now handled by the common frame
handling code in lib/igt_frame.

This also fixes how the surface is retreived in chamelium code,
which avoids destroying it too early.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_chamelium.c |  7 +--
 lib/igt_fb.c| 36 +---
 lib/igt_fb.h|  2 +-
 3 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index bff08c0e..93392af7 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -936,17 +936,13 @@ void chamelium_assert_frame_eq(const struct chamelium 
*chamelium,
   const struct chamelium_frame_dump *dump,
   struct igt_fb *fb)
 {
-   cairo_t *cr;
cairo_surface_t *fb_surface;
pixman_image_t *reference_src, *reference_bgr;
int w = dump->width, h = dump->height;
bool eq;
 
/* Get the cairo surface for the framebuffer */
-   cr = igt_get_cairo_ctx(chamelium->drm_fd, fb);
-   fb_surface = cairo_get_target(cr);
-   cairo_surface_reference(fb_surface);
-   cairo_destroy(cr);
+   fb_surface = igt_get_cairo_surface(chamelium->drm_fd, fb);
 
/*
 * Convert the reference image into the same format as the chamelium
@@ -964,7 +960,6 @@ void chamelium_assert_frame_eq(const struct chamelium 
*chamelium,
dump->size) == 0;
 
pixman_image_unref(reference_bgr);
-   cairo_surface_destroy(fb_surface);
 
igt_fail_on_f(!eq,
  "Chamelium frame dump didn't match reference image\n");
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d2b7e9e3..93e21c17 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1124,7 +1124,18 @@ static void create_cairo_surface__gtt(int fd, struct 
igt_fb *fb)
fb, destroy_cairo_surface__gtt);
 }
 
-static cairo_surface_t *get_cairo_surface(int fd, struct igt_fb *fb)
+/**
+ * igt_get_cairo_surface:
+ * @fd: open drm file descriptor
+ * @fb: pointer to an #igt_fb structure
+ *
+ * This function stores the contents of the supplied framebuffer into a cairo
+ * surface and returns it.
+ *
+ * Returns:
+ * A pointer to a cairo surface with the contents of the framebuffer.
+ */
+cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb)
 {
if (fb->cairo_surface == NULL) {
if (fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
@@ -1160,7 +1171,7 @@ cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb)
cairo_surface_t *surface;
cairo_t *cr;
 
-   surface = get_cairo_surface(fd, fb);
+   surface = igt_get_cairo_surface(fd, fb);
cr = cairo_create(surface);
cairo_surface_destroy(surface);
igt_assert(cairo_status(cr) == CAIRO_STATUS_SUCCESS);
@@ -1173,27 +1184,6 @@ cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb)
 }
 
 /**
- * igt_write_fb_to_png:
- * @fd: open i915 drm file descriptor
- * @fb: pointer to an #igt_fb structure
- * @filename: target name for the png image
- *
- * This function stores the contents of the supplied framebuffer into a png
- * image stored at @filename.
- */
-void igt_write_fb_to_png(int fd, struct igt_fb *fb, const char *filename)
-{
-   cairo_surface_t *surface;
-   cairo_status_t status;
-
-   surface = get_cairo_surface(fd, fb);
-   status = cairo_surface_write_to_png(surface, filename);
-   cairo_surface_destroy(surface);
-
-   igt_assert(status == CAIRO_STATUS_SUCCESS);
-}
-
-/**
  * igt_remove_fb:
  * @fd: open i915 drm file descriptor
  * @fb: pointer to an #igt_fb structure
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 4a680cef..f8a845cc 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -132,6 +132,7 @@ int igt_create_bo_with_dimensions(int fd, int width, int 
height, uint32_t format
 uint64_t igt_fb_mod_to_tiling(uint64_t modifier);
 
 /* cairo-based painting */
+cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb);
 cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb);
 void igt_paint_color(cairo_t *cr, int x, int y, int w, int h,
 double r, double g, double b);
@@ -145,7 +146,6 @@ void igt_paint_color_gradient_range(cairo_t *cr, int x, int 
y, int w, int h,
 void igt_paint_test_pattern(cairo_t *cr, int width, int height);
 void igt_paint_image(cairo_t *cr, const char *filename,
 int dst_x, int dst_y, int dst_width, int dst_height);
-void igt_write_fb_to_png(int fd, struct igt_fb *fb, const char *filename);
 int igt_cairo_printf_line(cairo_t *cr, enum igt_text_align align,
   double yspacing, const char *fmt, ...)
   __attribute__((format (printf, 4, 5)));
-- 
2.13.2

[Intel-gfx] [PATCH i-g-t v5 2/7] chamelium: Calculate CRC from framebuffer instead of hardcoding it

[Paul Kocialkowski]

This introduces CRC calculation for reference frames, instead of using
hardcoded values for them. The rendering of reference frames may differ
from machine to machine, especially due to font rendering, and the
frame itself may change with subsequent IGT changes.

These differences would cause the CRC checks to fail on different
setups. This allows them to pass regardless of the setup.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_chamelium.c | 143 
 lib/igt_chamelium.h |   5 ++
 tests/chamelium.c   |  77 +++-
 3 files changed, 167 insertions(+), 58 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 93392af7..abcdc522 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -94,6 +94,13 @@ struct chamelium_frame_dump {
struct chamelium_port *port;
 };
 
+struct chamelium_fb_crc_async_data {
+   cairo_surface_t *fb_surface;
+
+   pthread_t thread_id;
+   igt_crc_t *ret;
+};
+
 struct chamelium {
xmlrpc_env env;
xmlrpc_client *client;
@@ -998,6 +1005,142 @@ int chamelium_get_frame_limit(struct chamelium 
*chamelium,
return ret;
 }
 
+static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, int width,
+ int height, int k, int m)
+{
+   unsigned char r, g, b;
+   uint64_t sum = 0;
+   uint64_t count = 0;
+   uint64_t value;
+   uint32_t hash;
+   int index;
+   int i;
+
+   for (i=0; i < width * height; i++) {
+   if ((i % m) != k)
+   continue;
+
+   index = i * 4;
+
+   r = buffer[index + 2];
+   g = buffer[index + 1];
+   b = buffer[index + 0];
+
+   value = r | (g << 8) | (b << 16);
+   sum += ++count * value;
+   }
+
+   hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> 48)) & 0x;
+
+   return hash;
+}
+
+static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
+ igt_crc_t *out)
+{
+   unsigned char *buffer;
+   int n = 4;
+   int w, h;
+   int i, j;
+
+   buffer = cairo_image_surface_get_data(fb_surface);
+   w = cairo_image_surface_get_width(fb_surface);
+   h = cairo_image_surface_get_height(fb_surface);
+
+   for (i = 0; i < n; i++) {
+   j = n - i - 1;
+   out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, n);
+   }
+
+   out->n_words = n;
+}
+
+/**
+ * chamelium_calculate_fb_crc:
+ * @fd: The drm file descriptor
+ * @fb: The framebuffer to calculate the CRC for
+ *
+ * Calculates the CRC for the provided framebuffer, using the Chamelium's CRC
+ * algorithm. This calculates the CRC in a synchronous fashion.
+ *
+ * Returns: The calculated CRC
+ */
+igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
+{
+   igt_crc_t *ret = calloc(1, sizeof(igt_crc_t));
+   cairo_surface_t *fb_surface;
+
+   /* Get the cairo surface for the framebuffer */
+   fb_surface = igt_get_cairo_surface(fd, fb);
+
+   chamelium_do_calculate_fb_crc(fb_surface, ret);
+
+   return ret;
+}
+
+static void *chamelium_calculate_fb_crc_async_work(void *data)
+{
+   struct chamelium_fb_crc_async_data *fb_crc;
+
+   fb_crc = (struct chamelium_fb_crc_async_data *) data;
+
+   chamelium_do_calculate_fb_crc(fb_crc->fb_surface, fb_crc->ret);
+
+   return NULL;
+}
+
+/**
+ * chamelium_calculate_fb_crc_launch:
+ * @fd: The drm file descriptor
+ * @fb: The framebuffer to calculate the CRC for
+ *
+ * Launches the CRC calculation for the provided framebuffer, using the
+ * Chamelium's CRC algorithm. This calculates the CRC in an asynchronous
+ * fashion.
+ *
+ * The returned structure should be passed to a subsequent call to
+ * chamelium_calculate_fb_crc_result. It should not be freed.
+ *
+ * Returns: An intermediate structure for the CRC calculation work.
+ */
+struct chamelium_fb_crc_async_data *chamelium_calculate_fb_crc_async_start(int 
fd,
+  
struct igt_fb *fb)
+{
+   struct chamelium_fb_crc_async_data *fb_crc;
+
+   fb_crc = calloc(1, sizeof(struct chamelium_fb_crc_async_data));
+   fb_crc->ret = calloc(1, sizeof(igt_crc_t));
+
+   /* Get the cairo surface for the framebuffer */
+   fb_crc->fb_surface = igt_get_cairo_surface(fd, fb);
+
+   pthread_create(_crc->thread_id, NULL,
+  chamelium_calculate_fb_crc_async_work, fb_crc);
+
+   return fb_crc;
+}
+
+/**
+ * chamelium_calculate_fb_crc_result:
+ * @fb_crc: An intermediate structure with thread-related information
+ *
+ * Blocks until the asynchronous CRC calculation is finished, and then returns
+ * its result.
+ *
+ * Returns: The calculated CRC
+ */
+igt_crc_t *chamelium_calculate_fb_crc_async_finish(struct 

[Intel-gfx] [PATCH i-g-t v5 5/7] lib/igt_debugfs: Add extended helper to format crc to string

[Paul Kocialkowski]

This introduces a igt_crc_to_string_extended helper that allows
formatting a crc to a string with a given delimiter and size to print
per crc word.

Signed-off-by: Paul Kocialkowski 
---
 lib/igt_debugfs.c | 27 +++
 lib/igt_debugfs.h |  1 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index ef05dc77..2aa33586 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -348,28 +348,47 @@ bool igt_check_crc_equal(const igt_crc_t *a, const 
igt_crc_t *b)
 }
 
 /**
- * igt_crc_to_string:
+ * igt_crc_to_string_extended:
  * @crc: pipe CRC value to print
+ * @delimiter: The delimiter to use between crc words
+ * @crc_size: the number of bytes to print per crc word (either 4 or 2)
  *
- * This function allocates a string and formats @crc into it.
+ * This function allocates a string and formats @crc into it, depending on
+ * @delimiter and @crc_size.
  * The caller is responsible for freeing the string.
  *
  * This should only ever be used for diagnostic debug output.
  */
-char *igt_crc_to_string(igt_crc_t *crc)
+char *igt_crc_to_string_extended(igt_crc_t *crc, char delimiter, int crc_size)
 {
int i;
char *buf = calloc(128, sizeof(char));
+   const char *format[2] = { "%08x%c", "%04x%c" };
 
if (!buf)
return NULL;
 
for (i = 0; i < crc->n_words; i++)
-   sprintf(buf + strlen(buf), "%08x ", crc->crc[i]);
+   sprintf(buf + strlen(buf), format[crc_size == 2], crc->crc[i],
+   i == (crc->n_words - 1) ? '\0' : delimiter);
 
return buf;
 }
 
+/**
+ * igt_crc_to_string:
+ * @crc: pipe CRC value to print
+ *
+ * This function allocates a string and formats @crc into it.
+ * The caller is responsible for freeing the string.
+ *
+ * This should only ever be used for diagnostic debug output.
+ */
+char *igt_crc_to_string(igt_crc_t *crc)
+{
+   return igt_crc_to_string_extended(crc, ' ', 4);
+}
+
 #define MAX_CRC_ENTRIES 10
 #define MAX_LINE_LEN (10 + 11 * MAX_CRC_ENTRIES + 1)
 
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index fe355919..f1a76406 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -115,6 +115,7 @@ enum intel_pipe_crc_source {
 
 void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
 bool igt_check_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
+char *igt_crc_to_string_extended(igt_crc_t *crc, char delimiter, int crc_size);
 char *igt_crc_to_string(igt_crc_t *crc);
 
 void igt_require_pipe_crc(int fd);
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v5 0/7] CRC testing with Chamelium improvements

[Paul Kocialkowski]

Changes since v4:
* Moved igt_get_cairo_surface out of the thread function to properly
  handle assert failure
* Rebased on top of current master

Changes since v3:
* Renamed structure used by async crc calculation for more clarity
* Used const qualifier for untouched buffer when hashing
* Split actual calculation to a dedicated function
* Reworked async functions names for more clarity
* Reworked descriptions for better accuracy
* Exported framebuffer cairo surface and use it directly instead of
  (unused) png dumping
* Fix how the framebuffer cairo surface is obtained to avoid destroying
  it too early

* Made CRC checking logic common
* Added a check for the same number of words
* Made frame dumping configuration and helpers common
* Added an extended crc to string helper
* Added a chamelium helper for crc checking and frame dumping
* Split the merging of crc functions to a separate patch
* Added support for frame dump path global variable
* Added listing the dumped images in a file, possibly identified with
  an id global variable

The latter allows having one "dump report" file per run, possibly
identified with the id global variable, that indicates which files
are the output, while keeping the possibility to have the same files
for different runs. This allows saving significant disk space when
the images are identified with e.g. their crc, so that duplicate results
only lead to duplicate dump reports and not duplicate images.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v5 4/7] Introduce common frame dumping configuration and helpers

[Paul Kocialkowski]

This introduces a common FrameDumpPath configuration field, as well as
helper functions in dedicated igt_frame for writing cairo surfaces
to png files.

Signed-off-by: Paul Kocialkowski 
---
 lib/Makefile.sources |   2 +
 lib/igt.h|   1 +
 lib/igt_core.c   |  12 +
 lib/igt_core.h   |   2 +-
 lib/igt_frame.c  | 137 +++
 lib/igt_frame.h  |  43 
 6 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100644 lib/igt_frame.c
 create mode 100644 lib/igt_frame.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 53fdb54c..c2e58809 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -83,6 +83,8 @@ lib_source_list = \
uwildmat/uwildmat.c \
igt_kmod.c  \
igt_kmod.h  \
+   igt_frame.c \
+   igt_frame.h \
$(NULL)
 
 .PHONY: version.h.tmp
diff --git a/lib/igt.h b/lib/igt.h
index a069deb3..d16a4991 100644
--- a/lib/igt.h
+++ b/lib/igt.h
@@ -34,6 +34,7 @@
 #include "igt_draw.h"
 #include "igt_dummyload.h"
 #include "igt_fb.h"
+#include "igt_frame.h"
 #include "igt_gt.h"
 #include "igt_kms.h"
 #include "igt_pm.h"
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 1ba79361..5a3b00e8 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -235,6 +235,10 @@
  * An example configuration follows:
  *
  * |[
+ * # The common configuration secton follows.
+ * [Common]
+ * FrameDumpPath=/tmp # The path to dump frames that fail comparison checks
+ *
  * # The following section is used for configuring the Device Under Test.
  * # It is not mandatory and allows overriding default values.
  * [DUT]
@@ -290,6 +294,7 @@ static struct {
 static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 GKeyFile *igt_key_file;
+char *frame_dump_path;
 
 const char *igt_test_name(void)
 {
@@ -621,6 +626,13 @@ static int config_parse(void)
if (!igt_key_file)
return 0;
 
+   frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
+
+   if (!frame_dump_path)
+   frame_dump_path = g_key_file_get_string(igt_key_file, "Common",
+   "FrameDumpPath",
+   );
+
rc = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
);
if (error && error->code == G_KEY_FILE_ERROR_INVALID_VALUE)
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 0739ca83..1619a9d6 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -50,7 +50,7 @@
 extern const char* __igt_test_description __attribute__((weak));
 extern bool __igt_plain_output;
 extern GKeyFile *igt_key_file;
-
+extern char *frame_dump_path;
 
 /**
  * IGT_TEST_DESCRIPTION:
diff --git a/lib/igt_frame.c b/lib/igt_frame.c
new file mode 100644
index ..dfafe53d
--- /dev/null
+++ b/lib/igt_frame.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *  Paul Kocialkowski 
+ */
+
+#include "config.h"
+
+#include 
+#include 
+#include 
+
+#include "igt.h"
+
+/**
+ * SECTION:igt_frame
+ * @short_description: Library for frame-related tests
+ * @title: Frame
+ * @include: igt_frame.h
+ *
+ * This library contains helpers for frame-related tests. This includes common
+ * frame dumping as well as frame comparison helpers.
+ */
+
+/**
+ * igt_frame_dump_is_enabled:
+ *
+ * Get whether frame dumping is enabled.
+ *
+ * Returns: A boolean indicating whether frame dumping is enabled
+ */
+bool igt_frame_dump_is_enabled(void)
+{
+   return frame_dump_path != NULL;
+}
+
+static void igt_write_frame_to_png(cairo_surface_t *surface, int fd,
+   

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Avoid the gpu reset vs. modeset deadlock

[Daniel Vetter]

On Wed, Jul 19, 2017 at 3:32 PM, Chris Wilson  wrote:
> Quoting Daniel Vetter (2017-07-19 13:54:56)
>> ... using the biggest hammer we have. This is essentially a weaponized
>> version of the timeout-based wedging Chris added in
>>
>> commit 36703e79a982c8ce5a8e43833291f2719e92d0d1
>> Author: Chris Wilson 
>> Date:   Thu Jun 22 11:56:25 2017 +0100
>>
>> drm/i915: Break modeset deadlocks on reset
>>
>> Because defense-in-depth is good it's good to still have both. Also
>> note that with the locking change we can now restrict this a lot (old
>> gpus and special testing only), so this doesn't kill the TDR benefits
>> on at least anything remotely modern.
>>
>> And futuremore with a few tricks it should be possible to make a much
>> more educated guess about whether an atomic commit is stuck waiting on
>> the gpu (atomic_t counting the pending i915_sw_fence used by the
>> atomic modeset code should do it), so we can improve this.
>>
>> But for now just start with something that is guaranteed to recover
>> faster, for much better CI througput.
>>
>> This defacto reverts TDR on these platforms, but there's not really a
>> single commit to specify as the sole offender.
>>
>> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting 
>> for request completion")
>> Fixes: 221fe7994554 ("drm/i915: Perform a direct reset of the GPU from the 
>> waiter")
>> Cc: Chris Wilson 
>> Cc: Mika Kuoppala 
>> Cc: Joonas Lahtinen 
>> Signed-off-by: Daniel Vetter 
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 9ffa1566..010a1f3e000c 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3471,6 +3471,11 @@ void intel_prepare_reset(struct drm_i915_private 
>> *dev_priv)
>> !gpu_reset_clobbers_display(dev_priv))
>> return;
>>
>> +   /* We have a modeset vs reset deadlock, defensively unbreak it.
>> +*
>> +* FIXME: We can do a _lot_ better, this is just a first iteration.*/
>
> You should keep the error message for wedging the device. If all goes
> well it is removed in a few patches, so shouldn't be an eyesore for
> long.

Yeah makes sense, just in case the next patches need to be reverted
for some reasons. That's why I split it ou. Something like

DRM_ERROR("Wedging gpu to unblock modesets\n");

and then remove that again 2 patches later?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/9] drm/i915: More surgically unbreak the modeset vs reset deadlock

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 13:54:58)
> There's no reason to entirely wedge the gpu, for the minimal deadlock
> bugfix we only need to unbreak/decouple the atomic commit from the gpu
> reset. The simplest wait to fix that is by replacing the
> unconditional fence wait a the top of commit_tail by a wait which
> completes either when the fences are done (normal case, or when a
> reset doesn't need to touch the display state). Or when the gpu reset
> needs to force-unblock all pending modeset states.
> 
> Note that in both cases TDR itself keeps working, so from a userspace
> pov this trickery isn't observable. Users themselvs might spot a short
> glitch while the rendering is catching up again, but that's still
> better than pre-TDR where we've thrown away all the rendering,
> including innocent batches. Also, this fixes the regression TDR
> introduced of making gpu resets deadlock-prone when we do need to
> touch the display.
> 
> One thing I noticed is that gpu_error.flags seems to use both our own
> wait-queue in gpu_error.wait_queue, and the generic wait_on_bit
> facilities. Not entirely sure why this inconsistency exists, I just
> picked one style.
> 
> A possible future avenue could be to insert the gpu reset in-between
> ongoing modeset changes, which would avoid the momentary glitch. But
> that's a lot more work to implement in the atomic commit machinery,
> and given that we only need this for pre-g4x hw, of questionable
> utility just for the sake of polishing gpu reset even more on those
> old boxes. It might be useful for other features though.
> 
> v2: Rebase onto 4.13 with a s/wait_queue_t/struct wait_queue_entry/.
> 
> Cc: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Joonas Lahtinen 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 35 ++-
>  2 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 07e98b07c5bc..369968539b40 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1564,6 +1564,7 @@ struct i915_gpu_error {
> unsigned long flags;
>  #define I915_RESET_BACKOFF 0
>  #define I915_RESET_HANDOFF 1
> +#define I915_RESET_MODESET 2
>  #define I915_WEDGED(BITS_PER_LONG - 1)
>  #define I915_RESET_ENGINE  (I915_WEDGED - I915_NUM_ENGINES)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 5aa7ca1ab592..4762f158032d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3471,10 +3471,9 @@ void intel_prepare_reset(struct drm_i915_private 
> *dev_priv)
> !gpu_reset_clobbers_display(dev_priv))
> return;
>  
> -   /* We have a modeset vs reset deadlock, defensively unbreak it.
> -*
> -* FIXME: We can do a _lot_ better, this is just a first iteration.*/
> -   i915_gem_set_wedged(dev_priv);
> +   /* We have a modeset vs reset deadlock, defensively unbreak it. */
> +   set_bit(I915_RESET_MODESET, _priv->gpu_error.flags);
> +   wake_up_all(_priv->gpu_error.wait_queue);

How are we breaking the

modeset_lock -> struct_mutex -> wait_on_reset ?

We wait the modeset_lock next which stops the reset from
proceeding, and so the deadlock persists until the wedge-me timeout?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Avoid the gpu reset vs. modeset deadlock

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 13:54:56)
> ... using the biggest hammer we have. This is essentially a weaponized
> version of the timeout-based wedging Chris added in
> 
> commit 36703e79a982c8ce5a8e43833291f2719e92d0d1
> Author: Chris Wilson 
> Date:   Thu Jun 22 11:56:25 2017 +0100
> 
> drm/i915: Break modeset deadlocks on reset
> 
> Because defense-in-depth is good it's good to still have both. Also
> note that with the locking change we can now restrict this a lot (old
> gpus and special testing only), so this doesn't kill the TDR benefits
> on at least anything remotely modern.
> 
> And futuremore with a few tricks it should be possible to make a much
> more educated guess about whether an atomic commit is stuck waiting on
> the gpu (atomic_t counting the pending i915_sw_fence used by the
> atomic modeset code should do it), so we can improve this.
> 
> But for now just start with something that is guaranteed to recover
> faster, for much better CI througput.
> 
> This defacto reverts TDR on these platforms, but there's not really a
> single commit to specify as the sole offender.
> 
> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for 
> request completion")
> Fixes: 221fe7994554 ("drm/i915: Perform a direct reset of the GPU from the 
> waiter")
> Cc: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Joonas Lahtinen 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 9ffa1566..010a1f3e000c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3471,6 +3471,11 @@ void intel_prepare_reset(struct drm_i915_private 
> *dev_priv)
> !gpu_reset_clobbers_display(dev_priv))
> return;
>  
> +   /* We have a modeset vs reset deadlock, defensively unbreak it.
> +*
> +* FIXME: We can do a _lot_ better, this is just a first iteration.*/

You should keep the error message for wedging the device. If all goes
well it is removed in a few patches, so shouldn't be an eyesore for
long.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: Remove intel_flip_work infrastructure

[Daniel Vetter]

On Wed, Jul 19, 2017 at 02:07:36PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-07-19 13:55:01)
> > This gets rid of all the interactions between the legacy flip code and
> > the modeset code. Yay!
> 
> Including our missed vblank boosting. (That's been dead for a while.)

Hm right, but should be easy to add (and bonus, for every display update,
not just flips) in the intel_sw_fence_wait function. Can you pls point me
at what function I should call to reverse-boost an i915_sw_fence (and only
that)?

Then we could queue up a vblank callback that fires on the next vblank for
any of the CRTC in the update and boosts the i915_sw_fence. If we just
boost the fence (instead of the context or something) it should also be
easy to filter out boosting when the request has completed meanwhile.
-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

Re: [Intel-gfx] [PATCH 9/9] drm/i915: Drop unpin stall in atomic_prepare_commit

[Daniel Vetter]

On Wed, Jul 19, 2017 at 02:09:02PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-07-19 13:55:02)
> > The core already does this in setup_commit(). With this we can also
> > remove the unpin_work_count since it's the last user.
> 
> Also note that the loop only existed for the legacy pageflip support,
> with that removed it becomes entirely redundant.

Yeah, I just wanted to make clear that removing this isn't a bit of code
that we failed to move over to atomic. It's been dead since a while.
-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] Fixes that failed to backport to v4.13-rc1

[Jani Nikula]

Another kernel, another list of failed backports.

The following commits have been marked as Cc: stable or fixing something
in v4.13-rc1 or earlier, but failed to cherry-pick to
drm-intel-fixes. Please see if they are worth backporting, and please do
so if they are.

BR,
Jani.

54d20ed1fff2 ("drm/i915: Fix bad comparison in skl_compute_plane_wm, v2.")

-- 
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 7/9] drm/i915: adjust has_pending_fb_unpin to atomic

[Daniel Vetter]

On Wed, Jul 19, 2017 at 02:06:43PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-07-19 13:55:00)
> > A bit an oversight - the current code did nothing, since only
> > legacy flips used the unpin_work_count and assorted logic.
> > 
> > Cc: Maarten Lankhorst 
> > Cc: Ville Syrjälä 
> > Signed-off-by: Daniel Vetter 
> 
> There's a fence deadlock testcase for this, kms_flip/fence?

Crunching through them, but since I tend to test my stuff all mixed into
one pile I've hit a bug in a different patch series of mine. Given that
we've run without this for a while, not sure it's that critical really.
-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

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit

[Daniel Vetter]

On Wed, Jul 19, 2017 at 02:04:25PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-07-19 13:54:57)
> > Blocking in a worker is ok, 
> 
> but needlessly inefficient,
> 
> > that's what the unbound_wq is for. And it
> > unifies the paths between the blocking and nonblocking commit, giving
> > me just one path where I have to implement the deadlock avoidance
> > trickery in the next patch.
> 
> For reference, I did that the other way by moving it all over to fences.

Yeah the commit message fails to explain this here:

"I first tried to implement the following patch without this rework, but
force-completing i915_sw_fence creates some serious challenges around
properly cleaning things up. So wasn't a feasible short-term approach.
Another approach would be to simple keep track of all pending atomic
commit work items and manually queue them from the reset code. With the
caveat that double-queue in case we race with the i915_sw_fence must be
avoided. Given all that, taking the cost of a double schedule in atomic
for the short-term fix is the best approach, but can be changed in the
future of course."

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

Re: [Intel-gfx] [PATCH 9/9] drm/i915: Drop unpin stall in atomic_prepare_commit

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 13:55:02)
> The core already does this in setup_commit(). With this we can also
> remove the unpin_work_count since it's the last user.

Also note that the loop only existed for the legacy pageflip support,
with that removed it becomes entirely redundant.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: Remove intel_flip_work infrastructure

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 13:55:01)
> This gets rid of all the interactions between the legacy flip code and
> the modeset code. Yay!

Including our missed vblank boosting. (That's been dead for a while.)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/9] drm/i915: adjust has_pending_fb_unpin to atomic

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 13:55:00)
> A bit an oversight - the current code did nothing, since only
> legacy flips used the unpin_work_count and assorted logic.
> 
> Cc: Maarten Lankhorst 
> Cc: Ville Syrjälä 
> Signed-off-by: Daniel Vetter 

There's a fence deadlock testcase for this, kms_flip/fence?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit

[Chris Wilson]

Quoting Daniel Vetter (2017-07-19 13:54:57)
> Blocking in a worker is ok, 

but needlessly inefficient,

> that's what the unbound_wq is for. And it
> unifies the paths between the blocking and nonblocking commit, giving
> me just one path where I have to implement the deadlock avoidance
> trickery in the next patch.

For reference, I did that the other way by moving it all over to fences.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 9/9] drm/i915: Drop unpin stall in atomic_prepare_commit

[Daniel Vetter]

The core already does this in setup_commit(). With this we can also
remove the unpin_work_count since it's the last user.

Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 13 +
 drivers/gpu/drm/i915/intel_drv.h |  2 --
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e52a2adbaaa5..351208b7b1ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11825,18 +11825,7 @@ static int intel_atomic_check(struct drm_device *dev,
 static int intel_atomic_prepare_commit(struct drm_device *dev,
   struct drm_atomic_state *state)
 {
-   struct drm_i915_private *dev_priv = to_i915(dev);
-   struct drm_crtc_state *crtc_state;
-   struct drm_crtc *crtc;
-   int i, ret;
-
-   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
-   if (state->legacy_cursor_update)
-   continue;
-
-   if (atomic_read(_intel_crtc(crtc)->unpin_work_count) >= 2)
-   flush_workqueue(dev_priv->wq);
-   }
+   int ret;
 
ret = mutex_lock_interruptible(>struct_mutex);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9cb7e781e863..96402c06e295 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -798,8 +798,6 @@ struct intel_crtc {
unsigned long long enabled_power_domains;
struct intel_overlay *overlay;
 
-   atomic_t unpin_work_count;
-
/* Display surface base address adjustement for pageflips. Note that on
 * gen4+ this only adjusts up to a tile, offsets within a tile are
 * handled in the hw itself (with the TILEOFF register). */
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 6/9] drm/i915: Rip out legacy page_flip completion/irq handling

[Daniel Vetter]

All these races and things are now solved through the vblank evasion
trick, plus event handling is done using normal vblank even processing
and drm_crtc_arm_vblank_event. We can get rid of all this complexity.

Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_irq.c  | 151 
 drivers/gpu/drm/i915/intel_display.c | 215 ---
 drivers/gpu/drm/i915/intel_drv.h |   3 -
 3 files changed, 22 insertions(+), 347 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f0cb278cee8b..b64c89e0fbf1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1708,18 +1708,6 @@ static void gen9_guc_irq_handler(struct drm_i915_private 
*dev_priv, u32 gt_iir)
}
 }
 
-static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
-enum pipe pipe)
-{
-   bool ret;
-
-   ret = drm_handle_vblank(_priv->drm, pipe);
-   if (ret)
-   intel_finish_page_flip_mmio(dev_priv, pipe);
-
-   return ret;
-}
-
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
u32 iir, u32 pipe_stats[I915_MAX_PIPES])
 {
@@ -1784,12 +1772,8 @@ static void valleyview_pipestat_irq_handler(struct 
drm_i915_private *dev_priv,
enum pipe pipe;
 
for_each_pipe(dev_priv, pipe) {
-   if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
-   intel_pipe_handle_vblank(dev_priv, pipe))
-   intel_check_page_flip(dev_priv, pipe);
-
-   if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
-   intel_finish_page_flip_cs(dev_priv, pipe);
+   if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+   drm_handle_vblank(_priv->drm, pipe);
 
if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
i9xx_pipe_crc_irq_handler(dev_priv, pipe);
@@ -2241,19 +2225,14 @@ static void ilk_display_irq_handler(struct 
drm_i915_private *dev_priv,
DRM_ERROR("Poison interrupt\n");
 
for_each_pipe(dev_priv, pipe) {
-   if (de_iir & DE_PIPE_VBLANK(pipe) &&
-   intel_pipe_handle_vblank(dev_priv, pipe))
-   intel_check_page_flip(dev_priv, pipe);
+   if (de_iir & DE_PIPE_VBLANK(pipe))
+   drm_handle_vblank(_priv->drm, pipe);
 
if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
 
if (de_iir & DE_PIPE_CRC_DONE(pipe))
i9xx_pipe_crc_irq_handler(dev_priv, pipe);
-
-   /* plane/pipes map 1:1 on ilk+ */
-   if (de_iir & DE_PLANE_FLIP_DONE(pipe))
-   intel_finish_page_flip_cs(dev_priv, pipe);
}
 
/* check event from PCH */
@@ -2315,13 +2294,8 @@ static void ivb_display_irq_handler(struct 
drm_i915_private *dev_priv,
intel_opregion_asle_intr(dev_priv);
 
for_each_pipe(dev_priv, pipe) {
-   if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)) &&
-   intel_pipe_handle_vblank(dev_priv, pipe))
-   intel_check_page_flip(dev_priv, pipe);
-
-   /* plane/pipes map 1:1 on ilk+ */
-   if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
-   intel_finish_page_flip_cs(dev_priv, pipe);
+   if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)))
+   drm_handle_vblank(_priv->drm, pipe);
}
 
/* check event from PCH */
@@ -2502,7 +2476,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, 
u32 master_ctl)
}
 
for_each_pipe(dev_priv, pipe) {
-   u32 flip_done, fault_errors;
+   u32 fault_errors;
 
if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
continue;
@@ -2516,18 +2490,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, 
u32 master_ctl)
ret = IRQ_HANDLED;
I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
 
-   if (iir & GEN8_PIPE_VBLANK &&
-   intel_pipe_handle_vblank(dev_priv, pipe))
-   intel_check_page_flip(dev_priv, pipe);
-
-   flip_done = iir;
-   if (INTEL_INFO(dev_priv)->gen >= 9)
-   flip_done &= GEN9_PIPE_PLANE1_FLIP_DONE;
-   else
-   flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
-
-   if (flip_done)
-   intel_finish_page_flip_cs(dev_priv, pipe);
+   if (iir & GEN8_PIPE_VBLANK)
+   drm_handle_vblank(_priv->drm, pipe);
 
  

[Intel-gfx] [PATCH 7/9] drm/i915: adjust has_pending_fb_unpin to atomic

[Daniel Vetter]

A bit an oversight - the current code did nothing, since only
legacy flips used the unpin_work_count and assorted logic.

Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 02620f31374b..343883214113 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4140,21 +4140,22 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
 
 bool intel_has_pending_fb_unpin(struct drm_i915_private *dev_priv)
 {
-   struct intel_crtc *crtc;
-
-   /* Note that we don't need to be called with mode_config.lock here
-* as our list of CRTC objects is static for the lifetime of the
-* device and so cannot disappear as we iterate. Similarly, we can
-* happily treat the predicates as racy, atomic checks as userspace
-* cannot claim and pin a new fb without at least acquring the
-* struct_mutex and so serialising with us.
-*/
-   for_each_intel_crtc(_priv->drm, crtc) {
-   if (atomic_read(>unpin_work_count) == 0)
+   struct drm_crtc *crtc;
+   bool cleanup_done;
+
+   drm_for_each_crtc(crtc, _priv->drm) {
+   struct drm_crtc_commit *commit;
+   spin_lock(>commit_lock);
+   commit = list_first_entry_or_null(>commit_list,
+ struct drm_crtc_commit, 
commit_entry);
+   cleanup_done = commit ?
+   try_wait_for_completion(>cleanup_done) : true;
+   spin_unlock(>commit_lock);
+
+   if (cleanup_done)
continue;
 
-   if (crtc->flip_work)
-   intel_wait_for_vblank(dev_priv, crtc->pipe);
+   drm_crtc_wait_one_vblank(crtc);
 
return true;
}
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/9] drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit

[Daniel Vetter]

Blocking in a worker is ok, that's what the unbound_wq is for. And it
unifies the paths between the blocking and nonblocking commit, giving
me just one path where I have to implement the deadlock avoidance
trickery in the next patch.

Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 010a1f3e000c..5aa7ca1ab592 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12397,6 +12397,8 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
unsigned crtc_vblank_mask = 0;
int i;
 
+   i915_sw_fence_wait(_state->commit_ready);
+
drm_atomic_helper_wait_for_dependencies(state);
 
if (intel_state->modeset)
@@ -12564,10 +12566,7 @@ intel_atomic_commit_ready(struct i915_sw_fence *fence,
 
switch (notify) {
case FENCE_COMPLETE:
-   if (state->base.commit_work.func)
-   queue_work(system_unbound_wq, >base.commit_work);
break;
-
case FENCE_FREE:
{
struct intel_atomic_helper *helper =
@@ -12671,14 +12670,14 @@ static int intel_atomic_commit(struct drm_device *dev,
}
 
drm_atomic_state_get(state);
-   INIT_WORK(>commit_work,
- nonblock ? intel_atomic_commit_work : NULL);
+   INIT_WORK(>commit_work, intel_atomic_commit_work);
 
i915_sw_fence_commit(_state->commit_ready);
-   if (!nonblock) {
-   i915_sw_fence_wait(_state->commit_ready);
+   if (nonblock)
+   queue_work(system_unbound_wq, >commit_work);
+   else
intel_atomic_commit_tail(state);
-   }
+
 
return 0;
 }
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 8/9] drm/i915: Remove intel_flip_work infrastructure

[Daniel Vetter]

This gets rid of all the interactions between the legacy flip code and
the modeset code. Yay!

Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  70 -
 drivers/gpu/drm/i915/i915_drv.c  |   1 -
 drivers/gpu/drm/i915/i915_drv.h  |   4 --
 drivers/gpu/drm/i915/i915_gem.c  |   2 -
 drivers/gpu/drm/i915/intel_display.c | 117 +--
 drivers/gpu/drm/i915/intel_drv.h |  21 +--
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +--
 7 files changed, 3 insertions(+), 220 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ef75c1a6119..c25f42c60d61 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -543,75 +543,6 @@ static int i915_gem_gtt_info(struct seq_file *m, void 
*data)
return 0;
 }
 
-static int i915_gem_pageflip_info(struct seq_file *m, void *data)
-{
-   struct drm_i915_private *dev_priv = node_to_i915(m->private);
-   struct drm_device *dev = _priv->drm;
-   struct intel_crtc *crtc;
-   int ret;
-
-   ret = mutex_lock_interruptible(>struct_mutex);
-   if (ret)
-   return ret;
-
-   for_each_intel_crtc(dev, crtc) {
-   const char pipe = pipe_name(crtc->pipe);
-   const char plane = plane_name(crtc->plane);
-   struct intel_flip_work *work;
-
-   spin_lock_irq(>event_lock);
-   work = crtc->flip_work;
-   if (work == NULL) {
-   seq_printf(m, "No flip due on pipe %c (plane %c)\n",
-  pipe, plane);
-   } else {
-   u32 pending;
-   u32 addr;
-
-   pending = atomic_read(>pending);
-   if (pending) {
-   seq_printf(m, "Flip ioctl preparing on pipe %c 
(plane %c)\n",
-  pipe, plane);
-   } else {
-   seq_printf(m, "Flip pending (waiting for vsync) 
on pipe %c (plane %c)\n",
-  pipe, plane);
-   }
-   if (work->flip_queued_req) {
-   struct intel_engine_cs *engine = 
work->flip_queued_req->engine;
-
-   seq_printf(m, "Flip queued on %s at seqno %x, 
last submitted seqno %x [current breadcrumb %x], completed? %d\n",
-  engine->name,
-  work->flip_queued_req->global_seqno,
-  intel_engine_last_submit(engine),
-  intel_engine_get_seqno(engine),
-  
i915_gem_request_completed(work->flip_queued_req));
-   } else
-   seq_printf(m, "Flip not associated with any 
ring\n");
-   seq_printf(m, "Flip queued on frame %d, (was ready on 
frame %d), now %d\n",
-  work->flip_queued_vblank,
-  work->flip_ready_vblank,
-  intel_crtc_get_vblank_counter(crtc));
-   seq_printf(m, "%d prepares\n", 
atomic_read(>pending));
-
-   if (INTEL_GEN(dev_priv) >= 4)
-   addr = 
I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane)));
-   else
-   addr = I915_READ(DSPADDR(crtc->plane));
-   seq_printf(m, "Current scanout address 0x%08x\n", addr);
-
-   if (work->pending_flip_obj) {
-   seq_printf(m, "New framebuffer address 
0x%08lx\n", (long)work->gtt_offset);
-   seq_printf(m, "MMIO update completed? %d\n",  
addr == work->gtt_offset);
-   }
-   }
-   spin_unlock_irq(>event_lock);
-   }
-
-   mutex_unlock(>struct_mutex);
-
-   return 0;
-}
-
 static int i915_gem_batch_pool_info(struct seq_file *m, void *data)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4854,7 +4785,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_gem_gtt", i915_gem_gtt_info, 0},
{"i915_gem_pin_display", i915_gem_gtt_info, 0, (void *)1},
{"i915_gem_stolen", i915_gem_stolen_list_info },
-   {"i915_gem_pageflip", i915_gem_pageflip_info, 0},
{"i915_gem_request", i915_gem_request_info, 0},
{"i915_gem_seqno", i915_gem_seqno_info, 0},
{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
diff --git a/drivers/gpu/drm/i915/i915_drv.c 

[Intel-gfx] [PATCH 5/9] drm/i915: More surgically unbreak the modeset vs reset deadlock

[Daniel Vetter]

There's no reason to entirely wedge the gpu, for the minimal deadlock
bugfix we only need to unbreak/decouple the atomic commit from the gpu
reset. The simplest wait to fix that is by replacing the
unconditional fence wait a the top of commit_tail by a wait which
completes either when the fences are done (normal case, or when a
reset doesn't need to touch the display state). Or when the gpu reset
needs to force-unblock all pending modeset states.

Note that in both cases TDR itself keeps working, so from a userspace
pov this trickery isn't observable. Users themselvs might spot a short
glitch while the rendering is catching up again, but that's still
better than pre-TDR where we've thrown away all the rendering,
including innocent batches. Also, this fixes the regression TDR
introduced of making gpu resets deadlock-prone when we do need to
touch the display.

One thing I noticed is that gpu_error.flags seems to use both our own
wait-queue in gpu_error.wait_queue, and the generic wait_on_bit
facilities. Not entirely sure why this inconsistency exists, I just
picked one style.

A possible future avenue could be to insert the gpu reset in-between
ongoing modeset changes, which would avoid the momentary glitch. But
that's a lot more work to implement in the atomic commit machinery,
and given that we only need this for pre-g4x hw, of questionable
utility just for the sake of polishing gpu reset even more on those
old boxes. It might be useful for other features though.

v2: Rebase onto 4.13 with a s/wait_queue_t/struct wait_queue_entry/.

Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 35 ++-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 07e98b07c5bc..369968539b40 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1564,6 +1564,7 @@ struct i915_gpu_error {
unsigned long flags;
 #define I915_RESET_BACKOFF 0
 #define I915_RESET_HANDOFF 1
+#define I915_RESET_MODESET 2
 #define I915_WEDGED(BITS_PER_LONG - 1)
 #define I915_RESET_ENGINE  (I915_WEDGED - I915_NUM_ENGINES)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5aa7ca1ab592..4762f158032d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3471,10 +3471,9 @@ void intel_prepare_reset(struct drm_i915_private 
*dev_priv)
!gpu_reset_clobbers_display(dev_priv))
return;
 
-   /* We have a modeset vs reset deadlock, defensively unbreak it.
-*
-* FIXME: We can do a _lot_ better, this is just a first iteration.*/
-   i915_gem_set_wedged(dev_priv);
+   /* We have a modeset vs reset deadlock, defensively unbreak it. */
+   set_bit(I915_RESET_MODESET, _priv->gpu_error.flags);
+   wake_up_all(_priv->gpu_error.wait_queue);
 
/*
 * Need mode_config.mutex so that we don't
@@ -3569,6 +3568,8 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
drm_modeset_drop_locks(ctx);
drm_modeset_acquire_fini(ctx);
mutex_unlock(>mode_config.mutex);
+
+   clear_bit(I915_RESET_MODESET, _priv->gpu_error.flags);
 }
 
 static bool abort_flip_on_reset(struct intel_crtc *crtc)
@@ -12384,6 +12385,30 @@ static void 
intel_atomic_helper_free_state_worker(struct work_struct *work)
intel_atomic_helper_free_state(dev_priv);
 }
 
+static void intel_atomic_commit_fence_wait(struct intel_atomic_state 
*intel_state)
+{
+   struct wait_queue_entry wait_fence, wait_reset;
+   struct drm_i915_private *dev_priv = to_i915(intel_state->base.dev);
+
+   init_wait_entry(_fence, 0);
+   init_wait_entry(_reset, 0);
+   for (;;) {
+   prepare_to_wait(_state->commit_ready.wait,
+   _fence, TASK_UNINTERRUPTIBLE);
+   prepare_to_wait(_priv->gpu_error.wait_queue,
+   _reset, TASK_UNINTERRUPTIBLE);
+
+
+   if (i915_sw_fence_done(_state->commit_ready)
+   || (dev_priv->gpu_error.flags & I915_RESET_MODESET))
+   break;
+
+   schedule();
+   }
+   finish_wait(_state->commit_ready.wait, _fence);
+   finish_wait(_priv->gpu_error.wait_queue, _reset);
+}
+
 static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 {
struct drm_device *dev = state->dev;
@@ -12397,7 +12422,7 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
unsigned crtc_vblank_mask = 0;
int i;
 
-   i915_sw_fence_wait(_state->commit_ready);
+   intel_atomic_commit_fence_wait(intel_state);
 

[Intel-gfx] [PATCH 1/9] drm/i915: Nuke legacy flip queueing code

[Daniel Vetter]

Just a very minimal patch to nuke that code. Lots of the flip
interrupt handling stuff is still around.

Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.h  |   5 -
 drivers/gpu/drm/i915/intel_display.c | 656 ---
 2 files changed, 661 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f66c78d3a0a2..07e98b07c5bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -715,11 +715,6 @@ struct drm_i915_display_funcs {
void (*fdi_link_train)(struct intel_crtc *crtc,
   const struct intel_crtc_state *crtc_state);
void (*init_clock_gating)(struct drm_i915_private *dev_priv);
-   int (*queue_flip)(struct drm_device *dev, struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj,
- struct drm_i915_gem_request *req,
- uint32_t flags);
void (*hpd_irq_setup)(struct drm_i915_private *dev_priv);
/* clock updates for mode set */
/* cursor updates */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 74b0ea1badc3..3349ca947173 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2664,20 +2664,6 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
return false;
 }
 
-/* Update plane->state->fb to match plane->fb after driver-internal updates */
-static void
-update_state_fb(struct drm_plane *plane)
-{
-   if (plane->fb == plane->state->fb)
-   return;
-
-   if (plane->state->fb)
-   drm_framebuffer_unreference(plane->state->fb);
-   plane->state->fb = plane->fb;
-   if (plane->state->fb)
-   drm_framebuffer_reference(plane->state->fb);
-}
-
 static void
 intel_set_plane_visible(struct intel_crtc_state *crtc_state,
struct intel_plane_state *plane_state,
@@ -10163,35 +10149,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
kfree(intel_crtc);
 }
 
-static void intel_unpin_work_fn(struct work_struct *__work)
-{
-   struct intel_flip_work *work =
-   container_of(__work, struct intel_flip_work, unpin_work);
-   struct intel_crtc *crtc = to_intel_crtc(work->crtc);
-   struct drm_device *dev = crtc->base.dev;
-   struct drm_plane *primary = crtc->base.primary;
-
-   if (is_mmio_work(work))
-   flush_work(>mmio_work);
-
-   mutex_lock(>struct_mutex);
-   intel_unpin_fb_vma(work->old_vma);
-   i915_gem_object_put(work->pending_flip_obj);
-   mutex_unlock(>struct_mutex);
-
-   i915_gem_request_put(work->flip_queued_req);
-
-   intel_frontbuffer_flip_complete(to_i915(dev),
-   
to_intel_plane(primary)->frontbuffer_bit);
-   intel_fbc_post_update(crtc);
-   drm_framebuffer_unreference(work->old_fb);
-
-   BUG_ON(atomic_read(>unpin_work_count) == 0);
-   atomic_dec(>unpin_work_count);
-
-   kfree(work);
-}
-
 /* Is 'a' after or equal to 'b'? */
 static bool g4x_flip_count_after_eq(u32 a, u32 b)
 {
@@ -10336,346 +10293,6 @@ static inline void intel_mark_page_flip_active(struct 
intel_crtc *crtc,
atomic_set(>pending, 1);
 }
 
-static int intel_gen2_queue_flip(struct drm_device *dev,
-struct drm_crtc *crtc,
-struct drm_framebuffer *fb,
-struct drm_i915_gem_object *obj,
-struct drm_i915_gem_request *req,
-uint32_t flags)
-{
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   u32 flip_mask, *cs;
-
-   cs = intel_ring_begin(req, 6);
-   if (IS_ERR(cs))
-   return PTR_ERR(cs);
-
-   /* Can't queue multiple flips, so wait for the previous
-* one to finish before executing the next.
-*/
-   if (intel_crtc->plane)
-   flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
-   else
-   flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
-   *cs++ = MI_WAIT_FOR_EVENT | flip_mask;
-   *cs++ = MI_NOOP;
-   *cs++ = MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc->plane);
-   *cs++ = fb->pitches[0];
-   *cs++ = intel_crtc->flip_work->gtt_offset;
-   *cs++ = 0; /* aux display base address, unused */
-
-   return 0;
-}
-
-static int intel_gen3_queue_flip(struct drm_device *dev,
-struct drm_crtc *crtc,
-struct drm_framebuffer *fb,
-struct drm_i915_gem_object *obj,
-struct drm_i915_gem_request *req,
-

[Intel-gfx] [PATCH 2/9] drm/i915: Unbreak gpu reset vs. modeset locking

[Daniel Vetter]

Taking the modeset locks unconditionally isn't the greatest idea,
because atm that part is still broken and times out (and then atomic
keels over). And there's really no reason to do so, the old code
didn't do that either.

To make the patch a bit simpler let's also nuke 2 cases that are only
around for the old mmioflip paths. Atomic nonblocking workers will not
die (minus bugs) when a gpu reset happens.

And of course this doesn't fix any of the gpu reset vs. modeset
deadlock fun, but it at least stop modern CI machines from keeling
over all over the place for no reason at all.

And we still have the explicit testcases to run the fake gpu reset, so
coverage isn't that much worse.

v2: Split out additional changes on top, restrict this to purely reducing
the critical section of modeset locks.

v2: Review from Maarten
- update comments
- don't oops when state is NULL in intel_finish_reset, but try to at
  least still drop locks properly. The hw is going to be toast anyway.

Fixes: 739748939974 ("drm/i915: Fix modeset handling during gpu reset, v5.")
Cc: Maarten Lankhorst 
Reviewed-by: Maarten Lankhorst 
Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 60 +++-
 1 file changed, 18 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3349ca947173..9ffa1566 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3413,26 +3413,6 @@ static void intel_complete_page_flips(struct 
drm_i915_private *dev_priv)
intel_finish_page_flip_cs(dev_priv, crtc->pipe);
 }
 
-static void intel_update_primary_planes(struct drm_device *dev)
-{
-   struct drm_crtc *crtc;
-
-   for_each_crtc(dev, crtc) {
-   struct intel_plane *plane = to_intel_plane(crtc->primary);
-   struct intel_plane_state *plane_state =
-   to_intel_plane_state(plane->base.state);
-
-   if (plane_state->base.visible) {
-   trace_intel_update_plane(>base,
-to_intel_crtc(crtc));
-
-   plane->update_plane(plane,
-   to_intel_crtc_state(crtc->state),
-   plane_state);
-   }
-   }
-}
-
 static int
 __intel_display_resume(struct drm_device *dev,
   struct drm_atomic_state *state,
@@ -3485,6 +3465,12 @@ void intel_prepare_reset(struct drm_i915_private 
*dev_priv)
struct drm_atomic_state *state;
int ret;
 
+
+   /* reset doesn't touch the display */
+   if (!i915.force_reset_modeset_test &&
+   !gpu_reset_clobbers_display(dev_priv))
+   return;
+
/*
 * Need mode_config.mutex so that we don't
 * trample ongoing ->detect() and whatnot.
@@ -3498,12 +3484,6 @@ void intel_prepare_reset(struct drm_i915_private 
*dev_priv)
 
drm_modeset_backoff(ctx);
}
-
-   /* reset doesn't touch the display, but flips might get nuked anyway, */
-   if (!i915.force_reset_modeset_test &&
-   !gpu_reset_clobbers_display(dev_priv))
-   return;
-
/*
 * Disabling the crtcs gracefully seems nicer. Also the
 * g33 docs say we should at least disable all the planes.
@@ -3533,6 +3513,14 @@ void intel_finish_reset(struct drm_i915_private 
*dev_priv)
struct drm_atomic_state *state = dev_priv->modeset_restore_state;
int ret;
 
+   /* reset doesn't touch the display */
+   if (!i915.force_reset_modeset_test &&
+   !gpu_reset_clobbers_display(dev_priv))
+   return;
+
+   if (!state)
+   goto unlock;
+
/*
 * Flips in the rings will be nuked by the reset,
 * so complete all pending flips so that user space
@@ -3544,22 +3532,10 @@ void intel_finish_reset(struct drm_i915_private 
*dev_priv)
 
/* reset doesn't touch the display */
if (!gpu_reset_clobbers_display(dev_priv)) {
-   if (!state) {
-   /*
-* Flips in the rings have been nuked by the reset,
-* so update the base address of all primary
-* planes to the the last fb to make sure we're
-* showing the correct fb after a reset.
-*
-* FIXME: Atomic will make this obsolete since we won't 
schedule
-* CS-based flips (which might get lost in gpu resets) 
any more.
-*/
-   intel_update_primary_planes(dev);
-   } else {
-   ret = __intel_display_resume(dev, 

[Intel-gfx] [PATCH 3/9] drm/i915: Avoid the gpu reset vs. modeset deadlock

[Daniel Vetter]

... using the biggest hammer we have. This is essentially a weaponized
version of the timeout-based wedging Chris added in

commit 36703e79a982c8ce5a8e43833291f2719e92d0d1
Author: Chris Wilson 
Date:   Thu Jun 22 11:56:25 2017 +0100

drm/i915: Break modeset deadlocks on reset

Because defense-in-depth is good it's good to still have both. Also
note that with the locking change we can now restrict this a lot (old
gpus and special testing only), so this doesn't kill the TDR benefits
on at least anything remotely modern.

And futuremore with a few tricks it should be possible to make a much
more educated guess about whether an atomic commit is stuck waiting on
the gpu (atomic_t counting the pending i915_sw_fence used by the
atomic modeset code should do it), so we can improve this.

But for now just start with something that is guaranteed to recover
faster, for much better CI througput.

This defacto reverts TDR on these platforms, but there's not really a
single commit to specify as the sole offender.

Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for 
request completion")
Fixes: 221fe7994554 ("drm/i915: Perform a direct reset of the GPU from the 
waiter")
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9ffa1566..010a1f3e000c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3471,6 +3471,11 @@ void intel_prepare_reset(struct drm_i915_private 
*dev_priv)
!gpu_reset_clobbers_display(dev_priv))
return;
 
+   /* We have a modeset vs reset deadlock, defensively unbreak it.
+*
+* FIXME: We can do a _lot_ better, this is just a first iteration.*/
+   i915_gem_set_wedged(dev_priv);
+
/*
 * Need mode_config.mutex so that we don't
 * trample ongoing ->detect() and whatnot.
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 0/9] gpu reset vs modeset fix, plus page_flip removal

[Daniel Vetter]

Hi all,

This fixes the dreaded gpu reset vs. modeset deadlocks. And since the defunct
legacy page_flip code is the reason for a bunch of special cases I did remove
that too, on Maarten's request.

Please review, this is currently blocking extended CI runs on our haswell farm
rather badly. The critical patches are up to patch 5.

Compared to last time around I've dropped two patches of dubious benefit, they
broke gen3/4 reset a bit and aren't really needed.

Thanks,
Daniel

Daniel Vetter (9):
  drm/i915: Nuke legacy flip queueing code
  drm/i915: Unbreak gpu reset vs. modeset locking
  drm/i915: Avoid the gpu reset vs. modeset deadlock
  drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit
  drm/i915: More surgically unbreak the modeset vs reset deadlock
  drm/i915: Rip out legacy page_flip completion/irq handling
  drm/i915: adjust has_pending_fb_unpin to atomic
  drm/i915: Remove intel_flip_work infrastructure
  drm/i915: Drop unpin stall in atomic_prepare_commit

 drivers/gpu/drm/i915/i915_debugfs.c  |   70 ---
 drivers/gpu/drm/i915/i915_drv.c  |1 -
 drivers/gpu/drm/i915/i915_drv.h  |   10 +-
 drivers/gpu/drm/i915/i915_gem.c  |2 -
 drivers/gpu/drm/i915/i915_irq.c  |  151 +
 drivers/gpu/drm/i915/intel_display.c | 1129 +++---
 drivers/gpu/drm/i915/intel_drv.h |   26 +-
 drivers/gpu/drm/i915/intel_sprite.c  |8 +-
 8 files changed, 94 insertions(+), 1303 deletions(-)

-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v2 2/2] chamelium: Add support for VGA frame comparison testing

[Paul Kocialkowski]

On Mon, 2017-07-17 at 13:35 -0400, Lyude Paul wrote:
> Just one change for this patch below
> 
> On Wed, 2017-07-12 at 17:57 +0300, Paul Kocialkowski wrote:
> > This adds support for VGA frame comparison testing with the
> > reference
> > generated from cairo. The retrieved frame from the chamelium is
> > first
> > cropped, as it contains the blanking intervals, through a dedicated
> > helper. Another helper function asserts that the analogue frame
> > matches or dump it to png if not.
> > 
> > Signed-off-by: Paul Kocialkowski 
> > ---
> >  lib/igt_chamelium.c | 164
> > ++--
> >  lib/igt_chamelium.h |   7 ++-
> >  lib/igt_frame.c |   6 +-
> >  tests/chamelium.c   |  57 ++
> >  4 files changed, 225 insertions(+), 9 deletions(-)
> > 
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index df49936b..8701192e 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -938,6 +938,8 @@ static cairo_surface_t
> > *convert_frame_dump_argb32(const struct chamelium_frame_d
> > int w = dump->width, h = dump->height;
> > uint32_t *bits_bgr = (uint32_t *) dump->bgr;
> > unsigned char *bits_argb;
> > +   unsigned char *bits_target;
> > +   int size;
> >  
> > image_bgr = pixman_image_create_bits(
> > PIXMAN_b8g8r8, w, h, bits_bgr,
> > @@ -947,9 +949,13 @@ static cairo_surface_t
> > *convert_frame_dump_argb32(const struct chamelium_frame_d
> >  
> > bits_argb = (unsigned char *)
> > pixman_image_get_data(image_argb);
> >  
> > -   dump_surface = cairo_image_surface_create_for_data(
> > -   bits_argb, CAIRO_FORMAT_ARGB32, w, h,
> > -   PIXMAN_FORMAT_BPP(PIXMAN_x8r8g8b8) / 8 * w);
> > +   dump_surface = cairo_image_surface_create(
> > +   CAIRO_FORMAT_ARGB32, w, h);
> > +
> > +   bits_target = cairo_image_surface_get_data(dump_surface);
> > +   size = cairo_image_surface_get_stride(dump_surface) * h;
> > +   memcpy(bits_target, bits_argb, size);
> > +   cairo_surface_mark_dirty(dump_surface);
> >  
> > pixman_image_unref(image_argb);
> >  
> > @@ -1055,6 +1061,154 @@ void chamelium_assert_crc_eq_or_dump(struct
> > chamelium *chamelium,
> >  }
> >  
> >  /**
> > + * chamelium_assert_analogue_frame_match_or_dump:
> > + * @chamelium: The chamelium instance the frame dump belongs to
> > + * @frame: The chamelium frame dump to match
> > + * @fb: pointer to an #igt_fb structure
> > + *
> > + * Asserts that the provided captured frame matches the reference
> > frame from
> > + * the framebuffer. If they do not, this saves the reference and
> > captured frames
> > + * to a png file.
> > + */
> > +void chamelium_assert_analogue_frame_match_or_dump(struct chamelium
> > *chamelium,
> > +  struct
> > chamelium_port *port,
> > +  const struct
> > chamelium_frame_dump *frame,
> > +  struct igt_fb
> > *fb)
> > +{
> > +   cairo_surface_t *reference;
> > +   cairo_surface_t *capture;
> > +   igt_crc_t *reference_crc;
> > +   igt_crc_t *capture_crc;
> > +   char *reference_suffix;
> > +   char *capture_suffix;
> > +   bool match;
> > +
> > +   /* Grab the reference frame from framebuffer */
> > +   reference = igt_get_cairo_surface(chamelium->drm_fd, fb);
> > +
> > +   /* Grab the captured frame from chamelium */
> > +   capture = convert_frame_dump_argb32(frame);
> > +
> > +   match = igt_check_analogue_frame_match(reference, capture);
> > +   if (!match && igt_frame_dump_is_enabled()) {
> > +   reference_crc =
> > chamelium_calculate_fb_crc(chamelium-
> > > drm_fd,
> > 
> > +  fb);
> > +   capture_crc = chamelium_get_crc_for_area(chamelium,
> > port, 0, 0,
> > +0, 0);
> > +
> > +   reference_suffix =
> > igt_crc_to_string_extended(reference_crc,
> > + '-',
> > 2);
> > +   capture_suffix =
> > igt_crc_to_string_extended(capture_crc, '-',
> > +   2);
> > +
> > +   /* Write reference and capture frames to png */
> > +   igt_write_compared_frames_to_png(reference, capture,
> > +reference_suffix,
> > +capture_suffix);
> > +
> > +   free(reference_suffix);
> > +   free(capture_suffix);
> > +   }
> > +
> > +   cairo_surface_destroy(capture);
> > +
> > +   igt_assert(match);
> > +}
> > +
> > +
> > +/**
> > + * chamelium_analogue_frame_crop:
> > + * @chamelium: The Chamelium instance to 

Re: [Intel-gfx] [PATCH v2] drm/i915: Drain the device workqueue on unload

[Chris Wilson]

Quoting Mika Kuoppala (2017-07-19 12:51:04)
> Chris Wilson  writes:
> 
> > Quoting Mika Kuoppala (2017-07-19 12:18:47)
> >> Chris Wilson  writes:
> >> 
> >> > Workers on the i915->wq may rearm themselves so for completeness we need
> >> > to replace our flush_workqueue() with a call to drain_workqueue() before
> >> > unloading the device.
> >> >
> >> > v2: Reinforce the drain_workqueue with an preceeding rcu_barrier() as a
> >> > few of the tasks that need to be drained may first be armed by RCU.
> >> >
> >> > References: https://bugs.freedesktop.org/show_bug.cgi?id=101627
> >> > Signed-off-by: Chris Wilson 
> >> > Cc: Matthew Auld 
> >> > Cc: Mika Kuoppala 
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_drv.c  |  6 ++
> >> >  drivers/gpu/drm/i915/i915_drv.h  | 20 
> >> > 
> >> >  drivers/gpu/drm/i915/selftests/mock_gem_device.c |  2 +-
> >> >  3 files changed, 23 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> >> > b/drivers/gpu/drm/i915/i915_drv.c
> >> > index 4b62fd012877..41c5b11a7c8f 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.c
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> > @@ -596,7 +596,8 @@ static const struct vga_switcheroo_client_ops 
> >> > i915_switcheroo_ops = {
> >> >  
> >> >  static void i915_gem_fini(struct drm_i915_private *dev_priv)
> >> >  {
> >> > - flush_workqueue(dev_priv->wq);
> >> > + /* Flush any outstanding unpin_work. */
> >> > + i915_gem_drain_workqueue(dev_priv);
> >> >  
> >> >   mutex_lock(_priv->drm.struct_mutex);
> >> >   intel_uc_fini_hw(dev_priv);
> >> > @@ -1409,9 +1410,6 @@ void i915_driver_unload(struct drm_device *dev)
> >> >   cancel_delayed_work_sync(_priv->gpu_error.hangcheck_work);
> >> >   i915_reset_error_state(dev_priv);
> >> >  
> >> > - /* Flush any outstanding unpin_work. */
> >> > - drain_workqueue(dev_priv->wq);
> >> > -
> >> >   i915_gem_fini(dev_priv);
> >> >   intel_uc_fini_fw(dev_priv);
> >> >   intel_fbc_cleanup_cfb(dev_priv);
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> >> > b/drivers/gpu/drm/i915/i915_drv.h
> >> > index 667fb5c44483..e9a4b96dc775 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.h
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> > @@ -3300,6 +3300,26 @@ static inline void 
> >> > i915_gem_drain_freed_objects(struct drm_i915_private *i915)
> >> >   } while (flush_work(>mm.free_work));
> >> >  }
> >> >  
> >> > +static inline void i915_gem_drain_workqueue(struct drm_i915_private 
> >> > *i915)
> >> > +{
> >> > + /*
> >> > +  * Similar to objects above (see i915_gem_drain_freed-objects), in
> >> > +  * general we have workers that are armed by RCU and then rearm
> >> > +  * themselves in their callbacks. To be paranoid, we need to
> >> > +  * drain the workqueue a second time after waiting for the RCU
> >> > +  * grace period so that we catch work queued via RCU from the first
> >> > +  * pass. As neither drain_workqueue() nor flush_workqueue() report
> >> > +  * a result, we make an assumption that we only don't require more
> >> > +  * than 2 passes to catch all recursive RCU delayed work.
> >> > +  *
> >> > +  */
> >> > + int pass = 2;
> >> > + do {
> >> > + rcu_barrier();
> >> > + drain_workqueue(i915->wq);
> >> 
> >> I am fine with the paranoia, and it covers the case below. Still if we do:
> >> 
> >> drain_workqueue();
> >> rcu_barrier();
> >> 
> >> With drawining in progress, only chain queuing is allowed. I understand
> >> this so that when it returns, all the ctx pointers are now unreferenced
> >> but not freed.
> >> 
> >> Thus the rcu_barrier() after it cleans the trash and we are good to
> >> be unloaded. With one pass.
> >> 
> >> I guess it comes to how to understand the comment, so could you
> >> elaborate the 'we have workers that are armed by RCU and then rearm
> >> themselves'?. As from drain_workqueue desc, this should be covered.
> >
> > I'm considering that they may be rearmed via RCU in the general case,
> > e.g. context close frees an object and so goes onto an RCU list that
> > once processed kicks off a new worker and so requires another round of
> > drain_workqueue. We are in module unload so a few extra delays to belts
> > and braces are ok until somebody notices it takes a few minutes to run a
> > reload test ;)
> 
> Ok. Patch is
> Reviewed-by: Mika Kuoppala 

Thanks, I'm optimistic this will silence the bug, so marking it as
resolved. Pushed,
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/selftests: Attach a stub pm_domain

[Chris Wilson]

Quoting Matthew Auld (2017-07-19 11:51:51)
> On 18 July 2017 at 18:30, Chris Wilson  wrote:
> > Supply a pm_domain and its ops for our mock GEM device so that
> > device runtime pm doesn't complain even though we only want to mark it
> > permanently active!
> >
> > Signed-off-by: Chris Wilson 
> Fixes it for me, so:
> 
> Tested-by: Matthew Auld 
> Reviewed-by: Matthew Auld 

Took far too long to find a solution! Thanks, pushed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gvt: Dma-buf support for GVT-g

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Dma-buf support for GVT-g
URL   : https://patchwork.freedesktop.org/series/27572/
State : success

== Summary ==

Series 27572v1 drm/i915/gvt: Dma-buf support for GVT-g
https://patchwork.freedesktop.org/api/1.0/series/27572/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-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:453s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:438s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:356s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:545s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:512s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:496s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:485s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:606s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:417s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:417s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:496s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:576s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:590s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:560s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:463s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:582s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:472s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:474s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:433s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:474s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:546s
fi-snb-2600  total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  
time:404s

541e75cb2ac9f85388e9e6f228d98f266506eebd drm-tip: 2017y-07m-19d-10h-10m-36s UTC 
integration manifest
2cd6b00 drm/i915: Introduce GEM proxy
6ca8e14 vfio: ABI for mdev display dma-buf operation
fd3a206 drm/i915/gvt: add opregion support
11cb619 drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support
38501de drm: Introduce RGB 64-bit 16:16:16:16 float format
879bb65 drm/i915/gvt: Add framebuffer decoder support

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5233/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 00/14] i915 PMU and engine busy stats

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-18 15:36:04)
> From: Tvrtko Ursulin 
> 
> Rough sketch of the idea I mentioned a few times to various people - merging
> the engine busyness tracking with Chris i915 PMU RFC.
> 
> First patch is the actual PMU RFC by Chris. It is followed by some cleanup
> patches, then come a few improvements, cheap execlists engine busyness 
> tracking,
> debugfs view for the same, and finally the i915 PMU is extended to use this
> instead of timer based mmio sampling.
> 
> This makes it cheaper and also more accurate since engine busyness is not
> derived via sampling.
> 
> But I haven't figure out the perf API yet. For example is it possible to 
> access
> our events in an usable fashion via perf top/stat or something? Do we want to
> make the events discoverable as I did (patch 8).

In my dreams I have gpu activity in the same perf timechart as gpu
activity. But that can be mostly by the request tracepoints, but still
overlaying cpu/gpu activity is desirable and more importantly we want to
coordinate with nouveau/amdgpu so that such interfaces are as agnostic
as possible. There are definitely a bunch of global features in common
for all (engine enumeration & activity, mempool enumeration, size &
activty, power usage?). But the key question is how do we build for the
future? Split the event id range into common/driver?

> I could not find much (any?) kernel API level documentation for perf.

There isn't much indeed. Given that we now have a second pair of eyes go
over the sampling and improve its interaction with i915, we should start
getting PeterZ involved to check the interaction with perf.
 
> Btw patch series actually works since intel-gpu-overlay can use these events
> when they are available.
> 
> Chris Wilson (1):
>   RFC drm/i915: Expose a PMU interface for perf queries

One thing I would like is for any future interface (including this
engine/class/event id) to use the engine class/instance mapping.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: More stolen quirking

[Daniel Vetter]

On Wed, Jul 19, 2017 at 01:05:45PM +0300, Martin Peres wrote:
> On 19/07/17 13:00, Daniel Vetter wrote:
> > I've found a bios with an off-by-one at the other end. There's a pnp
> > reservation for 0xc540-0xc7fe and we want stolen in 0xc600
> > through 0xc800.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99872
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98683
> > Cc: Martin Peres 
> > Signed-off-by: Daniel Vetter 
> 
> Looks good, and it will reduce the noise in CI. Thanks!

Well, since this happens always it simply reduced coverage on that
machine. Not being sensitive for dmesg noise for the module reload
testcase makes that one rather useless.
> 
> Reviewed-by: Martin Peres 

Thanks for your review, merged.
-Daniel
> 
> > ---
> >   drivers/gpu/drm/i915/i915_gem_stolen.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index a817b3e0b17e..c11c915382e7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -254,9 +254,10 @@ static dma_addr_t i915_stolen_to_dma(struct 
> > drm_i915_private *dev_priv)
> >  * This is a BIOS w/a: Some BIOS wrap stolen in the root
> >  * PCI bus, but have an off-by-one error. Hence retry the
> >  * reservation starting from 1 instead of 0.
> > +* There's also BIOS with off-by-one on the other end.
> >  */
> > r = devm_request_mem_region(dev_priv->drm.dev, base + 1,
> > -   ggtt->stolen_size - 1,
> > +   ggtt->stolen_size - 2,
> > "Graphics Stolen Memory");
> > /*
> >  * GEN3 firmware likes to smash pci bridges into the stolen
> > 

-- 
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: Fix bad comparison in skl_compute_plane_wm.

[Maarten Lankhorst]

Op 17-07-17 om 14:06 schreef Mahesh Kumar:
> Reviewed-by: Mahesh Kumar 
Thanks, fix pushed. :)
> On Monday 17 July 2017 04:43 PM, Maarten Lankhorst wrote:
>> ddb_allocation && ddb_allocation / blocks_per_line >= 1 is the same
>> as ddb_allocation >= blocks_per_line, so use the latter to simplify
>> this.
>>
>> This fixes the following compiler warning:
>>
>> drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a
>> boolean expression with an integer other than 0 or 1.
>>
>> Signed-off-by: Maarten Lankhorst 
>> Fixes: d555cb5827d6 ("drm/i915/skl+: use linetime latency if ddb size is not 
>> available")
>> Cc: "Mahesh Kumar" 
>> Reported-by: David Binderman 
>> Cc: David Binderman 
>> Cc:  # v4.13-rc1+
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index 78b9acfc64c0..b9b3d8d45016 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4681,8 +4681,8 @@ static int skl_compute_plane_wm(const struct 
>> drm_i915_private *dev_priv,
>>   if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) &&
>>   (plane_bytes_per_line / 512 < 1))
>>   selected_result = method2;
>> -else if ((ddb_allocation && ddb_allocation /
>> -fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1)
>> +else if (ddb_allocation >=
>> + fixed_16_16_to_u32_round_up(plane_blocks_per_line))
>>   selected_result = min_fixed_16_16(method1, method2);
>>   else if (latency >= linetime_us)
>>   selected_result = min_fixed_16_16(method1, method2);
>

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v10] vfio: ABI for mdev display dma-buf operation

[Kirti Wankhede]

On 7/19/2017 11:55 AM, Gerd Hoffmann wrote:
> On Wed, 2017-07-19 at 00:16 +, Zhang, Tina wrote:
>>> -Original Message-
>>> From: Gerd Hoffmann [mailto:kra...@redhat.com]
>>> Sent: Monday, July 17, 2017 7:03 PM
>>> To: Kirti Wankhede ; Zhang, Tina
>>> ; Tian, Kevin ; linux-
>>> ker...@vger.kernel.org; intel-gfx@lists.freedesktop.org;
>>> alex.william...@redhat.com; zhen...@linux.intel.com; chris@chris-
>>> wilson.co.uk; Lv, Zhiyuan ; intel-gvt-
>>> d...@lists.freedesktop.org; Wang, Zhi A 
>>> Subject: Re: [PATCH v10] vfio: ABI for mdev display dma-buf
>>> operation
>>>
>>>   Hi,
>>>
 No need of flag here. If vGPU driver is not loaded in the guest,
 there
 is no surface being managed by vGPU, in that case this size will
 be
 zero.
>>>
>>> Ok, we certainly have the same situation with intel.  When the
>>> guest driver is not
>>> loaded (yet) there is no valid surface.
>>>
>>> We should cleanly define what the ioctl should do in that case, so
>>> all drivers
>>> behave the same way.
>>>
>>> I'd suggest that all fields defining the surface (drm_format,
>>> width, height, stride,
>>> size) should be set to zero in that case.
>>
>> Yeah, it's reasonable. How about the return value? Currently, the
>> ioctl also returns "-ENODEV" in that situation.
> 
> I think it should not return an error.  Querying the plane parameters
> worked fine.
> 

Sounds good to me too.

Thanks,
Kirti
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Drain the device workqueue on unload

[Mika Kuoppala]

Chris Wilson  writes:

> Quoting Mika Kuoppala (2017-07-19 12:18:47)
>> Chris Wilson  writes:
>> 
>> > Workers on the i915->wq may rearm themselves so for completeness we need
>> > to replace our flush_workqueue() with a call to drain_workqueue() before
>> > unloading the device.
>> >
>> > v2: Reinforce the drain_workqueue with an preceeding rcu_barrier() as a
>> > few of the tasks that need to be drained may first be armed by RCU.
>> >
>> > References: https://bugs.freedesktop.org/show_bug.cgi?id=101627
>> > Signed-off-by: Chris Wilson 
>> > Cc: Matthew Auld 
>> > Cc: Mika Kuoppala 
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.c  |  6 ++
>> >  drivers/gpu/drm/i915/i915_drv.h  | 20 
>> >  drivers/gpu/drm/i915/selftests/mock_gem_device.c |  2 +-
>> >  3 files changed, 23 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>> > b/drivers/gpu/drm/i915/i915_drv.c
>> > index 4b62fd012877..41c5b11a7c8f 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > @@ -596,7 +596,8 @@ static const struct vga_switcheroo_client_ops 
>> > i915_switcheroo_ops = {
>> >  
>> >  static void i915_gem_fini(struct drm_i915_private *dev_priv)
>> >  {
>> > - flush_workqueue(dev_priv->wq);
>> > + /* Flush any outstanding unpin_work. */
>> > + i915_gem_drain_workqueue(dev_priv);
>> >  
>> >   mutex_lock(_priv->drm.struct_mutex);
>> >   intel_uc_fini_hw(dev_priv);
>> > @@ -1409,9 +1410,6 @@ void i915_driver_unload(struct drm_device *dev)
>> >   cancel_delayed_work_sync(_priv->gpu_error.hangcheck_work);
>> >   i915_reset_error_state(dev_priv);
>> >  
>> > - /* Flush any outstanding unpin_work. */
>> > - drain_workqueue(dev_priv->wq);
>> > -
>> >   i915_gem_fini(dev_priv);
>> >   intel_uc_fini_fw(dev_priv);
>> >   intel_fbc_cleanup_cfb(dev_priv);
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index 667fb5c44483..e9a4b96dc775 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -3300,6 +3300,26 @@ static inline void 
>> > i915_gem_drain_freed_objects(struct drm_i915_private *i915)
>> >   } while (flush_work(>mm.free_work));
>> >  }
>> >  
>> > +static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915)
>> > +{
>> > + /*
>> > +  * Similar to objects above (see i915_gem_drain_freed-objects), in
>> > +  * general we have workers that are armed by RCU and then rearm
>> > +  * themselves in their callbacks. To be paranoid, we need to
>> > +  * drain the workqueue a second time after waiting for the RCU
>> > +  * grace period so that we catch work queued via RCU from the first
>> > +  * pass. As neither drain_workqueue() nor flush_workqueue() report
>> > +  * a result, we make an assumption that we only don't require more
>> > +  * than 2 passes to catch all recursive RCU delayed work.
>> > +  *
>> > +  */
>> > + int pass = 2;
>> > + do {
>> > + rcu_barrier();
>> > + drain_workqueue(i915->wq);
>> 
>> I am fine with the paranoia, and it covers the case below. Still if we do:
>> 
>> drain_workqueue();
>> rcu_barrier();
>> 
>> With drawining in progress, only chain queuing is allowed. I understand
>> this so that when it returns, all the ctx pointers are now unreferenced
>> but not freed.
>> 
>> Thus the rcu_barrier() after it cleans the trash and we are good to
>> be unloaded. With one pass.
>> 
>> I guess it comes to how to understand the comment, so could you
>> elaborate the 'we have workers that are armed by RCU and then rearm
>> themselves'?. As from drain_workqueue desc, this should be covered.
>
> I'm considering that they may be rearmed via RCU in the general case,
> e.g. context close frees an object and so goes onto an RCU list that
> once processed kicks off a new worker and so requires another round of
> drain_workqueue. We are in module unload so a few extra delays to belts
> and braces are ok until somebody notices it takes a few minutes to run a
> reload test ;)

Ok. Patch is
Reviewed-by: Mika Kuoppala 

> -Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Use AUX for backlight only if eDP 1.4 or later

[Patchwork]

== Series Details ==

Series: drm/i915: Use AUX for backlight only if eDP 1.4 or later
URL   : https://patchwork.freedesktop.org/series/27566/
State : success

== Summary ==

Series 27566v1 drm/i915: Use AUX for backlight only if eDP 1.4 or later
https://patchwork.freedesktop.org/api/1.0/series/27566/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:441s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:429s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:358s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:546s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:514s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:496s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:487s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:600s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:437s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:415s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:418s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:501s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:471s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:577s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:578s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:558s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:461s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:584s
fi-skl-6700k total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  
time:471s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:474s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:434s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:471s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:540s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:403s

541e75cb2ac9f85388e9e6f228d98f266506eebd drm-tip: 2017y-07m-19d-10h-10m-36s UTC 
integration manifest
b10d4f6 drm/i915: Use AUX for backlight only if eDP 1.4 or later

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5232/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [intel-gfx][i-g-t][help] gem_rungfill skiped

[Chris Wilson]

Quoting Gu, HailinX (2017-07-19 12:28:25)
> Hi~ all,
> 
>  
> 
> There is one case  will skip and prints  “No known gpu found”. There are some
> test cases will check on this, but they have not skiped.

The test uses the vgem module to measure the ring size on your machine.
The error message is still unhelpful.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: More stolen quirking

[Patchwork]

== Series Details ==

Series: drm/i915: More stolen quirking
URL   : https://patchwork.freedesktop.org/series/27561/
State : success

== Summary ==

Series 27561v1 drm/i915: More stolen quirking
https://patchwork.freedesktop.org/api/1.0/series/27561/revisions/1/mbox/

Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705
Test drv_module_reload:
Subgroup basic-reload:
dmesg-warn -> PASS   (fi-skl-6700k) fdo#98683 +3

fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fdo#98683 https://bugs.freedesktop.org/show_bug.cgi?id=98683

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:445s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:429s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:360s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:541s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:509s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:500s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:491s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:604s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:439s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:415s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:419s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:505s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:486s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:463s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:574s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:586s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:560s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:457s
fi-skl-6700hqtotal:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  
time:591s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:466s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:480s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:436s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:502s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:543s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:403s

541e75cb2ac9f85388e9e6f228d98f266506eebd drm-tip: 2017y-07m-19d-10h-10m-36s UTC 
integration manifest
59995ab drm/i915: More stolen quirking

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5231/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [intel-gfx][i-g-t] test kms_frontbuffer_tracking basic fail

[Chris Wilson]

Quoting Gu, HailinX (2017-07-19 11:58:37)
> Hi Arek
> 
> This time it looks like a bug
> 
> I run this test on i7-4770, by command `./igt/tests/kms_frontbuffer_tracking
> --run-subtest basic`
> 
> I guess it may be caused by I have no monitor connected. But if that is the
> case it should be skiped rather than fail.

It's because your kernel doesn't have a bugfix for WC domains and the
test isn't checking that it can use mmap_wc first.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Drain the device workqueue on unload

[Chris Wilson]

Quoting Mika Kuoppala (2017-07-19 12:18:47)
> Chris Wilson  writes:
> 
> > Workers on the i915->wq may rearm themselves so for completeness we need
> > to replace our flush_workqueue() with a call to drain_workqueue() before
> > unloading the device.
> >
> > v2: Reinforce the drain_workqueue with an preceeding rcu_barrier() as a
> > few of the tasks that need to be drained may first be armed by RCU.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=101627
> > Signed-off-by: Chris Wilson 
> > Cc: Matthew Auld 
> > Cc: Mika Kuoppala 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c  |  6 ++
> >  drivers/gpu/drm/i915/i915_drv.h  | 20 
> >  drivers/gpu/drm/i915/selftests/mock_gem_device.c |  2 +-
> >  3 files changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 4b62fd012877..41c5b11a7c8f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -596,7 +596,8 @@ static const struct vga_switcheroo_client_ops 
> > i915_switcheroo_ops = {
> >  
> >  static void i915_gem_fini(struct drm_i915_private *dev_priv)
> >  {
> > - flush_workqueue(dev_priv->wq);
> > + /* Flush any outstanding unpin_work. */
> > + i915_gem_drain_workqueue(dev_priv);
> >  
> >   mutex_lock(_priv->drm.struct_mutex);
> >   intel_uc_fini_hw(dev_priv);
> > @@ -1409,9 +1410,6 @@ void i915_driver_unload(struct drm_device *dev)
> >   cancel_delayed_work_sync(_priv->gpu_error.hangcheck_work);
> >   i915_reset_error_state(dev_priv);
> >  
> > - /* Flush any outstanding unpin_work. */
> > - drain_workqueue(dev_priv->wq);
> > -
> >   i915_gem_fini(dev_priv);
> >   intel_uc_fini_fw(dev_priv);
> >   intel_fbc_cleanup_cfb(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 667fb5c44483..e9a4b96dc775 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3300,6 +3300,26 @@ static inline void 
> > i915_gem_drain_freed_objects(struct drm_i915_private *i915)
> >   } while (flush_work(>mm.free_work));
> >  }
> >  
> > +static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915)
> > +{
> > + /*
> > +  * Similar to objects above (see i915_gem_drain_freed-objects), in
> > +  * general we have workers that are armed by RCU and then rearm
> > +  * themselves in their callbacks. To be paranoid, we need to
> > +  * drain the workqueue a second time after waiting for the RCU
> > +  * grace period so that we catch work queued via RCU from the first
> > +  * pass. As neither drain_workqueue() nor flush_workqueue() report
> > +  * a result, we make an assumption that we only don't require more
> > +  * than 2 passes to catch all recursive RCU delayed work.
> > +  *
> > +  */
> > + int pass = 2;
> > + do {
> > + rcu_barrier();
> > + drain_workqueue(i915->wq);
> 
> I am fine with the paranoia, and it covers the case below. Still if we do:
> 
> drain_workqueue();
> rcu_barrier();
> 
> With drawining in progress, only chain queuing is allowed. I understand
> this so that when it returns, all the ctx pointers are now unreferenced
> but not freed.
> 
> Thus the rcu_barrier() after it cleans the trash and we are good to
> be unloaded. With one pass.
> 
> I guess it comes to how to understand the comment, so could you
> elaborate the 'we have workers that are armed by RCU and then rearm
> themselves'?. As from drain_workqueue desc, this should be covered.

I'm considering that they may be rearmed via RCU in the general case,
e.g. context close frees an object and so goes onto an RCU list that
once processed kicks off a new worker and so requires another round of
drain_workqueue. We are in module unload so a few extra delays to belts
and braces are ok until somebody notices it takes a few minutes to run a
reload test ;)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [intel-gfx][i-g-t][help] gem_rungfill skiped

[Gu, HailinX]

Hi~ all,

There is one case  will skip and prints  "No known gpu found". There are some 
test cases will check on this, but they have not skiped.
I think there may be something wrong.

I run the test by command:
./igt/tests/gem_ringfill --run-subtest basic-default-interruptible

./igt/tests/gem_ringfill --run-subtest basic-default-forked"

./igt/tests/gem_ringfill --run-subtest basic-default-interruptible",



And the out put shows below

IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.9.0 x86_64)

Test requirement not met in function drm_open_driver, file drmtest.c:359:

Test requirement: !(fd<0)

No known gpu found

Last errno: 2, No such file or directory

Subtest basic-default: SKIP



I know It will call "fd=drm_open_driver(DRIVER_INTEL);" before the test program.

But it doesn't skip here in other test cases.

And here is the cpu info. A total of 8 processors

-

vendor_id   : GenuineIntel

cpu family  : 6

model   : 60

model name  : Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz

stepping: 3

microcode   : 0x16

cpu MHz : 3392.130

cache size  : 8192 KB

physical id : 0

siblings: 8

core id : 3

cpu cores   : 4

apicid  : 7

initial apicid  : 7

fpu : yes

fpu_exception   : yes

cpuid level : 13

wp  : yes

---



Thank you

Gu

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v12 6/6] drm/i915: Introduce GEM proxy

[Chris Wilson]

s/GEM proxy/a GEM proxy object/

Quoting Tina Zhang (2017-07-19 11:59:05)

Rationale goes here.

> Signed-off-by: Tina Zhang 
> ---
>  drivers/gpu/drm/i915/i915_gem.c| 26 +-
>  drivers/gpu/drm/i915/i915_gem_object.h |  9 +
>  drivers/gpu/drm/i915/i915_gem_tiling.c |  5 +
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1b2dfa8..ebacc04 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1612,6 +1612,12 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void 
> *data,
> if (!obj)
> return -ENOENT;
>  
> +   /* proxy gem object does not support setting domain */

Yes, this is what the code is doing. The comment tells us why.

/*
 * Proxy objects do not control access to the backing storage, ergo
 * they cannot be used as a means to manipulate the cache domain
 * tracking for that backing storage. The proxy object is always
 * considered to be outside of any cache domain.
 */

However, set-domain does have some other side-effects that includes
waiting which should still be performed, i.e. this check should be after
the lockless wait.

> +   if (i915_gem_object_is_proxy(obj)) {
> +   err = -EPERM;
> +   goto out;
> +   }
> +
> /* Try to flush the object off the GPU without holding the lock.
>  * We will repeat the flush holding the lock in the normal manner
>  * to catch cases where we are gazumped.
> @@ -1680,6 +1686,12 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void 
> *data,
> if (!obj)
> return -ENOENT;
>  

A comment could explain that since proxy objects are barred from CPU
access, we do not need to ban sw_finish as it is a nop.

> +   /* proxy gem obj does not support this operation */
> +   if (i915_gem_object_is_proxy(obj)) {
> +   i915_gem_object_put(obj);
> +   return -EPERM;
> +   }
> +
> /* Pinned buffers may be scanout, so flush the cache */
> i915_gem_object_flush_if_display(obj);
> i915_gem_object_put(obj);
> @@ -1730,7 +1742,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  */
> if (!obj->base.filp) {
> i915_gem_object_put(obj);
> -   return -EINVAL;
> +   return -EPERM;
> }
>  
> addr = vm_mmap(obj->base.filp, 0, args->size,
> @@ -3764,6 +3776,12 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, 
> void *data,
> if (!obj)
> return -ENOENT;
>  
> +   /* proxy gem obj does not support setting caching mode */

More rationale. Also is the proxied object (its target) also banned from
changing the PTE bits or do we inherit all such changes automatically?

> +   if (i915_gem_object_is_proxy(obj)) {
> +   ret = -EPERM;
> +   goto out;
> +   }
> +
> if (obj->cache_level == level)
> goto out;
>  
> @@ -4210,6 +4228,12 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void 
> *data,
> if (!obj)
> return -ENOENT;
>  
> +   /* proxy gem obj does not support changing backding storage */

This one could be generalised to I915_GEM_OBJECT_IS_SHRINKABLE?

> +   if (i915_gem_object_is_proxy(obj)) {
> +   err = -EPERM;
> +   goto out;
> +   }
> +
> err = mutex_lock_interruptible(>mm.lock);
> if (err)
> goto out;
> diff --git a/drivers/gpu/drm/i915/i915_gem_object.h 
> b/drivers/gpu/drm/i915/i915_gem_object.h
> index 5b19a49..c91e336 100644
> --- a/drivers/gpu/drm/i915/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/i915_gem_object.h
> @@ -39,6 +39,7 @@ struct drm_i915_gem_object_ops {
> unsigned int flags;
>  #define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0)
>  #define I915_GEM_OBJECT_IS_SHRINKABLE   BIT(1)
> +#define I915_GEM_OBJECT_IS_PROXY   BIT(2)
>  
> /* Interface between the GEM object and its backing storage.
>  * get_pages() is called once prior to the use of the associated set
> @@ -198,6 +199,8 @@ struct drm_i915_gem_object {
> } userptr;
>  
> unsigned long scratch;
> +
> +   void *gvt_info;

Unrelated chunk, this should go into the gvt patch to use the object.

> };
>  
> /** for phys allocated objects */
> @@ -300,6 +303,12 @@ i915_gem_object_is_shrinkable(const struct 
> drm_i915_gem_object *obj)
>  }
>  
>  static inline bool
> +i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj)
> +{
> +   return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY;
> +}
> +
> +static inline bool
>  i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
>  {
> return obj->active_count;
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
> 

Re: [Intel-gfx] [PATCH v2] drm/i915: Drain the device workqueue on unload

[Mika Kuoppala]

Chris Wilson  writes:

> Workers on the i915->wq may rearm themselves so for completeness we need
> to replace our flush_workqueue() with a call to drain_workqueue() before
> unloading the device.
>
> v2: Reinforce the drain_workqueue with an preceeding rcu_barrier() as a
> few of the tasks that need to be drained may first be armed by RCU.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=101627
> Signed-off-by: Chris Wilson 
> Cc: Matthew Auld 
> Cc: Mika Kuoppala 
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |  6 ++
>  drivers/gpu/drm/i915/i915_drv.h  | 20 
>  drivers/gpu/drm/i915/selftests/mock_gem_device.c |  2 +-
>  3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 4b62fd012877..41c5b11a7c8f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -596,7 +596,8 @@ static const struct vga_switcheroo_client_ops 
> i915_switcheroo_ops = {
>  
>  static void i915_gem_fini(struct drm_i915_private *dev_priv)
>  {
> - flush_workqueue(dev_priv->wq);
> + /* Flush any outstanding unpin_work. */
> + i915_gem_drain_workqueue(dev_priv);
>  
>   mutex_lock(_priv->drm.struct_mutex);
>   intel_uc_fini_hw(dev_priv);
> @@ -1409,9 +1410,6 @@ void i915_driver_unload(struct drm_device *dev)
>   cancel_delayed_work_sync(_priv->gpu_error.hangcheck_work);
>   i915_reset_error_state(dev_priv);
>  
> - /* Flush any outstanding unpin_work. */
> - drain_workqueue(dev_priv->wq);
> -
>   i915_gem_fini(dev_priv);
>   intel_uc_fini_fw(dev_priv);
>   intel_fbc_cleanup_cfb(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 667fb5c44483..e9a4b96dc775 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3300,6 +3300,26 @@ static inline void i915_gem_drain_freed_objects(struct 
> drm_i915_private *i915)
>   } while (flush_work(>mm.free_work));
>  }
>  
> +static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915)
> +{
> + /*
> +  * Similar to objects above (see i915_gem_drain_freed-objects), in
> +  * general we have workers that are armed by RCU and then rearm
> +  * themselves in their callbacks. To be paranoid, we need to
> +  * drain the workqueue a second time after waiting for the RCU
> +  * grace period so that we catch work queued via RCU from the first
> +  * pass. As neither drain_workqueue() nor flush_workqueue() report
> +  * a result, we make an assumption that we only don't require more
> +  * than 2 passes to catch all recursive RCU delayed work.
> +  *
> +  */
> + int pass = 2;
> + do {
> + rcu_barrier();
> + drain_workqueue(i915->wq);

I am fine with the paranoia, and it covers the case below. Still if we do:

drain_workqueue();
rcu_barrier();

With drawining in progress, only chain queuing is allowed. I understand
this so that when it returns, all the ctx pointers are now unreferenced
but not freed.

Thus the rcu_barrier() after it cleans the trash and we are good to
be unloaded. With one pass.

I guess it comes to how to understand the comment, so could you
elaborate the 'we have workers that are armed by RCU and then rearm
themselves'?. As from drain_workqueue desc, this should be covered.

Thanks,
-Mika

> + } while (--pass);
> +}
> +
>  struct i915_vma * __must_check
>  i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
>const struct i915_ggtt_view *view,
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c 
> b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index 47613d20bba8..7a468cb30946 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -57,7 +57,7 @@ static void mock_device_release(struct drm_device *dev)
>  
>   cancel_delayed_work_sync(>gt.retire_work);
>   cancel_delayed_work_sync(>gt.idle_work);
> - flush_workqueue(i915->wq);
> + i915_gem_drain_workqueue(i915);
>  
>   mutex_lock(>drm.struct_mutex);
>   for_each_engine(engine, i915, id)
> -- 
> 2.13.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v4 2/7] chamelium: Calculate CRC from framebuffer instead of hardcoding it

[Paul Kocialkowski]

On Mon, 2017-07-17 at 12:29 -0400, Lyude Paul wrote:
> On Wed, 2017-07-12 at 17:50 +0300, Paul Kocialkowski wrote:
> > This introduces CRC calculation for reference frames, instead of
> > using
> > hardcoded values for them. The rendering of reference frames may
> > differ
> > from machine to machine, especially due to font rendering, and the
> > frame itself may change with subsequent IGT changes.
> > 
> > These differences would cause the CRC checks to fail on different
> > setups. This allows them to pass regardless of the setup.
> 
> Just one question before I push this since I didn't think of testing
> this previously and don't have access to my chamelium at the moment.
> Have you made sure that this doesn't break things with igt if a test
> gets interrupted due to failure in the middle of an asynchronous CRC
> calculation?

So I have now tested that a failed assert (in the main thread) in the
middle of the threaded calculation will not cause any segfault. There is
no reason why it should, because none of the ressources used for the
calculation are liberated prior to the end of the process. Everything is
really cleaned up when the process dies.

On the other hand, it may happen, due to the call to
igt_get_cairo_surface that an igt_assert fails in the calculation
thread, which definitely causes a segfault.

I will rework the patches so that the call to the function is made prior
to starting the thread, thus avoiding any possibility of hitting a
failed igt_assert in the thread itself.

> Other then that, everything here looks good.
> > 
> > Signed-off-by: Paul Kocialkowski 
> > ---
> >  lib/igt_chamelium.c | 143
> > 
> >  lib/igt_chamelium.h |   5 ++
> >  tests/chamelium.c   |  77 +++-
> >  3 files changed, 167 insertions(+), 58 deletions(-)
> > 
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index 93392af7..baa6399c 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -94,6 +94,14 @@ struct chamelium_frame_dump {
> > struct chamelium_port *port;
> >  };
> >  
> > +struct chamelium_fb_crc_async_data {
> > +   int fd;
> > +   struct igt_fb *fb;
> > +
> > +   pthread_t thread_id;
> > +   igt_crc_t *ret;
> > +};
> > +
> >  struct chamelium {
> > xmlrpc_env env;
> > xmlrpc_client *client;
> > @@ -998,6 +1006,141 @@ int chamelium_get_frame_limit(struct
> > chamelium
> > *chamelium,
> > return ret;
> >  }
> >  
> > +static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer,
> > int width,
> > + int height, int k, int m)
> > +{
> > +   unsigned char r, g, b;
> > +   uint64_t sum = 0;
> > +   uint64_t count = 0;
> > +   uint64_t value;
> > +   uint32_t hash;
> > +   int index;
> > +   int i;
> > +
> > +   for (i=0; i < width * height; i++) {
> > +   if ((i % m) != k)
> > +   continue;
> > +
> > +   index = i * 4;
> > +
> > +   r = buffer[index + 2];
> > +   g = buffer[index + 1];
> > +   b = buffer[index + 0];
> > +
> > +   value = r | (g << 8) | (b << 16);
> > +   sum += ++count * value;
> > +   }
> > +
> > +   hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >>
> > 48)) & 0x;
> > +
> > +   return hash;
> > +}
> > +
> > +static void chamelium_do_calculate_fb_crc(int fd, struct igt_fb
> > *fb,
> > + igt_crc_t *out)
> > +{
> > +   unsigned char *buffer;
> > +   cairo_surface_t *fb_surface;
> > +   int n = 4;
> > +   int w, h;
> > +   int i, j;
> > +
> > +   /* Get the cairo surface for the framebuffer */
> > +   fb_surface = igt_get_cairo_surface(fd, fb);
> > +
> > +   buffer = cairo_image_surface_get_data(fb_surface);
> > +   w = fb->width;
> > +   h = fb->height;
> > +
> > +   for (i = 0; i < n; i++) {
> > +   j = n - i - 1;
> > +   out->crc[i] = chamelium_xrgb_hash16(buffer, w, h,
> > j,
> > n);
> > +   }
> > +
> > +   out->n_words = n;
> > +}
> > +
> > +/**
> > + * chamelium_calculate_fb_crc:
> > + * @fd: The drm file descriptor
> > + * @fb: The framebuffer to calculate the CRC for
> > + *
> > + * Calculates the CRC for the provided framebuffer, using the
> > Chamelium's CRC
> > + * algorithm. This calculates the CRC in a synchronous fashion.
> > + *
> > + * Returns: The calculated CRC
> > + */
> > +igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
> > +{
> > +   igt_crc_t *ret = calloc(1, sizeof(igt_crc_t));
> > +
> > +   chamelium_do_calculate_fb_crc(fd, fb, ret);
> > +
> > +   return ret;
> > +}
> > +
> > +static void *chamelium_calculate_fb_crc_async_work(void *data)
> > +{
> > +   struct chamelium_fb_crc_async_data *fb_crc;
> > +
> > +   fb_crc = (struct chamelium_fb_crc_async_data *) data;
> > +
> > +   chamelium_do_calculate_fb_crc(fb_crc->fd, fb_crc->fb,
> > fb_crc->ret);
> > +
> > +   return NULL;
> > +}
> > +
> > +/**
> > + * 

Re: [Intel-gfx] [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-19 10:30:13)
> 
> On 18/07/2017 16:22, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> >> +u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
> >> +{
> >> +   unsigned long flags;
> >> +   u64 total;
> >> +
> >> +   spin_lock_irqsave(>stats.lock, flags);
> >> +
> >> +   total = engine->stats.total;
> >> +
> >> +   /*
> >> +* If the engine is executing something at the moment
> >> +* add it to the total.
> >> +*/
> >> +   if (engine->stats.ref)
> >> +   total += ktime_get_real_ns() - engine->stats.start;
> >> +
> >> +   spin_unlock_irqrestore(>stats.lock, flags);
> > 
> > Answers to another patch found here. I would say this is the other half
> > of the interface and should be kept together.
> 
> Yes, it was an ugly split.
> 
> On 18/07/2017 16:43, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> >> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> >> +{
> >> +   if (!i915.enable_execlists)
> >> +   return -ENODEV;
> >> +
> >> +   mutex_lock(_engine_stats_mutex);
> >> +   if (i915_engine_stats_ref++ == 0) {
> >> +   struct intel_engine_cs *engine;
> >> +   enum intel_engine_id id;
> >> +
> >> +   for_each_engine(engine, dev_priv, id) {
> >> +   memset(>stats, 0, sizeof(engine->stats));
> >> +   spin_lock_init(>stats.lock);
> >> +   }
> >> +
> >> +   static_branch_enable(_engine_stats_key);
> >> +   }
> >> +   mutex_unlock(_engine_stats_mutex);
> > 
> > I don't think static_branch_enable() is a might_sleep, so it looks like
> > you can rewrite this avoiding the mutex and thus not requiring the
> > worker and then can use the error code here to decide if you need to
> > use the timer instead.
> 
> Perhaps I could get rid of the mutex though by using atomic_inc/dec_return.
> 
> But there is a mutex in jump label handling,

Totally missed it. I wonder why it is a mutex, certainly serialising
enable/disable need something. The comments suggest that once upon a
time (or different arch?) it was much more of a stop_machine().

> so I think the workers have 
> to stay - and it is also beneficial to have delayed static branch disable,
> since the perf core seems to like calling start/stop on the events a lot.
> But it is recommended practice for static branches in any way.

Interesting there is a static_key_slow_dec_deferred. But honestly I
think it is hard to defend a global static_key for a per-device
interface.

> So from that angle I could perhaps even move the delayed disable to this
> patch so it is automatically shared by all callers.
> 
> >> +static DEFINE_MUTEX(i915_engine_stats_mutex);
> >> +static int i915_engine_stats_ref;
> >>   
> >>   /* Haswell does have the CXT_SIZE register however it does not appear to 
> >> be
> >>* valid. Now, docs explain in dwords what is in the context object. The 
> >> full
> >> @@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct 
> >> drm_i915_private *i915)
> >>  }
> >>   }
> >>   
> >> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> > 
> > The pattern I've been trying to use here is
> > 
> >   intel_engine_* - operate on the named engine
> > 
> >   intel_engines_* - operate on all engines
> 
> Ok.
>  
> > Long term though having a global static key is going to be a nasty wart.
> > Joonas will definitely ask the question how much will it cost us to use
> > an engine->bool and what we can do to minimise that cost.
> 
> Why you think it is nasty? Sounds pretty cool to me.

If we enable sampling on one device (engine even!), it affects another.
But the device is the more compelling argument against.
 
> But I think can re-organize the series to start with a normal branch and
> then add the static one on top if so is desired.

Ok. I like the idea of dynamically patching in branches, and hate to be
party pooper!
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx