[Intel-gfx] [PATCH] drm/i915: increase the tries for HDMI hotplug live status checking

[Gary Wang]

The total delay of HDMI hotplug detecting with 30ms is sometimes not
enoughtfor HDMI live status up with specific HDMI monitors in BSW platform.

After doing experiments for following monitors, it needs 80ms at least
for those worst cases.

Lenovo L246 1xwA (4 failed, necessary hot-plug delay: 58/40/60/40ms)
Philips HH2AP (9 failed, necessary hot-plug delay: 80/50/50/60/46/40/58/58/39ms)
BENQ ET-0035-N (6 failed, necessary hot-plug delay: 60/50/50/80/80/40ms)
DELL U2713HM (2 failed, necessary hot-plug delay: 58/59ms)
HP HP-LP2475w (5 failed, necessary hot-plug delay: 70/50/40/60/40ms)

It looks like 70-80 ms is BSW platform needs in some bad cases of the
monitors at this end (8 times delay at most). Keep less than 100ms for
HDCP pulse HPD low (with at least 100ms) to respond a plug out.

Reviewed-by: Cooper Chiou 
Tested-by: Gary Wang 
Cc: Gavin Hindman 
Cc: Sonika Jindal 
Cc: Shashank Sharma 
Cc: Shobhit Kumar 
Signed-off-by: Gary Wang 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 mode change 100644 => 100755 drivers/gpu/drm/i915/intel_hdmi.c

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
old mode 100644
new mode 100755
index 6214175..4a77639
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1392,7 +1392,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
force)
 
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
 
-   for (try = 0; !live_status && try < 4; try++) {
+   for (try = 0; !live_status && try < 9; try++) {
if (try)
msleep(10);
live_status = intel_digital_port_connected(dev_priv,
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915: increase the tries for HDMI hotplug live status checking

[Wang, Gary C]

For my test on this issue, it only got high fail-rate with some specific HDMI 
monitors. 

It’s also not easy to reproduce it by modern HDMI monitors (with 1080p/2K/4K 
resolutions…etc.) or will get low fail-rat (e.g. 1/50 failed test) with that 
ones. So Linux BSW Gfx validation team could also suffer being hard to 
reproduce it.

I just would like to raise this problem (about HDMI compatibility) and collect 
practical cases for handling this kind of issue more smoothly.

Thanks!

Gary

-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Gary Wang
Sent: Wednesday, December 23, 2015 4:12 PM
To: intel-gfx@lists.freedesktop.org
Cc: Kumar, Shobhit 
Subject: [Intel-gfx] [PATCH] drm/i915: increase the tries for HDMI hotplug live 
status checking

The total delay of HDMI hotplug detecting with 30ms is sometimes not enoughtfor 
HDMI live status up with specific HDMI monitors in BSW platform.

After doing experiments for following monitors, it needs 80ms at least for 
those worst cases.

Lenovo L246 1xwA (4 failed, necessary hot-plug delay: 58/40/60/40ms) Philips 
HH2AP (9 failed, necessary hot-plug delay: 80/50/50/60/46/40/58/58/39ms) BENQ 
ET-0035-N (6 failed, necessary hot-plug delay: 60/50/50/80/80/40ms) DELL 
U2713HM (2 failed, necessary hot-plug delay: 58/59ms) HP HP-LP2475w (5 failed, 
necessary hot-plug delay: 70/50/40/60/40ms)

It looks like 70-80 ms is BSW platform needs in some bad cases of the monitors 
at this end (8 times delay at most). Keep less than 100ms for HDCP pulse HPD 
low (with at least 100ms) to respond a plug out.

Reviewed-by: Cooper Chiou 
Tested-by: Gary Wang 
Cc: Gavin Hindman 
Cc: Sonika Jindal 
Cc: Shashank Sharma 
Cc: Shobhit Kumar 
Signed-off-by: Gary Wang 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)  mode change 100644 => 100755 
drivers/gpu/drm/i915/intel_hdmi.c

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
old mode 100644
new mode 100755
index 6214175..4a77639
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1392,7 +1392,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
force)
 
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
 
-   for (try = 0; !live_status && try < 4; try++) {
+   for (try = 0; !live_status && try < 9; try++) {
if (try)
msleep(10);
live_status = intel_digital_port_connected(dev_priv,
--
1.9.1

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

[Intel-gfx] ✗ failure: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on 7e671e69deffb88d60687dacffe6e34a5d046500 drm-intel-nightly: 
2015y-12m-22d-13h-28m-34s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (skl-i5k-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS   (ilk-hp8440p)
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (bsw-nuc-2)
dmesg-warn -> PASS   (hsw-xps12)
pass   -> DMESG-WARN (hsw-brixbox)
dmesg-warn -> PASS   (ilk-hp8440p)
dmesg-warn -> PASS   (byt-nuc)
Subgroup basic-flip-vs-wf_vblank:
dmesg-warn -> PASS   (snb-x220t)
Subgroup basic-plain-flip:
pass   -> DMESG-WARN (hsw-xps12)
pass   -> DMESG-WARN (bdw-nuci7)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (snb-x220t)
Subgroup read-crc-pipe-a:
pass   -> DMESG-WARN (snb-x220t)
Subgroup read-crc-pipe-a-frame-sequence:
dmesg-warn -> PASS   (byt-nuc)
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS   (skl-i5k-2)
Subgroup read-crc-pipe-c:
pass   -> DMESG-WARN (skl-i5k-2)
Test kms_psr_sink_crc:
Subgroup psr_basic:
dmesg-warn -> PASS   (bdw-ultra)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass   -> DMESG-WARN (bdw-ultra)

bdw-nuci7total:132  pass:121  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultratotal:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc  total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox  total:135  pass:126  dwarn:2   dfail:0   fail:0   skip:7  
hsw-xps12total:132  pass:125  dwarn:3   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430stotal:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
skl-i7k-2total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
snb-dellxps  total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220ttotal:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c

Results at /archive/results/CI_IGT_test/Patchwork_801/

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

[Intel-gfx] [PATCH] tests/kms_rotation_crc : Added Support for 90/270 roation tests for RGB565 Formats

[Mayuresh Gharpure]

Signed-off-by: Mayuresh Gharpure 
---
 tests/kms_rotation_crc.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index c3241cf..8732eac 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -268,7 +268,7 @@ static void test_plane_rotation(data_t *data, enum 
igt_plane plane_type)
 
igt_plane_set_rotation(plane, data->rotation);
ret = igt_display_try_commit2(display, commit);
-   if (data->override_fmt || data->override_tiling) {
+   if ((data->override_fmt && data->override_fmt != 
DRM_FORMAT_RGB565) || data->override_tiling) {
igt_assert_eq(ret, -EINVAL);
} else {
igt_assert_eq(ret, 0);
@@ -579,6 +579,34 @@ igt_main
test_plane_rotation_exhaust_fences(&data, IGT_PLANE_PRIMARY);
}
 
+   igt_subtest_f("primary-rotation-RGB565-90") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_90;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_PRIMARY);
+   }
+
+   igt_subtest_f("primary-rotation-RGB565-270") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_270;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_PRIMARY);
+   }
+
+   igt_subtest_f("sprite-rotation-RGB565-90") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_90;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_2);
+   }
+
+   igt_subtest_f("sprite-rotation-RGB565-270") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_270;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_2);
+   }
+
igt_fixture {
igt_display_fini(&data.display);
}
-- 
1.9.1

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

[Intel-gfx] [PATCH] tests/kms_rotation_crc : Added Support for 90/270 roation tests for RGB565 Formats

[Mayuresh Gharpure]

Signed-off-by: Mayuresh Gharpure 
---
 tests/kms_rotation_crc.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index c3241cf..8732eac 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -268,7 +268,7 @@ static void test_plane_rotation(data_t *data, enum 
igt_plane plane_type)
 
igt_plane_set_rotation(plane, data->rotation);
ret = igt_display_try_commit2(display, commit);
-   if (data->override_fmt || data->override_tiling) {
+   if ((data->override_fmt && data->override_fmt != 
DRM_FORMAT_RGB565) || data->override_tiling) {
igt_assert_eq(ret, -EINVAL);
} else {
igt_assert_eq(ret, 0);
@@ -579,6 +579,34 @@ igt_main
test_plane_rotation_exhaust_fences(&data, IGT_PLANE_PRIMARY);
}
 
+   igt_subtest_f("primary-rotation-RGB565-90") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_90;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_PRIMARY);
+   }
+
+   igt_subtest_f("primary-rotation-RGB565-270") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_270;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_PRIMARY);
+   }
+
+   igt_subtest_f("sprite-rotation-RGB565-90") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_90;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_2);
+   }
+
+   igt_subtest_f("sprite-rotation-RGB565-270") {
+   igt_require(gen >= 9);
+   data.rotation = IGT_ROTATION_270;
+   data.override_fmt = DRM_FORMAT_RGB565;
+   test_plane_rotation(&data, IGT_PLANE_2);
+   }
+
igt_fixture {
igt_display_fini(&data.display);
}
-- 
1.9.1

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

Re: [Intel-gfx] [RFC 2/2] drm/i915: Render decompression support for Gen9

[Daniel Stone]

Hi Vandana,

On 23 December 2015 at 03:20, Kannan, Vandana  wrote:
> How does VT switch work in case of rotation, setting different pixel format,
> etc?

Pixel format is a property of the framebuffer, not a per-plane
property, so is unaffected. Rotation is generic, so there is specific
code to handle it for fbdev etc.

But the real problem is that this is an Intel-specific property, and
in order to get correct results, userspace must know about these
specific properties in order to unset them. This means that you can't
run generic userspace, and you can't run old userspace which predates
these properties either. This seems like a total non-starter to me.

It would be much cleaner if there was a way to attach the render
compression property to the framebuffer, e.g. by using the format
modifiers which were specifically introduced to deal with compression
and tiling.

Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/6] drm: Create Color Management DRM properties

[Daniel Stone]

Hi,

On 21 December 2015 at 12:38, Daniel Vetter  wrote:
> On Fri, Dec 18, 2015 at 04:53:28PM +, Daniel Stone wrote:
>> > +struct drm_r32g32b32 {
>> > +   /*
>> > +* Data is in U8.24 fixed point format.
>> > +* All platforms support values within [0, 1.0] range,
>> > +* for Red, Green and Blue colors.
>> > +*/
>> > +   __u32 r32;
>> > +   __u32 g32;
>> > +   __u32 b32;
>> > +   __u32 reserved;
>> > +};
>>
>> Wait, it's U8.24 (i.e. 0 -> 255.0999403953517), but the supported
>> range is [0..1.0]? What am I missing?
>
> Probably means:
> - all platforms MUST support [0.0, 1.0] range, inclusive
> - platforms CAN support larger values (which can make sense in degamma
>   tables if your CTM will shrink things down again).

Ah, makes sense.

>> I also don't have a good answer on how to support non-CM-aware
>> clients. Currently they'll just blindly carry around the correction
>> factors from previous clients, which is _really_ bad: imagine you have
>> Weston ported to use KMS/CM to correct perfectly, and then start
>> Mutter/GNOME which still corrects perfectly, but using lcms and
>> various client-side compensation rather than the CM engine. Your
>> output will now be double-corrected, i.e. wrong, because Mutter won't
>> know to reset the CM properties.
>
> Hm yeah, I think legacy entry points should remap to atomic ones.
> Otherwise things are massively inconsistent (and we have a problem
> figuring out when/which table applies in the driver). One problem with
> that approach is that the legacy table has the assumption of a fixed
> 256 entries (well we expose the size somewhere, but that's what everyone
> uses). At least on some platforms, the full-blown table used by these
> patches isn't even an integer multiple of that.

It's not even a legacy vs. atomic thing, this can happen in
pure-atomic as well. Same as the render-compression plane property
that I just replied to here as well.

- Weston starts and sets up palette/CTM properties
- VT switch to Mutter, which isn't aware of new properties
- Mutter atomically sets new plane state, containing framebuffer which
is already colour-corrected for the target
- result is now double-corrected as Mutter didn't know to unset the
old properties

IOW, in the current model, any user of CM has to explicitly unset it
before handover (not always actually possible), or any generic
userspace must unset every property it sees and doesn't know about,
hoping that setting to 0 will do the right thing.

Or we could add another client cap, which would prevent the CM
properties from ever being exposed to userspace which doesn't know how
to work with it. Passing the client caps through to
plane_duplicate_state also means that we can unset the CM properties
at this early point for unaware clients. This would mean that we
wouldn't have to have code to explicitly unset it in, e.g., every
legacy hook.

>> About the only thing I can think of is to add a
>> DRM_CLIENT_CAP_COLOR_MGMT, and modify drm_atomic_*_duplicate_state to
>> take client caps (passed in from file_priv with the atomic ioctl, or
>> explicitly set by other kernel code, according to its capabilities),
>> and if the CM cap is not set, clear out the blobs when duplicating
>> state.

(As here.)

>> All of this also needs to be backed up by a lot more extensive IGT,
>> including disabling (from _every_ mode, not just 12-bit mode on BDW)
>> via setting blob == NULL, testing the various depths (I still don't
>> understand the difference between 8 and 10-bit on CHV), legacy gamma
>> hooks when using CM, testing reset/dumb clients when the above
>> duplicate_state issue is resolved, and especially the boundary cases
>> in conversions (e.g. the sign wraparound on CHV). The documentation
>> also _really_ needs fixing to be consistent with the code, and
>> consistent with itself. When that's done, I think I'll be
>> better-placed to do a second pass review, because after however many
>> revisions, I still can't clearly see how it would be used from generic
>> userspace (as opposed to an Intel-specific colour manager).
>
> One bit I'm not sure (and which isn't documented here) is that there's
> supposed to be a read-only hint property telling new generic userspace
> what tables are expected. This might mean you're not always using the most
> awesome configuration, but it should at least work. That part of the
> design seems to be undocumented.

Yeah, there is a single 'length' property per table, but given the
dizzying array of options available, I'm not really sure if that's
sufficient.

Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 2/2] drm/i915: Check DP no aux transaction bit on link training

[Jani Nikula]

On Tue, 22 Dec 2015, Lukas Wunner  wrote:
> Hi Mika,
>
> On Mon, Dec 21, 2015 at 01:39:15PM +0200, Mika Kahola wrote:
>> Check if no AUX transactions are required on DP link training.
>> If this bit is set, we can reuse the known good drive current
>> and pre-emphasis level from the last "full" link training.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
>> Signed-off-by: Mika Kahola 
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c   |  2 +-
>>  drivers/gpu/drm/i915/intel_dp_link_training.c | 19 +++
>>  drivers/gpu/drm/i915/intel_drv.h  |  1 +
>>  3 files changed, 21 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 6b36d82..3137187 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3852,7 +3852,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>>  intel_dp->DP = DP;
>>  }
>>  
>> -static bool
>> +bool
>>  intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>  {
>>  struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
>> b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> index 59d59be..5fb3f97 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> @@ -85,6 +85,25 @@ static bool
>>  intel_dp_reset_link_train(struct intel_dp *intel_dp,
>>  uint8_t dp_train_pat)
>>  {
>> +bool has_dpcd;
>> +bool no_aux_handshake = false;
>> +
>> +has_dpcd = intel_dp_get_dpcd(intel_dp);
>> +
>> +/*
>> + * Source device can try to use drive current and pre-emphasis
>> + * parameters computed by the last "full" link training if the
>> + * DP_NO_AUX_HANDSHAKE_LINK_TRAINING bit is set to 1.
>> + */
>> +if (has_dpcd) {
>> +if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
>> +no_aux_handshake = (intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
>> +DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
>> +}
>> +}
>> +
>> +intel_dp->train_set_valid &= no_aux_handshake;
>> +
>
> If Thierry Reding's patches get merged (posted to dri-devel@ on Dec 14
> with <1450097764-30063-1-git-send-email-thierry.red...@gmail.com>),
> you could just use
>
>   if (has_dpcd && drm_dp_fast_training_cap(intel_dp->dpcd))
>   intel_dp->train_set_valid &= true;
>
> Also, if the patches get merged, the no_aux_handshake attribute in dev_priv
> can be dropped (set in intel_edp_init_connector() but never read).

Good points, but we don't generally write code, let alone bugfixes, on
top of code that hasn't been merged.

BR,
Jani.


>
> Best regards,
>
> Lukas
>
>>  DRM_DEBUG_KMS("link training optimization: %s\n",
>>intel_dp->train_set_valid ? "true" : "false");
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index d523ebb..c36a70c 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1239,6 +1239,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
>> *crc);
>>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>>   struct intel_crtc_state *pipe_config);
>>  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
>> +bool intel_dp_get_dpcd(struct intel_dp *intel_dp);
>>  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
>>bool long_hpd);
>>  void intel_edp_backlight_on(struct intel_dp *intel_dp);
>> -- 
>> 1.9.1
>> 
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PULL] drm-intel-next

[Chris Wilson]

On Tue, Dec 22, 2015 at 04:31:22PM +, Tvrtko Ursulin wrote:
> 
> On 22/12/15 14:31, Chris Wilson wrote:
> >On Tue, Dec 22, 2015 at 03:05:14PM +0100, Daniel Vetter wrote:
> >>On Tue, Dec 22, 2015 at 11:37:18AM +0100, Daniel Vetter wrote:
> >>>Hi Dave,
> >>>
> >>>Final 4.5 feature pull for drm/i915!
> >>>
> >>>drm-intel-next-2015-12-18:
> >>>- fix atomic watermark recomputation logic (Maarten)
> >>>- modeset sequence fixes for LPT (Ville)
> >>>- more kbl enabling&prep work (Rodrigo, Wayne)
> >>>- first bits for mst audio
> >>>- page dirty tracking fixes from Dave Gordon
> >>>- new get_eld hook from Takashi, also included in the sound tree
> >>>- fixup cursor handling when placed at address 0 (Ville)
> >>>- refactor VBT parsing code (Jani)
> >>>- rpm wakelock debug infrastructure ( Imre)
> >>>- fbdev is pinned again (Chris)
> >>>- tune the busywait logic to avoid wasting cpu cycles (Chris)
> >>>
> >>>Two small caveats as a heads up:
> >>>- the runtime pm wakelock debug stuff catches a few bugs. rpm is disabled
> >>>   by default, but lots enable it (e.g. powertop does), and we iirc have
> >>>   fixes floating for most. If we can't squeeze them all in for 4.5 because
> >>>   too big or late we can just tune down the dmesg noise since the
> >>>   uncovered bugs are all as old as rpm support.
> >>>- softpin is still thrashing around: Chris complains that the ABI can't be
> >>>   used of anything else than beignet, but I think that's ok since easy to
> >>>   remedy and softpin was done primarily for buffered svm opencl mode. And
> >>>   then there's some confusion around canonical 48bit addresses that I
> >>>   don't fully understand myself. I expect Tvrtko to handle this before
> >>>   your merge window pull goes out.
> >>
> >>So just with Tvrtko and the canonical address is something
> >>userspace/beignet will never hit under legitimate usage. So it's just a
> >>bit of closing a corner-case, and the patch+testcase is ready except for
> >>bit of final polish and unfortunately people going on holidays already.
> >
> >Nope, it was reported in the wild and it imposes an ABI constraint on
> >the execobject.offsets.
> 
> Plan is for "drm/i915: Avoid writing relocs with addresses in
> non-canonical form" to be ready as a fixup either before, or
> slightly after rc1. As long as we hit that, slight wobbling in the
> ABI between two release candidates shouldn't be an issue. That is my
> understanding at least.

What about the other ABI issue you just ignored? What about the severe
scaling issues that were known and addressed before you pushed a patch
*out of context*?

> Especially given how the announced user plans to pass in user
> pointer allocated addresses which will already be in canonical
> format.

Not good enough for ABI as the existing code that you just enabled
didn't adhere to the required ABI.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PULL] drm-intel-fixes

[Jani Nikula]

Hi Dave (and/or Linus, depending on the new arrival and eggnog
schedules) -

Here's a batch of i915 fixes all around. It may be slightly bigger than
one would hope for at this stage, but they've all been through testing
in our -next before being picked up for v4.4. Also, I missed Dave's
fixes pull earlier today just because I wanted an extra testing round on
this. So I'm fairly confident.

Wishing you all the things it is customary to wish this time of the
year.


BR,
Jani.

The following changes since commit 4ef7675344d687a0ef5b0d7c0cee12da005870c0:

  Linux 4.4-rc6 (2015-12-20 16:06:09 -0800)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-12-23

for you to fetch changes up to a98728e0bb978fbe9246c93ea89198de612c22e6:

  drm/i915: Correct max delay for HDMI hotplug live status checking (2015-12-22 
13:01:24 +0200)


Chris Wilson (4):
  drm/i915: Set the map-and-fenceable flag for preallocated objects
  drm/i915: Break busywaiting for requests on pending signals
  drm/i915: Limit the busy wait on requests to 5us not 10ms!
  drm/i915: Only spin whilst waiting on the current request

Daniel Vetter (1):
  drm/i915: mdelay(10) considered harmful

Gary Wang (1):
  drm/i915: Correct max delay for HDMI hotplug live status checking

Matt Roper (1):
  drm/i915: Disable primary plane if we fail to reconstruct BIOS fb (v2)

Ville Syrjälä (3):
  drm/i915: Drop the broken cursor base==0 special casing
  drm/i915: Workaround CHV pipe C cursor fail
  drm/i915: Kill intel_crtc->cursor_bo

 drivers/gpu/drm/i915/i915_drv.h|  28 ++---
 drivers/gpu/drm/i915/i915_gem.c| 111 +
 drivers/gpu/drm/i915/i915_gem_gtt.c|   1 +
 drivers/gpu/drm/i915/i915_gem_stolen.c |   1 +
 drivers/gpu/drm/i915/intel_display.c   |  66 +---
 drivers/gpu/drm/i915/intel_drv.h   |   1 -
 drivers/gpu/drm/i915/intel_hdmi.c  |   7 ++-
 7 files changed, 154 insertions(+), 61 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [RFC] drm/i915/bxt: Add pipe_src size property

[Nabendu Maiti]

This patch is adding pipesource size as property as intel property.User
application is allowed to change the pipe source size in runtime on BXT/SKL.
Added the property as inteli crtc property.

Comments and suggestions are requested for whether we can keep the
property as intel crtc property or move to drm level.

Signed-off-by: Nabendu Maiti 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_atomic.c  | 66 
 drivers/gpu/drm/i915/intel_display.c | 46 +++--
 drivers/gpu/drm/i915/intel_drv.h |  9 +
 4 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf7e0fc..d789841 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1867,6 +1867,7 @@ struct drm_i915_private {
 
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+   struct drm_property *crtc_src_size_prop;
 
/* hda/i915 audio component */
struct i915_audio_component *audio_component;
diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 4625f8a..b386ba9 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -73,6 +73,72 @@ intel_connector_atomic_get_property(struct drm_connector 
*connector,
return -EINVAL;
 }
 
+/* intel_crtc_atomic_set_property - duplicate crtc state
+ * @crtc: drm crtc
+ *
+ * Allocates and returns a copy of the crtc state (both common and
+ * Intel-specific) for the specified crtc.
+ *
+ * Returns: The newly allocated crtc state, or NULL on failure.
+ */
+int
+intel_crtc_atomic_set_property(struct drm_crtc *crtc,
+  struct drm_crtc_state *state,
+  struct drm_property *property,
+  uint64_t val)
+{
+   struct drm_device *dev = crtc->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(state);
+   u32 pipe_src_size = ((intel_crtc_state->pipe_src_w << 16) |
+(intel_crtc_state->pipe_src_h));
+   struct drm_display_mode *adjusted_mode =
+   &intel_crtc->config->base.adjusted_mode;
+
+   if (property == dev_priv->crtc_src_size_prop) {
+   if (val != pipe_src_size) {
+   if (val) {
+   intel_crtc_state->pipe_src_w = (val >> 16);
+   intel_crtc_state->pipe_src_h = val & 0x;
+   } else {
+
+   /* for 0 set standard modeset calculated values 
*/
+   intel_crtc_state->pipe_src_w = 
adjusted_mode->hdisplay;
+   intel_crtc_state->pipe_src_h = 
adjusted_mode->vdisplay;
+   }
+   intel_crtc_state->update_pipe = true;
+   }
+   }
+   return 0;
+}
+
+/*
+ * intel_crtc_atomic_get_property - duplicate crtc state
+ * @crtc: drm crtc
+ *
+ * Get Crtc properties.
+ *
+ * Returns: The newly allocated crtc state, or NULL on failure.
+ */
+int
+intel_crtc_atomic_get_property(struct drm_crtc *crtc,
+  const struct drm_crtc_state *state,
+  struct drm_property *property,
+  uint64_t *val)
+{
+   struct drm_device *dev = crtc->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(state);
+   u32 pipe_src_size = ((intel_crtc_state->pipe_src_w << 16) |
+   (intel_crtc_state->pipe_src_h));
+
+   if (property == dev_priv->crtc_src_size_prop) {
+   *val = pipe_src_size;
+   }
+   return 0;
+}
+
 /*
  * intel_crtc_duplicate_state - duplicate crtc state
  * @crtc: drm crtc
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2d0b006..ca4e26b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3340,6 +3340,12 @@ static void intel_update_pipe_config(struct intel_crtc 
*crtc,
 * sized surface.
 */
 
+   /* Restore modeset values */
+   if (!crtc->config->pipe_src_w && crtc->config->pipe_src_h) {
+   crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay;
+   crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay;
+   }
+
I915_WRITE(PIPESRC(crtc->pipe),
   ((pipe_config->pipe_src_w - 1) << 16) |
   (pipe_config->pipe_src_h - 1));
@@ -12068,9 +12074,23 @@ static int intel_crtc_atomic_check(struct drm_crtc 
*crtc,
}
 
if (INTEL_INFO(dev)->gen >= 9) {
-   if (mo

Re: [Intel-gfx] [PATCH v5] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping

[Jani Nikula]

On Thu, 17 Dec 2015, Daniel Vetter  wrote:
> On Thu, Dec 10, 2015 at 05:41:30PM +0100, Takashi Iwai wrote:
>> On Thu, 10 Dec 2015 17:36:04 +0100,
>> Ville Syrjälä wrote:
>> > 
>> > On Fri, Dec 04, 2015 at 04:05:26PM +, Chris Wilson wrote:
>> > > A long time ago (before 3.14) we relied on a permanent pinning of the
>> > > ifbdev to lock the fb in place inside the GGTT. However, the
>> > > introduction of stealing the BIOS framebuffer and reusing its address in
>> > > the GGTT for the fbdev has muddied waters and we use an inherited fb.
>> > > However, the inherited fb is only pinned whilst it is active and we no
>> > > longer have an explicit pin for the info->system_base mmapping used by
>> > > the fbdev. The result is that after some aperture pressure the fbdev may
>> > > be evicted, but we continue to write the fbcon into the same GGTT
>> > > address - overwriting anything else that may be put into that offset.
>> > > The effect is most pronounced across suspend/resume as
>> > > intel_fbdev_set_suspend() does a full clear over the whole scanout.
>> > > 
>> > > v2: Only unpin the intel_fb is we allocate it. If we inherit the fb from
>> > > the BIOS, we do not own the pinned vma (except for the reference we add
>> > > in this patch for our access via info->screen_base).
>> > > 
>> > > v3: Finish balancing the vma pinning for the normal !preallocated case.
>> > > 
>> > > v4: Try to simplify the pinning even further.
>> > > v5: Leak the VMA (cleaned up by object-free) to avoid complicated error 
>> > > paths.
>> > > 
>> > > Signed-off-by: Chris Wilson 
>> > > Cc: "Goel, Akash" 
>> > > Cc: Daniel Vetter 
>> > > Cc: Jesse Barnes 
>> > > Cc: Lukas Wunner 
>> > > Cc: sta...@vger.kernel.org
>> > 
>> > This seems to have fixed my garbled text+fbcon dead after
>> > suspend/hibernate issues. Well, only had the patch in for a day or so,
>> > but so far so good.
>> > 
>> > Tested-by: Ville Syrjälä 
>> > 
>> > Takashi, don't know if you already found this patch, but it's definitely
>> > something you should try as well.
>> 
>> Great, I'll give this a try.  Thanks!
>
> Pulled both patches into dinq. Jani, can you please cherry-pick?

Picked the first, but I don't have the time to fix the conflicts on the
second one.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] RFC drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

With a bit of care (and leniency) we can iterate over the object and
wait for previous rendering to complete with judicial use of atomic
reference counting. The ABI requires us to ensure that an active object
is eventually flushed (like the busy-ioctl) which is guaranteed by our
management of requests (i.e. everything that is submitted to hardware is
flushed in the same request). All we have to do is ensure that we can
detect when the requests are complete for reporting when the object is
idle (without triggering ETIME) - this is handled by
__i915_wait_request.

The biggest danger in the code is walking the object without holding any
locks. We iterate over the set of last requests and carefully grab a
reference upon it. (If it is changing beneath us, that is the usual
userspace race and even with locking you get the same indeterminate
results.) If the request is unreferenced beneath us, it will be disposed
of into the request cache - so we have to carefully order the retrieval
of the request pointer with its removal.

The impact of this is actually quite small - the return to userspace
following the wait was already lockless. What we achieve here is
completing an already finished wait without hitting the struct_mutex,
our hold is quite short and so we are typically just a victim of
contention rather than a cause.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c | 53 +++--
 drivers/gpu/drm/i915/i915_gem_request.c |  2 +-
 drivers/gpu/drm/i915/i915_gem_request.h |  8 -
 3 files changed, 26 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c169574758d5..33de69eff93a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2414,57 +2414,40 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, 
struct drm_file *file)
 {
struct drm_i915_gem_wait *args = data;
struct drm_i915_gem_object *obj;
-   struct drm_i915_gem_request *req[I915_NUM_RINGS];
-   int i, n = 0;
-   int ret;
+   int i, ret = 0;
 
if (args->flags != 0)
return -EINVAL;
 
-   ret = i915_mutex_lock_interruptible(dev);
-   if (ret)
-   return ret;
-
obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->bo_handle));
-   if (&obj->base == NULL) {
-   mutex_unlock(&dev->struct_mutex);
+   if (&obj->base == NULL)
return -ENOENT;
-   }
-
-   /* Need to make sure the object gets inactive eventually. */
-   ret = i915_gem_object_flush_active(obj);
-   if (ret)
-   goto out;
 
if (!obj->active)
goto out;
 
-   /* Do this after OLR check to make sure we make forward progress polling
-* on this IOCTL with a timeout == 0 (like busy ioctl)
-*/
-   if (args->timeout_ns == 0) {
-   ret = -ETIME;
-   goto out;
-   }
-
for (i = 0; i < I915_NUM_RINGS; i++) {
-   if (obj->last_read[i].request == NULL)
+   struct drm_i915_gem_request *req;
+
+reload:
+   req = READ_ONCE(obj->last_read[i].request);
+   if (req == NULL)
continue;
 
-   req[n++] = i915_gem_request_get(obj->last_read[i].request);
+   req = i915_gem_request_get_rcu(req);
+   if (req == NULL)
+   goto reload;
+
+   ret = __i915_wait_request(req, true,
+ args->timeout_ns > 0 ? 
&args->timeout_ns : NULL,
+ to_rps_client(file));
+   i915_gem_request_put(req);
+   if (ret)
+   goto out;
}
 
 out:
-   drm_gem_object_unreference(&obj->base);
-   mutex_unlock(&dev->struct_mutex);
-
-   for (i = 0; i < n; i++) {
-   if (ret == 0)
-   ret = __i915_wait_request(req[i], true,
- args->timeout_ns > 0 ? 
&args->timeout_ns : NULL,
- to_rps_client(file));
-   i915_gem_request_put(req[i]);
-   }
+   drm_gem_object_unreference_unlocked(&obj->base);
return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c 
b/drivers/gpu/drm/i915/i915_gem_request.c
index 4e087143b1d2..a1efaf3063f2 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -335,7 +335,7 @@ static void __i915_gem_request_retire_active(struct 
drm_i915_gem_request *req)
 * and pass along the auxiliary information.
 */
INIT_LIST_HEAD(&active->link);
-   active->request = NULL;
+   smp_store_mb(active->request, NULL);
 
active->retire(active, req);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_reque

Re: [Intel-gfx] [PATCH] RFC drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

On Wed, Dec 23, 2015 at 11:19:23AM +, Chris Wilson wrote:
> With a bit of care (and leniency) we can iterate over the object and
> wait for previous rendering to complete with judicial use of atomic
> reference counting. The ABI requires us to ensure that an active object
> is eventually flushed (like the busy-ioctl) which is guaranteed by our
> management of requests (i.e. everything that is submitted to hardware is
> flushed in the same request). All we have to do is ensure that we can
> detect when the requests are complete for reporting when the object is
> idle (without triggering ETIME) - this is handled by
> __i915_wait_request.
> 
> The biggest danger in the code is walking the object without holding any
> locks. We iterate over the set of last requests and carefully grab a
> reference upon it. (If it is changing beneath us, that is the usual
> userspace race and even with locking you get the same indeterminate
> results.) If the request is unreferenced beneath us, it will be disposed
> of into the request cache - so we have to carefully order the retrieval
> of the request pointer with its removal.
> 
> The impact of this is actually quite small - the return to userspace
> following the wait was already lockless. What we achieve here is
> completing an already finished wait without hitting the struct_mutex,
> our hold is quite short and so we are typically just a victim of
> contention rather than a cause.
> 
> Signed-off-by: Chris Wilson 

Some food for thought. I would especially like someone to poke holes in
the racy pointer lookup and check the store mb() versus the
rcu-reference.

The same technique would also apply to busy-ioctl, and that's about the
limits of its applicablity. Though isolating those from contention and
having to do a flush-active should be beneficial system-wide.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: increase the tries for HDMI hotplug live status checking

[Kumar, Shobhit]

On 12/23/2015 01:41 PM, Gary Wang wrote:

The total delay of HDMI hotplug detecting with 30ms is sometimes not
enoughtfor HDMI live status up with specific HDMI monitors in BSW platform.

After doing experiments for following monitors, it needs 80ms at least
for those worst cases.

Lenovo L246 1xwA (4 failed, necessary hot-plug delay: 58/40/60/40ms)
Philips HH2AP (9 failed, necessary hot-plug delay: 80/50/50/60/46/40/58/58/39ms)
BENQ ET-0035-N (6 failed, necessary hot-plug delay: 60/50/50/80/80/40ms)
DELL U2713HM (2 failed, necessary hot-plug delay: 58/59ms)
HP HP-LP2475w (5 failed, necessary hot-plug delay: 70/50/40/60/40ms)

It looks like 70-80 ms is BSW platform needs in some bad cases of the
monitors at this end (8 times delay at most). Keep less than 100ms for
HDCP pulse HPD low (with at least 100ms) to respond a plug out.



Tested-by: Shobhit Kumar 

Adding
Cc: drm-intel-fi...@lists.freedesktop.org


Reviewed-by: Cooper Chiou 
Tested-by: Gary Wang 
Cc: Gavin Hindman 
Cc: Sonika Jindal 
Cc: Shashank Sharma 
Cc: Shobhit Kumar 
Signed-off-by: Gary Wang 
---
  drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
  mode change 100644 => 100755 drivers/gpu/drm/i915/intel_hdmi.c

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
old mode 100644
new mode 100755
index 6214175..4a77639
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1392,7 +1392,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
force)

intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);

-   for (try = 0; !live_status && try < 4; try++) {
+   for (try = 0; !live_status && try < 9; try++) {
if (try)
msleep(10);
live_status = intel_digital_port_connected(dev_priv,


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

Re: [Intel-gfx] vsync + vaapi question

[Chris Wilson]

On Tue, Dec 22, 2015 at 11:25:53PM +0100, Lukas Hejtmanek wrote:
> On Tue, Dec 22, 2015 at 08:54:08PM +, Chris Wilson wrote:
> > Not really. The Primary output will be used as the vsync so long as a
> > single pixel of the Window is upon it. Fundamentally, we need to use the
> > output that the Window is on in order to driver the vblank update.
> 
> Which window? Any window or the va-api window? In the latter case, it would be
> quite ok to setup left-of instead of clone and move mplayer window solely to
> the external monitor. Do I understand it correctly?

The clipped extents of the va-api Window is used to determine which CRTC
to use as the vblank source. Hmm, the Primary Output is used as the
preference (i.e. if any part of that Window is on the Primary, it is
used as the CRTC - the idea being that Primary is important and this
should share the vblank source for more Windows and prevent arbitrary
jumps between vblank sources as the Window moves).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] RFC drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

On Wed, Dec 23, 2015 at 11:32:23AM +, Chris Wilson wrote:
> On Wed, Dec 23, 2015 at 11:19:23AM +, Chris Wilson wrote:
> > With a bit of care (and leniency) we can iterate over the object and
> > wait for previous rendering to complete with judicial use of atomic
> > reference counting. The ABI requires us to ensure that an active object
> > is eventually flushed (like the busy-ioctl) which is guaranteed by our
> > management of requests (i.e. everything that is submitted to hardware is
> > flushed in the same request). All we have to do is ensure that we can
> > detect when the requests are complete for reporting when the object is
> > idle (without triggering ETIME) - this is handled by
> > __i915_wait_request.
> > 
> > The biggest danger in the code is walking the object without holding any
> > locks. We iterate over the set of last requests and carefully grab a
> > reference upon it. (If it is changing beneath us, that is the usual
> > userspace race and even with locking you get the same indeterminate
> > results.) If the request is unreferenced beneath us, it will be disposed
> > of into the request cache - so we have to carefully order the retrieval
> > of the request pointer with its removal.
> > 
> > The impact of this is actually quite small - the return to userspace
> > following the wait was already lockless. What we achieve here is
> > completing an already finished wait without hitting the struct_mutex,
> > our hold is quite short and so we are typically just a victim of
> > contention rather than a cause.
> > 
> > Signed-off-by: Chris Wilson 
> 
> Some food for thought. I would especially like someone to poke holes in
> the racy pointer lookup and check the store mb() versus the
> rcu-reference.

So what I think is the missing element here is then

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 871aaae1a9d5..4d4ab8e6423f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4193,7 +4193,8 @@ i915_gem_load(struct drm_device *dev)
dev_priv->requests =
kmem_cache_create("i915_gem_request",
  sizeof(struct drm_i915_gem_request), 0,
- SLAB_HWCACHE_ALIGN,
+ SLAB_HWCACHE_ALIGN |
+ SLAB_DESTROY_BY_RCU,
  NULL);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] dim: Add helper command to generate Fixes: lines

[Daniel Vetter]

Unfortunately a simple git alias doesn't work since the linux kernel
wants a sha1 shortened to 12 characters, and the git commit
prettifying can't do that with e.g. %12h. sed to the rescue.

I'm using this when editing commit messages after applying (:read !dim
fixes sha1 in vim).

Signed-off-by: Daniel Vetter 
---
 bash_completion | 4 ++--
 dim | 5 +
 dim.rst | 5 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/bash_completion b/bash_completion
index 7f129ca818eb..e44e5fc844b4 100644
--- a/bash_completion
+++ b/bash_completion
@@ -29,7 +29,7 @@ _dim ()
cmds="$cmds magic-patch mp cd"
cmds="$cmds magic-rebase-resolve mrr"
cmds="$cmds apply-igt ai"
-   cmds="$cmds apply-resolved ar tc check-patch cp cherry-pick"
+   cmds="$cmds apply-resolved ar tc fixes check-patch cp cherry-pick"
cmds="$cmds pull-request pull-request-fixes pull-request-next 
pull-request-next-fixes"
cmds="$cmds update-next"
cmds="$cmds create-branch remove-branch create-workdir 
for-each-workdirs fw"
@@ -73,7 +73,7 @@ _dim ()
COMPREPLY=( $( compgen -o nospace -W "-a" -- 
$cur ) )
fi
;;
-   tc)
+   tc|fixes)
# FIXME needs a git sha1
;;
check-patch|cp)
diff --git a/dim b/dim
index 9ecb95053718..c749cebd1187 100755
--- a/dim
+++ b/dim
@@ -839,6 +839,11 @@ case "$subcommand" in
origin/master | sed 's/^ *//'
fi
;;
+   fixes)
+   sha1=$1
+   git log -1 $sha1 "--pretty=format:Fixes: %H (\"%s\")%n" | \
+   sed -e 's/\([0-f]\{12\}\)[0-f]*/\1/'
+   ;;
check-patch|cp)
dim_checkrange $@
;;
diff --git a/dim.rst b/dim.rst
index 567bcb8bb13e..e37d6630aecb 100644
--- a/dim.rst
+++ b/dim.rst
@@ -177,6 +177,11 @@ tc *commit-ish*
 Print the oldest Linux kernel release or -rc tag that contains the supplied
 *commit-ish*, or, if none do, print the upstream branches that contain it.
 
+fixes *commit-ish*
+---
+Print the Fixes: line for the supplied *commit-ish* in the linux kernel
+CodingStyle approved format.
+
 check-patch|cp [*commit-ish* [.. *commit-ish*]]
 ---
 Runs the given commit range commit-ish..commit-ish through the check tools. If
-- 
2.6.4

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

Re: [Intel-gfx] [PATCH] RFC drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

On Wed, Dec 23, 2015 at 12:05:00PM +, Chris Wilson wrote:
> On Wed, Dec 23, 2015 at 11:32:23AM +, Chris Wilson wrote:
> > On Wed, Dec 23, 2015 at 11:19:23AM +, Chris Wilson wrote:
> > > With a bit of care (and leniency) we can iterate over the object and
> > > wait for previous rendering to complete with judicial use of atomic
> > > reference counting. The ABI requires us to ensure that an active object
> > > is eventually flushed (like the busy-ioctl) which is guaranteed by our
> > > management of requests (i.e. everything that is submitted to hardware is
> > > flushed in the same request). All we have to do is ensure that we can
> > > detect when the requests are complete for reporting when the object is
> > > idle (without triggering ETIME) - this is handled by
> > > __i915_wait_request.
> > > 
> > > The biggest danger in the code is walking the object without holding any
> > > locks. We iterate over the set of last requests and carefully grab a
> > > reference upon it. (If it is changing beneath us, that is the usual
> > > userspace race and even with locking you get the same indeterminate
> > > results.) If the request is unreferenced beneath us, it will be disposed
> > > of into the request cache - so we have to carefully order the retrieval
> > > of the request pointer with its removal.
> > > 
> > > The impact of this is actually quite small - the return to userspace
> > > following the wait was already lockless. What we achieve here is
> > > completing an already finished wait without hitting the struct_mutex,
> > > our hold is quite short and so we are typically just a victim of
> > > contention rather than a cause.
> > > 
> > > Signed-off-by: Chris Wilson 
> > 
> > Some food for thought. I would especially like someone to poke holes in
> > the racy pointer lookup and check the store mb() versus the
> > rcu-reference.
> 
> So what I think is the missing element here is then
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 871aaae1a9d5..4d4ab8e6423f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4193,7 +4193,8 @@ i915_gem_load(struct drm_device *dev)
> dev_priv->requests =
> kmem_cache_create("i915_gem_request",
>   sizeof(struct drm_i915_gem_request), 0,
> - SLAB_HWCACHE_ALIGN,
> + SLAB_HWCACHE_ALIGN |
> + SLAB_DESTROY_BY_RCU,
>   NULL);

And we promptly run into memory exhaustion issues.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] vsync + vaapi question

[Lukas Hejtmanek]

On Wed, Dec 23, 2015 at 12:01:44PM +, Chris Wilson wrote:
> The clipped extents of the va-api Window is used to determine which CRTC
> to use as the vblank source. Hmm, the Primary Output is used as the
> preference (i.e. if any part of that Window is on the Primary, it is
> used as the CRTC - the idea being that Primary is important and this
> should share the vblank source for more Windows and prevent arbitrary
> jumps between vblank sources as the Window moves).

just tested, with xv output, vsync seems to be quite ok. however, with va-api
it not ok on both LVDS and HDMI. HDMI seems to be far worse with va-api (i.e.
out of sync). Maybe a bug in va-api sync code?

-- 
Lukáš Hejtmánek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] RFC drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

On Wed, Dec 23, 2015 at 12:13:15PM +, Chris Wilson wrote:
> On Wed, Dec 23, 2015 at 12:05:00PM +, Chris Wilson wrote:
> > On Wed, Dec 23, 2015 at 11:32:23AM +, Chris Wilson wrote:
> > > On Wed, Dec 23, 2015 at 11:19:23AM +, Chris Wilson wrote:
> > > > With a bit of care (and leniency) we can iterate over the object and
> > > > wait for previous rendering to complete with judicial use of atomic
> > > > reference counting. The ABI requires us to ensure that an active object
> > > > is eventually flushed (like the busy-ioctl) which is guaranteed by our
> > > > management of requests (i.e. everything that is submitted to hardware is
> > > > flushed in the same request). All we have to do is ensure that we can
> > > > detect when the requests are complete for reporting when the object is
> > > > idle (without triggering ETIME) - this is handled by
> > > > __i915_wait_request.
> > > > 
> > > > The biggest danger in the code is walking the object without holding any
> > > > locks. We iterate over the set of last requests and carefully grab a
> > > > reference upon it. (If it is changing beneath us, that is the usual
> > > > userspace race and even with locking you get the same indeterminate
> > > > results.) If the request is unreferenced beneath us, it will be disposed
> > > > of into the request cache - so we have to carefully order the retrieval
> > > > of the request pointer with its removal.
> > > > 
> > > > The impact of this is actually quite small - the return to userspace
> > > > following the wait was already lockless. What we achieve here is
> > > > completing an already finished wait without hitting the struct_mutex,
> > > > our hold is quite short and so we are typically just a victim of
> > > > contention rather than a cause.
> > > > 
> > > > Signed-off-by: Chris Wilson 
> > > 
> > > Some food for thought. I would especially like someone to poke holes in
> > > the racy pointer lookup and check the store mb() versus the
> > > rcu-reference.
> > 
> > So what I think is the missing element here is then
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 871aaae1a9d5..4d4ab8e6423f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4193,7 +4193,8 @@ i915_gem_load(struct drm_device *dev)
> > dev_priv->requests =
> > kmem_cache_create("i915_gem_request",
> >   sizeof(struct drm_i915_gem_request), 0,
> > - SLAB_HWCACHE_ALIGN,
> > + SLAB_HWCACHE_ALIGN |
> > + SLAB_DESTROY_BY_RCU,
> >   NULL);
> 
> And we promptly run into memory exhaustion issues.

And with some carefully placed synchronize_rcu() we may be able to get
the best of both worlds...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 2/2] drm/i915: Check DP no aux transaction bit on link training

[Mika Kahola]

On Tue, 2015-12-22 at 16:53 +0100, Lukas Wunner wrote:
> Hi Mika,
> 
> On Mon, Dec 21, 2015 at 01:39:15PM +0200, Mika Kahola wrote:
> > Check if no AUX transactions are required on DP link training.
> > If this bit is set, we can reuse the known good drive current
> > and pre-emphasis level from the last "full" link training.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 19 +++
> >  drivers/gpu/drm/i915/intel_drv.h  |  1 +
> >  3 files changed, 21 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 6b36d82..3137187 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3852,7 +3852,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> > intel_dp->DP = DP;
> >  }
> >  
> > -static bool
> > +bool
> >  intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >  {
> > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c 
> > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index 59d59be..5fb3f97 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -85,6 +85,25 @@ static bool
> >  intel_dp_reset_link_train(struct intel_dp *intel_dp,
> > uint8_t dp_train_pat)
> >  {
> > +   bool has_dpcd;
> > +   bool no_aux_handshake = false;
> > +
> > +   has_dpcd = intel_dp_get_dpcd(intel_dp);
> > +
> > +   /*
> > +* Source device can try to use drive current and pre-emphasis
> > +* parameters computed by the last "full" link training if the
> > +* DP_NO_AUX_HANDSHAKE_LINK_TRAINING bit is set to 1.
> > +*/
> > +   if (has_dpcd) {
> > +   if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
> > +   no_aux_handshake = (intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
> > +   DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
> > +   }
> > +   }
> > +
> > +   intel_dp->train_set_valid &= no_aux_handshake;
> > +
> 
> If Thierry Reding's patches get merged (posted to dri-devel@ on Dec 14
> with <1450097764-30063-1-git-send-email-thierry.red...@gmail.com>),
> you could just use
> 
>   if (has_dpcd && drm_dp_fast_training_cap(intel_dp->dpcd))
>   intel_dp->train_set_valid &= true;
> 
> Also, if the patches get merged, the no_aux_handshake attribute in dev_priv
> can be dropped (set in intel_edp_init_connector() but never read).
> 
Thank you for the tip. I have missed this Thierry's patch.

Happy Holidays!
-Mika-

> Best regards,
> 
> Lukas
> 
> > DRM_DEBUG_KMS("link training optimization: %s\n",
> >   intel_dp->train_set_valid ? "true" : "false");
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index d523ebb..c36a70c 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1239,6 +1239,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
> > *crc);
> >  bool intel_dp_compute_config(struct intel_encoder *encoder,
> >  struct intel_crtc_state *pipe_config);
> >  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
> > +bool intel_dp_get_dpcd(struct intel_dp *intel_dp);
> >  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port 
> > *intel_dig_port,
> >   bool long_hpd);
> >  void intel_edp_backlight_on(struct intel_dp *intel_dp);
> > -- 
> > 1.9.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

[Intel-gfx] [PULL] topic/drm-misc

[Daniel Vetter]

Hi Dave,

Just one more patch in drm-misc before I disappear into the swiss alps. I
was a bit on the fence whether this should go into -fixes, but then
figured meh. If it blows up in reality (the linked bug is just our QA) we
can always cherry-pick afterwards.

Cheers, Daniel


The following changes since commit e112e593b215c394c0303dbf0534db0928e87967:

  drm: use dev_name as default unique name in drm_dev_alloc() (2015-12-15 
13:56:06 +0100)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2015-12-23

for you to fetch changes up to e112e593b215c394c0303dbf0534db0928e87967:

  drm: use dev_name as default unique name in drm_dev_alloc() (2015-12-15 
13:56:06 +0100)



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [maintainer-tools PATCH 2/2] drm-intel: document new fixes flow

[Jani Nikula]

Signed-off-by: Jani Nikula 
---
 drm-intel-flow.dot |  9 +++---
 drm-intel.rst  | 81 ++
 2 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/drm-intel-flow.dot b/drm-intel-flow.dot
index 0a16a915a49c..cfee83872c37 100644
--- a/drm-intel-flow.dot
+++ b/drm-intel-flow.dot
@@ -77,8 +77,7 @@ strict digraph "drm-intel" {
"intel-gfx" [label="intel-gfx mailing list"]
"internal" [label="internal mailing list"]
 
-   "fixes for current" -> "intel-gfx"
-   "fixes for next" -> "intel-gfx"
+   "fixes" -> "intel-gfx"
"feature patches" -> "intel-gfx"
 
"embargoed patches" -> "internal"
@@ -86,7 +85,7 @@ strict digraph "drm-intel" {
 
"internal" -> "drm-intel-internal" [label="maintainers pick\nalways 
open"]
 
-   "intel-gfx" -> "drm-intel-next-queued" [label="maintainers pick\nalways 
open"]
-   "intel-gfx" -> "drm-intel-fixes" [label="maintainers pick\nrc1..rcN of 
current"]
-   "intel-gfx" -> "drm-intel-next-fixes" [label="maintainers 
pick\n~rc5..release of current"]
+   "intel-gfx" -> "drm-intel-next-queued" [label="committers/maintainers 
pick\nalways open"]
+   "drm-intel-next-queued" -> "drm-intel-fixes" [label="maintainers 
cherry-pick\nrc1..rcN of current" color=blue]
+   "drm-intel-next-queued" -> "drm-intel-next-fixes" [label="maintainers 
cherry-pick\n~rc5..release of current" color=blue]
 }
diff --git a/drm-intel.rst b/drm-intel.rst
index 66654899fed2..0e774047b1b8 100644
--- a/drm-intel.rst
+++ b/drm-intel.rst
@@ -93,7 +93,8 @@ drm-intel-next-fixes (aka "dinf")
 ~
 
 This branch contains drm/i915 specific fixes to drm-next after the drm/i915
-features have been merged there.
+features have been merged there. Fixes are first applied to
+drm-intel-next-queued, and cherry-picked to drm-intel-next-fixes.
 
 Pull requests to Dave are sent as needed, with no particular schedule.
 
@@ -101,13 +102,14 @@ drm-intel-fixes (aka "-fixes")
 ~~
 
 This branch contains fixes to Linus' tree after drm-next has been merged during
-the merge window. The fixes are merged through drm-fixes. Valid from -rc1 to 
the
-kernel release.
+the merge window. Fixes are first applied to drm-intel-next-queued, and
+cherry-picked to drm-intel-fixes. The fixes are then merged through drm-fixes.
+Valid from -rc1 to the kernel release.
 
 Usually Linus releases each -rc on a Sunday, and drm-intel-fixes gets rebased 
on
-that the following Monday. The pull request to Dave for new fixes is typically
-sent on the following Thursday. This is repeated until final release of the
-kernel.
+that the following Monday. Usually this is a fast-forward. The pull request to
+Dave for new fixes is typically sent on the following Thursday. This is 
repeated
+until final release of the kernel.
 
 This is the fastest path to getting fixes to Linus' tree. It is generally for
 the regressions, cc:stable, black screens, GPU hangs only, and should pretty
@@ -130,6 +132,73 @@ flow of the commits to drm-upstream and Linus' tree.
 
 Legend: Green = Linus. Red = drm-upstream. Blue = drm-intel. Black = patches.
 
+Features
+
+
+Features are picked up and pushed to drm-intel-next-queued by committers and
+maintainers. See committer guidelines below for details.
+
+Fixes
+-
+
+Fixes are picked up and pushed to drm-intel-next-queued by committers and
+maintainers, just like any other patches. This is to ensure fixes are pushed in
+a timely manner. Fixes that are relevant for stable, current development
+kernels, or drm-next, will be cherry-picked by maintainers to drm-intel-fixes 
or
+drm-intel-next-fixes.
+
+To make this work, patches should be labeled as fixes (see below), and extra
+care should be put into making fixes the first patches in series, not depending
+on preparatory work or cleanup.
+
+Labeling Fixes Before Pushing
+~
+
+To label fixes that should be cherry-picked to the current -rc development
+kernel or drm-next, the commit message should contain either:
+
+   Cc: drm-intel-fi...@lists.freedesktop.org
+
+or, if the fix is relevant for a released kernel,
+
+   Cc: sta...@vger.kernel.org
+
+If the Cc: was forgotten, you can still reply to the list with that, just like
+any other tags, and they should be picked up by whoever pushes the patch.
+
+The maintainers will cherry-pick labeled patches from drm-intel-next-queued to
+the appropriate branches.
+
+If possible, the commit message should also contain a Fixes: tag as described 
in
+`Documentation/SubmittingPatches
+`_
+to aid the maintainers in identifying the right branch.
+
+Requesting Fixes Cherry-Pick Afterwards
+~~~
+
+It's not uncommon for a patch to have been committed before it's identified as 
a
+fix needing to be

[Intel-gfx] [maintainer-tools PATCH 1/2] drm-intel: add committer guidelines

[Jani Nikula]

Add guidelines to help our committers make the right calls when pushing
patches.

Signed-off-by: Jani Nikula 
---
 drm-intel.rst | 115 +-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/drm-intel.rst b/drm-intel.rst
index c6b0800e2dbc..66654899fed2 100644
--- a/drm-intel.rst
+++ b/drm-intel.rst
@@ -71,7 +71,9 @@ The Upstream i915 Driver Repository
 
 `git://anongit.freedesktop.org/drm-intel`
 
-Maintained by Daniel Vetter and co. Consists mostly of `drivers/gpu/drm/i915`.
+Maintained by Daniel Vetter and Jani Nikula, with several developers also 
having
+commit access for pushing `drm-intel-next-queued`. Consists mostly of
+`drivers/gpu/drm/i915`.
 
 drm-intel-next-queued (aka "dinq")
 ~~
@@ -141,6 +143,117 @@ release. There are no fast paths.
 For predictions on the future merge windows and releases, see
 http://phb-crystal-ball.org/.
 
+Committer Guidelines
+
+
+This section describes the guidelines for pushing patches. Strict rules 
covering
+all cases are impossible write and follow. We put a lot of trust in the sound
+judgement of the people with commit access, and that only develops with
+experience. These guidelines are primarily for the committers to aid in making
+the right decisions, and for others to set their the expectations right.
+
+The short list:
+
+* Only push patches changing `drivers/gpu/drm/i915`.
+
+* Only push patches to `drm-intel-next-queued` branch.
+
+* Ensure certain details are covered, see separate list below.
+
+* You have confidence in the patches you push, proportionate to the complexity
+  and impact they have. The confidence must be explicitly documented with
+  special tags (Reviewed-by, Acked-by, Tested-by, Bugzilla, etc.) in the commit
+  message. This is also your insurance, and you want to have it when the commit
+  comes back haunting you. The complexity and impact are properties of the 
patch
+  that must be justified in the commit message.
+
+* Last but not least, especially when getting started, you can't go wrong with
+  asking or deferring to the maintainers. If you don't feel comfortable pushing
+  a patch for any reason (technical concerns, unresolved or conflicting
+  feedback, management or peer pressure, or anything really) it's best to defer
+  to the maintainers. This is what the maintainers are there for.
+
+* After pushing, please reply to the list that you've done so.
+
+Detail Check List
+-
+
+An inexhaustive list of details to check:
+
+* The patch conforms to `Documentation/SubmittingPatches
+  
`_.
+
+* The commit message is sensible, and includes adequate details and references.
+
+* Bug fixes are clearly indicated as such.
+
+* IGT test cases, if applicable, must be complete and reviewed. Please see
+  `details on testing requirements
+  `_.
+
+* An open source userspace, reviewed and released by the upstream project, must
+  be available for new kernel ABI. Please see `details on upstreaming
+  requirements
+  `_.
+
+* Relevant documentation must be updated as part of the patch or series.
+
+* Patch series builds and is expected to boot every step of the way, i.e. is
+  bisectable.
+
+* Each patch is of a sensible size. A good rule of thumb metric is, would you
+  (or the author) stand a chance to fairly quickly understand what goes wrong 
if
+  the commit is reported to cause a regression?
+
+* `checkpatch.pl` does not complain. (Some of the more subjective warnings may
+  be ignored at the committer's discretion.)
+
+* The patch does not introduce new `sparse` warnings.
+
+On Confidence, Complexity, and Transparency
+---
+
+* Reviewed-by/Acked-by/Tested-by must include the name and email of a real
+  person for transparency. Anyone can give these, and therefore you have to
+  value them according to the merits of the person. Quality matters, not
+  quantity. Be suspicious of rubber stamps.
+
+* Reviewed-by/Acked-by/Tested-by can be asked for and given informally (on the
+  list, IRC, in person, in a meeting) but must be added to the commit.
+
+* Reviewed-by. All patches must be reviewed, no exceptions. Please see
+  "Reviewer's statement of oversight" in `Documentation/SubmittingPatches
+  
`_
+  and `review training
+  `_.
+
+* Acked-by. Indicates acceptance. No reply is not a sign of acceptance, unless
+  you've involved the person (added Cc:, explicitly included in discussion).
+
+* Tested-by. Please solicit separate Tested-by especially from the bug
+  reporters, or pe

Re: [Intel-gfx] [maintainer-tools PATCH 1/2] drm-intel: add committer guidelines

[Jani Nikula]

On Wed, 23 Dec 2015, Jani Nikula  wrote:
> Add guidelines to help our committers make the right calls when pushing
> patches.

Actually just pushed these two anyway; posted the patches for
transparency and for a place for discussion. We can update the docs as
we go.

BR,
Jani.


>
> Signed-off-by: Jani Nikula 
> ---
>  drm-intel.rst | 115 
> +-
>  1 file changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/drm-intel.rst b/drm-intel.rst
> index c6b0800e2dbc..66654899fed2 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -71,7 +71,9 @@ The Upstream i915 Driver Repository
>  
>  `git://anongit.freedesktop.org/drm-intel`
>  
> -Maintained by Daniel Vetter and co. Consists mostly of 
> `drivers/gpu/drm/i915`.
> +Maintained by Daniel Vetter and Jani Nikula, with several developers also 
> having
> +commit access for pushing `drm-intel-next-queued`. Consists mostly of
> +`drivers/gpu/drm/i915`.
>  
>  drm-intel-next-queued (aka "dinq")
>  ~~
> @@ -141,6 +143,117 @@ release. There are no fast paths.
>  For predictions on the future merge windows and releases, see
>  http://phb-crystal-ball.org/.
>  
> +Committer Guidelines
> +
> +
> +This section describes the guidelines for pushing patches. Strict rules 
> covering
> +all cases are impossible write and follow. We put a lot of trust in the sound
> +judgement of the people with commit access, and that only develops with
> +experience. These guidelines are primarily for the committers to aid in 
> making
> +the right decisions, and for others to set their the expectations right.
> +
> +The short list:
> +
> +* Only push patches changing `drivers/gpu/drm/i915`.
> +
> +* Only push patches to `drm-intel-next-queued` branch.
> +
> +* Ensure certain details are covered, see separate list below.
> +
> +* You have confidence in the patches you push, proportionate to the 
> complexity
> +  and impact they have. The confidence must be explicitly documented with
> +  special tags (Reviewed-by, Acked-by, Tested-by, Bugzilla, etc.) in the 
> commit
> +  message. This is also your insurance, and you want to have it when the 
> commit
> +  comes back haunting you. The complexity and impact are properties of the 
> patch
> +  that must be justified in the commit message.
> +
> +* Last but not least, especially when getting started, you can't go wrong 
> with
> +  asking or deferring to the maintainers. If you don't feel comfortable 
> pushing
> +  a patch for any reason (technical concerns, unresolved or conflicting
> +  feedback, management or peer pressure, or anything really) it's best to 
> defer
> +  to the maintainers. This is what the maintainers are there for.
> +
> +* After pushing, please reply to the list that you've done so.
> +
> +Detail Check List
> +-
> +
> +An inexhaustive list of details to check:
> +
> +* The patch conforms to `Documentation/SubmittingPatches
> +  
> `_.
> +
> +* The commit message is sensible, and includes adequate details and 
> references.
> +
> +* Bug fixes are clearly indicated as such.
> +
> +* IGT test cases, if applicable, must be complete and reviewed. Please see
> +  `details on testing requirements
> +  `_.
> +
> +* An open source userspace, reviewed and released by the upstream project, 
> must
> +  be available for new kernel ABI. Please see `details on upstreaming
> +  requirements
> +  `_.
> +
> +* Relevant documentation must be updated as part of the patch or series.
> +
> +* Patch series builds and is expected to boot every step of the way, i.e. is
> +  bisectable.
> +
> +* Each patch is of a sensible size. A good rule of thumb metric is, would you
> +  (or the author) stand a chance to fairly quickly understand what goes 
> wrong if
> +  the commit is reported to cause a regression?
> +
> +* `checkpatch.pl` does not complain. (Some of the more subjective warnings 
> may
> +  be ignored at the committer's discretion.)
> +
> +* The patch does not introduce new `sparse` warnings.
> +
> +On Confidence, Complexity, and Transparency
> +---
> +
> +* Reviewed-by/Acked-by/Tested-by must include the name and email of a real
> +  person for transparency. Anyone can give these, and therefore you have to
> +  value them according to the merits of the person. Quality matters, not
> +  quantity. Be suspicious of rubber stamps.
> +
> +* Reviewed-by/Acked-by/Tested-by can be asked for and given informally (on 
> the
> +  list, IRC, in person, in a meeting) but must be added to the commit.
> +
> +* Reviewed-by. All patches must be reviewed, no exceptions. Please see
> +  "Reviewer's statement of oversight" in `Documentation/SubmittingPat

[Intel-gfx] [PATCH v2 2/3] drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

With a bit of care (and leniency) we can iterate over the object and
wait for previous rendering to complete with judicial use of atomic
reference counting. The ABI requires us to ensure that an active object
is eventually flushed (like the busy-ioctl) which is guaranteed by our
management of requests (i.e. everything that is submitted to hardware is
flushed in the same request). All we have to do is ensure that we can
detect when the requests are complete for reporting when the object is
idle (without triggering ETIME) - this is handled by
__i915_wait_request.

The biggest danger in the code is walking the object without holding any
locks. We iterate over the set of last requests and carefully grab a
reference upon it. (If it is changing beneath us, that is the usual
userspace race and even with locking you get the same indeterminate
results.) If the request is unreferenced beneath us, it will be disposed
of into the request cache - so we have to carefully order the retrieval
of the request pointer with its removal, and to do this we employ RCU on
the request cache and upon the last_request pointer tracking.

The impact of this is actually quite small - the return to userspace
following the wait was already lockless. What we achieve here is
completing an already finished wait without hitting the struct_mutex,
our hold is quite short and so we are typically just a victim of
contention rather than a cause.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c | 55 ++---
 1 file changed, 19 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 696ada3891ed..3e331f7e9d74 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2414,57 +2414,40 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, 
struct drm_file *file)
 {
struct drm_i915_gem_wait *args = data;
struct drm_i915_gem_object *obj;
-   struct drm_i915_gem_request *req[I915_NUM_RINGS];
-   int i, n = 0;
-   int ret;
+   int i, ret = 0;
 
if (args->flags != 0)
return -EINVAL;
 
-   ret = i915_mutex_lock_interruptible(dev);
-   if (ret)
-   return ret;
-
obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->bo_handle));
-   if (&obj->base == NULL) {
-   mutex_unlock(&dev->struct_mutex);
+   if (&obj->base == NULL)
return -ENOENT;
-   }
-
-   /* Need to make sure the object gets inactive eventually. */
-   ret = i915_gem_object_flush_active(obj);
-   if (ret)
-   goto out;
-
-   if (!obj->active)
-   goto out;
 
-   /* Do this after OLR check to make sure we make forward progress polling
-* on this IOCTL with a timeout == 0 (like busy ioctl)
-*/
-   if (args->timeout_ns == 0) {
-   ret = -ETIME;
+   if (!obj->active) /* XXX READ_ONCE(obj->flags) */
goto out;
-   }
 
+   rcu_read_lock();
for (i = 0; i < I915_NUM_RINGS; i++) {
-   if (obj->last_read[i].request == NULL)
+   struct drm_i915_gem_request *req;
+
+   req = i915_gem_active_get_request_rcu(&obj->last_read[i]);
+   if (req == NULL)
continue;
 
-   req[n++] = i915_gem_request_get(obj->last_read[i].request);
+   rcu_read_unlock();
+   ret = __i915_wait_request(req, true,
+ args->timeout_ns > 0 ? 
&args->timeout_ns : NULL,
+ to_rps_client(file));
+   i915_gem_request_put(req);
+   if (ret)
+   goto out;
+
+   rcu_read_lock();
}
+   rcu_read_unlock();
 
 out:
-   drm_gem_object_unreference(&obj->base);
-   mutex_unlock(&dev->struct_mutex);
-
-   for (i = 0; i < n; i++) {
-   if (ret == 0)
-   ret = __i915_wait_request(req[i], true,
- args->timeout_ns > 0 ? 
&args->timeout_ns : NULL,
- to_rps_client(file));
-   i915_gem_request_put(req[i]);
-   }
+   drm_gem_object_unreference_unlocked(&obj->base);
return ret;
 }
 
-- 
2.6.4

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

[Intel-gfx] [PATCH v2 3/3] drm/i915: Remove (struct_mutex) locking for busy-ioctl

[Chris Wilson]

By applying the same logic as for wait-ioctl, we can query whether a
request has completed without holding struct_mutex. The biggest impact
system-wide is removing the flush_active and the contention that causes.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c | 53 +
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3e331f7e9d74..80810d5b5caf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3591,37 +3591,38 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 {
struct drm_i915_gem_busy *args = data;
struct drm_i915_gem_object *obj;
-   int ret;
-
-   ret = i915_mutex_lock_interruptible(dev);
-   if (ret)
-   return ret;
 
obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
-   if (&obj->base == NULL) {
-   ret = -ENOENT;
-   goto unlock;
-   }
+   if (&obj->base == NULL)
+   return -ENOENT;
 
-   /* Count all active objects as busy, even if they are currently not used
-* by the gpu. Users of this interface expect objects to eventually
-* become non-busy without any further actions, therefore emit any
-* necessary flushes here.
-*/
-   ret = i915_gem_object_flush_active(obj);
-   if (ret)
-   goto unref;
+   args->busy = 0;
+   if (obj->active) { /* XXX READ_ONCE(obj->flags) */
+   struct drm_i915_gem_request *req;
+   int i;
 
-   BUILD_BUG_ON(I915_NUM_RINGS > 16);
-   args->busy = obj->active << 16;
-   if (obj->last_write.request)
-   args->busy |= obj->last_write.request->engine->id;
+   BUILD_BUG_ON(I915_NUM_RINGS > 16);
+   rcu_read_lock();
+   for (i = 0; i < I915_NUM_RINGS; i++) {
+   req = 
i915_gem_active_get_request_rcu(&obj->last_read[i]);
+   if (req == NULL)
+   continue;
 
-unref:
-   drm_gem_object_unreference(&obj->base);
-unlock:
-   mutex_unlock(&dev->struct_mutex);
-   return ret;
+   if (i915_gem_request_completed(req))
+   args->busy |= 1 << (16 + i);
+   i915_gem_request_put(req);
+   }
+
+   req = i915_gem_active_get_request_rcu(&obj->last_write);
+   if (req) {
+   args->busy |= req->engine->id;
+   i915_gem_request_put(req);
+   }
+   rcu_read_unlock();
+   }
+
+   drm_gem_object_unreference_unlocked(&obj->base);
+   return 0;
 }
 
 int
-- 
2.6.4

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

[Intel-gfx] [PATCH v2 1/3] drm/i915: Enable lockless lookup of request tracking via RCU

[Chris Wilson]

If we enable RCU for the requests (providing a grace period where we can
inspect a "dead" request before it is freed), we can allow callers to
carefully perform lockless lookup of an active request.

However, by enabling deferred freeing of requests, we can potentially
hog a lot of memory when dealing with tens of thousands of requests per
second - with a quick insertion of the a synchronize_rcu() inside our
shrinker callback, that issue disappears.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c  |  3 ++-
 drivers/gpu/drm/i915/i915_gem_request.c  |  2 +-
 drivers/gpu/drm/i915/i915_gem_request.h  | 24 +++-
 drivers/gpu/drm/i915/i915_gem_shrinker.c |  1 +
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c169574758d5..696ada3891ed 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4222,7 +4222,8 @@ i915_gem_load(struct drm_device *dev)
dev_priv->requests =
kmem_cache_create("i915_gem_request",
  sizeof(struct drm_i915_gem_request), 0,
- SLAB_HWCACHE_ALIGN,
+ SLAB_HWCACHE_ALIGN |
+ SLAB_DESTROY_BY_RCU,
  NULL);
 
INIT_LIST_HEAD(&dev_priv->context_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c 
b/drivers/gpu/drm/i915/i915_gem_request.c
index 4e087143b1d2..380a5963d957 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -335,7 +335,7 @@ static void __i915_gem_request_retire_active(struct 
drm_i915_gem_request *req)
 * and pass along the auxiliary information.
 */
INIT_LIST_HEAD(&active->link);
-   active->request = NULL;
+   rcu_assign_pointer(active->request, NULL);
 
active->retire(active, req);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h 
b/drivers/gpu/drm/i915/i915_gem_request.h
index 4eb15ed9205e..fae85b111a1e 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -139,6 +139,12 @@ i915_gem_request_get(struct drm_i915_gem_request *req)
return to_request(fence_get(&req->fence));
 }
 
+static inline struct drm_i915_gem_request *
+i915_gem_request_get_rcu(struct drm_i915_gem_request *req)
+{
+   return to_request(fence_get_rcu(&req->fence));
+}
+
 static inline void
 i915_gem_request_put(struct drm_i915_gem_request *req)
 {
@@ -243,7 +249,23 @@ i915_gem_request_mark_active(struct drm_i915_gem_request 
*request,
 struct i915_gem_active *active)
 {
list_move(&active->link, &request->active_list);
-   active->request = request;
+   rcu_assign_pointer(active->request, request);
+}
+
+static inline struct drm_i915_gem_request *
+i915_gem_active_get_request_rcu(struct i915_gem_active *active)
+{
+   do {
+   struct drm_i915_gem_request *request;
+
+   request = rcu_dereference(active->request);
+   if (request == NULL)
+   return NULL;
+
+   request = i915_gem_request_get_rcu(request);
+   if (request)
+   return request;
+   } while (1);
 }
 
 #endif /* I915_GEM_REQUEST_H */
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c 
b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index c561ed2b8287..03a8bbb3e31e 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -142,6 +142,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
}
 
i915_gem_retire_requests(dev_priv->dev);
+   synchronize_rcu(); /* expedite the grace period to free the requests */
 
return count;
 }
-- 
2.6.4

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

Re: [Intel-gfx] vsync + vaapi question

[Chris Wilson]

On Wed, Dec 23, 2015 at 01:13:46PM +0100, Lukas Hejtmanek wrote:
> On Wed, Dec 23, 2015 at 12:01:44PM +, Chris Wilson wrote:
> > The clipped extents of the va-api Window is used to determine which CRTC
> > to use as the vblank source. Hmm, the Primary Output is used as the
> > preference (i.e. if any part of that Window is on the Primary, it is
> > used as the CRTC - the idea being that Primary is important and this
> > should share the vblank source for more Windows and prevent arbitrary
> > jumps between vblank sources as the Window moves).
> 
> just tested, with xv output, vsync seems to be quite ok. however, with va-api
> it not ok on both LVDS and HDMI. HDMI seems to be far worse with va-api (i.e.
> out of sync). Maybe a bug in va-api sync code?

Try (libva):

diff --git a/va/x11/dri2_util.c b/va/x11/dri2_util.c
index d076fb3..9ff5e95 100644
--- a/va/x11/dri2_util.c
+++ b/va/x11/dri2_util.c
@@ -95,8 +95,9 @@ dri2SwapBuffer(VADriverContextP ctx, struct dri_drawable 
*dri_drawable)
 if (dri2_drawable->has_backbuffer) {
 if (gsDRI2SwapAvailable) {
 CARD64 ret;
-VA_DRI2SwapBuffers(ctx->native_dpy, dri_drawable->x_drawable, 0, 0,
-   0, &ret);
+VA_DRI2SwapBuffers(ctx->native_dpy, dri_drawable->x_drawable,
+  0, 1, 0,
+  &ret);
 } else {
 xrect.x = 0;
 xrect.y = 0;

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] dim: Add helper command to generate Fixes: lines

[Daniel Vetter]

Unfortunately a simple git alias doesn't work since the linux kernel
wants a sha1 shortened to 12 characters, and the git commit
prettifying can't do that with e.g. %12h. sed to the rescue.

I'm using this when editing commit messages after applying (:read !dim
fixes sha1 in vim).

v2: Also update documentation.

Signed-off-by: Daniel Vetter 
---
 bash_completion |  4 ++--
 dim |  5 +
 dim.rst |  5 +
 drm-intel.rst   | 12 ++--
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/bash_completion b/bash_completion
index 7f129ca818eb..e44e5fc844b4 100644
--- a/bash_completion
+++ b/bash_completion
@@ -29,7 +29,7 @@ _dim ()
cmds="$cmds magic-patch mp cd"
cmds="$cmds magic-rebase-resolve mrr"
cmds="$cmds apply-igt ai"
-   cmds="$cmds apply-resolved ar tc check-patch cp cherry-pick"
+   cmds="$cmds apply-resolved ar tc fixes check-patch cp cherry-pick"
cmds="$cmds pull-request pull-request-fixes pull-request-next 
pull-request-next-fixes"
cmds="$cmds update-next"
cmds="$cmds create-branch remove-branch create-workdir 
for-each-workdirs fw"
@@ -73,7 +73,7 @@ _dim ()
COMPREPLY=( $( compgen -o nospace -W "-a" -- 
$cur ) )
fi
;;
-   tc)
+   tc|fixes)
# FIXME needs a git sha1
;;
check-patch|cp)
diff --git a/dim b/dim
index 9ecb95053718..c749cebd1187 100755
--- a/dim
+++ b/dim
@@ -839,6 +839,11 @@ case "$subcommand" in
origin/master | sed 's/^ *//'
fi
;;
+   fixes)
+   sha1=$1
+   git log -1 $sha1 "--pretty=format:Fixes: %H (\"%s\")%n" | \
+   sed -e 's/\([0-f]\{12\}\)[0-f]*/\1/'
+   ;;
check-patch|cp)
dim_checkrange $@
;;
diff --git a/dim.rst b/dim.rst
index 567bcb8bb13e..e37d6630aecb 100644
--- a/dim.rst
+++ b/dim.rst
@@ -177,6 +177,11 @@ tc *commit-ish*
 Print the oldest Linux kernel release or -rc tag that contains the supplied
 *commit-ish*, or, if none do, print the upstream branches that contain it.
 
+fixes *commit-ish*
+---
+Print the Fixes: line for the supplied *commit-ish* in the linux kernel
+CodingStyle approved format.
+
 check-patch|cp [*commit-ish* [.. *commit-ish*]]
 ---
 Runs the given commit range commit-ish..commit-ish through the check tools. If
diff --git a/drm-intel.rst b/drm-intel.rst
index 0e774047b1b8..6b3b469e1dc6 100644
--- a/drm-intel.rst
+++ b/drm-intel.rst
@@ -163,8 +163,16 @@ or, if the fix is relevant for a released kernel,
 
Cc: sta...@vger.kernel.org
 
-If the Cc: was forgotten, you can still reply to the list with that, just like
-any other tags, and they should be picked up by whoever pushes the patch.
+If your patch fixes a regression then please include a Fixes: line to help
+maintainers where to cherry-pick a patch to. This also extremely useful for
+product groups to know which bugfixes they must include. To follow the
+recommended format please generate the Fixes: line using
+
+$ dim fixes $regressing_commit
+
+If the Cc: or Fixes: was forgotten, you can still reply to the list with that,
+just like any other tags, and they should be picked up by whoever pushes the
+patch.
 
 The maintainers will cherry-pick labeled patches from drm-intel-next-queued to
 the appropriate branches.
-- 
2.6.4

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

Re: [Intel-gfx] [PATCH] dim: Add helper command to generate Fixes: lines

[Daniel Vetter]

On Wed, Dec 23, 2015 at 02:50:41PM +0100, Daniel Vetter wrote:
> Unfortunately a simple git alias doesn't work since the linux kernel
> wants a sha1 shortened to 12 characters, and the git commit
> prettifying can't do that with e.g. %12h. sed to the rescue.
> 
> I'm using this when editing commit messages after applying (:read !dim
> fixes sha1 in vim).
> 
> v2: Also update documentation.
> 
> Signed-off-by: Daniel Vetter 

Bit more explanation: Since we now push everything to dinq releases (and
backports, but that's not new) happen asynchronously and it's harder to
predict when a regression fix will land. Always adding the Fixes: line
will allow maintainers to add Cc: stable if needed, in case a patch missed
the last -fixes pull train before the release by a few days.
-Daniel

> ---
>  bash_completion |  4 ++--
>  dim |  5 +
>  dim.rst |  5 +
>  drm-intel.rst   | 12 ++--
>  4 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/bash_completion b/bash_completion
> index 7f129ca818eb..e44e5fc844b4 100644
> --- a/bash_completion
> +++ b/bash_completion
> @@ -29,7 +29,7 @@ _dim ()
>   cmds="$cmds magic-patch mp cd"
>   cmds="$cmds magic-rebase-resolve mrr"
>   cmds="$cmds apply-igt ai"
> - cmds="$cmds apply-resolved ar tc check-patch cp cherry-pick"
> + cmds="$cmds apply-resolved ar tc fixes check-patch cp cherry-pick"
>   cmds="$cmds pull-request pull-request-fixes pull-request-next 
> pull-request-next-fixes"
>   cmds="$cmds update-next"
>   cmds="$cmds create-branch remove-branch create-workdir 
> for-each-workdirs fw"
> @@ -73,7 +73,7 @@ _dim ()
>   COMPREPLY=( $( compgen -o nospace -W "-a" -- 
> $cur ) )
>   fi
>   ;;
> - tc)
> + tc|fixes)
>   # FIXME needs a git sha1
>   ;;
>   check-patch|cp)
> diff --git a/dim b/dim
> index 9ecb95053718..c749cebd1187 100755
> --- a/dim
> +++ b/dim
> @@ -839,6 +839,11 @@ case "$subcommand" in
>   origin/master | sed 's/^ *//'
>   fi
>   ;;
> + fixes)
> + sha1=$1
> + git log -1 $sha1 "--pretty=format:Fixes: %H (\"%s\")%n" | \
> + sed -e 's/\([0-f]\{12\}\)[0-f]*/\1/'
> + ;;
>   check-patch|cp)
>   dim_checkrange $@
>   ;;
> diff --git a/dim.rst b/dim.rst
> index 567bcb8bb13e..e37d6630aecb 100644
> --- a/dim.rst
> +++ b/dim.rst
> @@ -177,6 +177,11 @@ tc *commit-ish*
>  Print the oldest Linux kernel release or -rc tag that contains the supplied
>  *commit-ish*, or, if none do, print the upstream branches that contain it.
>  
> +fixes *commit-ish*
> +---
> +Print the Fixes: line for the supplied *commit-ish* in the linux kernel
> +CodingStyle approved format.
> +
>  check-patch|cp [*commit-ish* [.. *commit-ish*]]
>  ---
>  Runs the given commit range commit-ish..commit-ish through the check tools. 
> If
> diff --git a/drm-intel.rst b/drm-intel.rst
> index 0e774047b1b8..6b3b469e1dc6 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -163,8 +163,16 @@ or, if the fix is relevant for a released kernel,
>  
>   Cc: sta...@vger.kernel.org
>  
> -If the Cc: was forgotten, you can still reply to the list with that, just 
> like
> -any other tags, and they should be picked up by whoever pushes the patch.
> +If your patch fixes a regression then please include a Fixes: line to help
> +maintainers where to cherry-pick a patch to. This also extremely useful for
> +product groups to know which bugfixes they must include. To follow the
> +recommended format please generate the Fixes: line using
> +
> +$ dim fixes $regressing_commit
> +
> +If the Cc: or Fixes: was forgotten, you can still reply to the list with 
> that,
> +just like any other tags, and they should be picked up by whoever pushes the
> +patch.
>  
>  The maintainers will cherry-pick labeled patches from drm-intel-next-queued 
> to
>  the appropriate branches.
> -- 
> 2.6.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] igt_core: Fix logging to display extended line

[Derek Morton]

line[strlen(line)] will always evaluate to NULL so line_continuation
was always true. That prevented the program name, pid and log level
ever being printed.
Changed to [strlen(line) - 1] so the last character before the null
terminator is compared with '\n' to determine line_continuation.

Signed-off-by: Derek Morton 
---
 lib/igt_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 84cf8d2..221ed7e 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1748,7 +1748,7 @@ void igt_vlog(const char *domain, enum igt_log_level 
level, const char *format,
goto out;
}
 
-   line_continuation = line[strlen(line)] != '\n';
+   line_continuation = line[strlen(line) - 1] != '\n';
 
/* append log buffer */
_igt_log_buffer_append(formatted_line);
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v4] drm/i915/guc: Move GuC wq_check_space to alloc_request_extras

[Dave Gordon]

On 16/12/15 19:45, yu@intel.com wrote:

From: Alex Dai 

Split GuC work queue space checking from submission and move it to
ring_alloc_request_extras. The reason is that failure in later
i915_add_request() won't be handled. In the case timeout happens,
driver can return early in order to handle the error.

v1: Move wq_reserve_space to ring_reserve_space
v2: Move wq_reserve_space to alloc_request_extras (Chris Wilson)
v3: The work queue head pointer is cached by driver now. So we can
 quickly return if space is available.
 s/reserve/check/g (Dave Gordon)
v4: Update cached wq head after ring doorbell; check wq space before
 ring doorbell in case unexpected error happens; call wq space
 check only when GuC submission is enabled. (Dave Gordon)

Signed-off-by: Alex Dai 


LGTM.
Reviewed-by: Dave Gordon 


diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index ef20071..7554d16 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -247,6 +247,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
db_exc.cookie = 1;
}

+   /* Finally, update the cached copy of the GuC's WQ head */
+   gc->wq_head = desc->head;
+
kunmap_atomic(base);
return ret;
  }
@@ -472,28 +475,30 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
 sizeof(desc) * client->ctx_index);
  }

-/* Get valid workqueue item and return it back to offset */
-static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset)
+int i915_guc_wq_check_space(struct i915_guc_client *gc)
  {
struct guc_process_desc *desc;
void *base;
u32 size = sizeof(struct guc_wq_item);
int ret = -ETIMEDOUT, timeout_counter = 200;

+   if (!gc)
+   return 0;
+
+   /* Quickly return if wq space is available since last time we cache the
+* head position. */
+   if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size)
+   return 0;
+
base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0));
desc = base + gc->proc_desc_offset;

while (timeout_counter-- > 0) {
-   if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= size) {
-   *offset = gc->wq_tail;
+   gc->wq_head = desc->head;

-   /* advance the tail for next workqueue item */
-   gc->wq_tail += size;
-   gc->wq_tail &= gc->wq_size - 1;
-
-   /* this will break the loop */
-   timeout_counter = 0;
+   if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size) {
ret = 0;
+   break;
}

if (timeout_counter)
@@ -511,12 +516,16 @@ static int guc_add_workqueue_item(struct i915_guc_client 
*gc,
enum intel_ring_id ring_id = rq->ring->id;
struct guc_wq_item *wqi;
void *base;
-   u32 tail, wq_len, wq_off = 0;
-   int ret;
+   u32 tail, wq_len, wq_off, space;
+
+   space = CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size);
+   if (WARN_ON(space < sizeof(struct guc_wq_item)))
+   return -ENOSPC; /* shouldn't happen */

-   ret = guc_get_workqueue_space(gc, &wq_off);
-   if (ret)
-   return ret;
+   /* postincrement WQ tail for next time */
+   wq_off = gc->wq_tail;
+   gc->wq_tail += sizeof(struct guc_wq_item);
+   gc->wq_tail &= gc->wq_size - 1;

/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
 * should not have the case where structure wqi is across page, neither
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 0e048bf..5cf555d 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -43,6 +43,7 @@ struct i915_guc_client {
uint32_t wq_offset;
uint32_t wq_size;
uint32_t wq_tail;
+   uint32_t wq_head;

/* GuC submission statistics & status */
uint64_t submissions[I915_NUM_RINGS];
@@ -123,5 +124,6 @@ int i915_guc_submit(struct i915_guc_client *client,
struct drm_i915_gem_request *rq);
  void i915_guc_submission_disable(struct drm_device *dev);
  void i915_guc_submission_fini(struct drm_device *dev);
+int i915_guc_wq_check_space(struct i915_guc_client *client);

  #endif
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 272f36f..cd232d2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -667,6 +667,19 @@ int intel_logical_ring_alloc_request_extras(struct 
drm_i915_gem_request *request
return ret;
}

+   if (i915.enable_guc_submission) {
+   /*
+* Check that the GuC has spac

Re: [Intel-gfx] [PULL] drm-intel-fixes

[Linus Torvalds]

On Wed, Dec 23, 2015 at 2:40 AM, Jani Nikula  wrote:
>
> Hi Dave (and/or Linus, depending on the new arrival and eggnog
> schedules) -

I'll pull it directly. Dave just sent me his pending drm fixes, he may
or may not be around any more, it's already christmas eve down under.

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

[Intel-gfx] Suspend To RAM failure in >= 4.1 - bissected to "drm/i915: Track GEN6 page table usage"

[Sylvain Munaut]

Hi,


When trying to upgrade my kernel yesterday to the latest 4.3.3 I
noticed that the suspend to ram was not working. Basically it goes to
sleep but never wakes up. It seems to power up but no screen, not
available through ssh either and afaict nothing runs afterwards.

I first tried a couple official release to see where it broke and I
found that 4.0.9 was working fine, but 4.1.15 was not.

I then ran a git bissect between v4.0 and v4.1 from Linus's tree and
found the "guilty" commit was

commit 678d96fbb3b5995a2fdff2bca5e1ab4a40b7e968
Author: Ben Widawsky 
Date:   Mon Mar 16 16:00:56 2015 +

drm/i915: Track GEN6 page table usage


Anyone know how to fix that ?


Here's the fulbisectl log :

git bisect start
# bad: [b953c0d234bc72e8489d3bf51a276c5c4ec85345] Linux 4.1
git bisect bad b953c0d234bc72e8489d3bf51a276c5c4ec85345
# good: [39a8804455fb23f09157341d3ba7db6d7ae6ee76] Linux 4.0
git bisect good 39a8804455fb23f09157341d3ba7db6d7ae6ee76
# good: [d0a3997c0c3f9351e24029349dee65dd1d9e8d84] Merge tag
'sound-4.1-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good d0a3997c0c3f9351e24029349dee65dd1d9e8d84
# bad: [cf82f52d3619d2e15c83ec9a03c6ce8cdf6c6b58] watchdog:
stmp3xxx_rtc_wdt: fix broken email address
git bisect bad cf82f52d3619d2e15c83ec9a03c6ce8cdf6c6b58
# good: [79319a052cb0ae862954fe9f6e606417f1698ddb] Merge tag
'iommu-updates-v4.1' of
git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu
git bisect good 79319a052cb0ae862954fe9f6e606417f1698ddb
# bad: [8f443e2372ba23d51ee365974f54507acd6f69d1] Revert "ocfs2:
incorrect check for debugfs returns"
git bisect bad 8f443e2372ba23d51ee365974f54507acd6f69d1
# bad: [3165c074175cddab1dcfd553042ea4f363bc76e7] drm/i915: Use atomic
state in intel_ddi_crtc_get_new_encoder()
git bisect bad 3165c074175cddab1dcfd553042ea4f363bc76e7
# good: [8dd0eb3566711d81bfbe2b4421b33f0dd723cec4] Merge tag
'drm-intel-next-2015-02-27' of git://anongit.freedesktop.org/drm-intel
into drm-next
git bisect good 8dd0eb3566711d81bfbe2b4421b33f0dd723cec4
# good: [5704195c3f3c04a00c16334a033b180f16db1f94] drm/i915/skl:
Updated the gen6_set_rps function
git bisect good 5704195c3f3c04a00c16334a033b180f16db1f94
# good: [07749ef32c4fd60334c2451739460dd1cf600281] drm/i915: page
table generalizations
git bisect good 07749ef32c4fd60334c2451739460dd1cf600281
# bad: [58072ccbb81c6f2d67c5b4cc7597707c4fb86a5e] drm/i915: fix race
when clearing RPS IIR bits
git bisect bad 58072ccbb81c6f2d67c5b4cc7597707c4fb86a5e
# bad: [48fe4691ae639e60fda37faf06dccdff60245149] drm/i915: Eliminate
plane control register RMW from sprite code
git bisect bad 48fe4691ae639e60fda37faf06dccdff60245149
# bad: [bdd7554d568fa165b0e86fc32b1cde3c895ff774] drm/i915: Kill
intel_plane->obj
git bisect bad bdd7554d568fa165b0e86fc32b1cde3c895ff774
# bad: [678d96fbb3b5995a2fdff2bca5e1ab4a40b7e968] drm/i915: Track GEN6
page table usage
git bisect bad 678d96fbb3b5995a2fdff2bca5e1ab4a40b7e968
# good: [317b4e903636305cfe702ab3e5b3d68547a69e72] drm/i915: Extract
context switch skip and add pd load logic
git bisect good 317b4e903636305cfe702ab3e5b3d68547a69e72
# first bad commit: [678d96fbb3b5995a2fdff2bca5e1ab4a40b7e968]
drm/i915: Track GEN6 page table usage


The machine is a Lenovo T440s laptop :


00:00.0 Host bridge: Intel Corporation Haswell-ULT DRAM Controller (rev 0b)
00:02.0 VGA compatible controller: Intel Corporation Haswell-ULT
Integrated Graphics Controller (rev 0b)
00:03.0 Audio device: Intel Corporation Haswell-ULT HD Audio Controller (rev 0b)
00:14.0 USB controller: Intel Corporation 8 Series USB xHCI HC (rev 04)
00:16.0 Communication controller: Intel Corporation 8 Series HECI #0 (rev 04)
00:16.3 Serial controller: Intel Corporation 8 Series HECI KT (rev 04)
00:19.0 Ethernet controller: Intel Corporation Ethernet Connection
I218-LM (rev 04)
00:1b.0 Audio device: Intel Corporation 8 Series HD Audio Controller (rev 04)
00:1c.0 PCI bridge: Intel Corporation 8 Series PCI Express Root Port 6 (rev e4)
00:1c.1 PCI bridge: Intel Corporation 8 Series PCI Express Root Port 3 (rev e4)
00:1c.4 PCI bridge: Intel Corporation 8 Series PCI Express Root Port 5 (rev e4)
00:1d.0 USB controller: Intel Corporation 8 Series USB EHCI #1 (rev 04)
00:1f.0 ISA bridge: Intel Corporation 8 Series LPC Controller (rev 04)
00:1f.2 SATA controller: Intel Corporation 8 Series SATA Controller 1
[AHCI mode] (rev 04)
00:1f.3 SMBus: Intel Corporation 8 Series SMBus Controller (rev 04)
02:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd.
RTS5227 PCI Express Card Reader (rev 01)
03:00.0 Network controller: Intel Corporation Wireless 7260 (rev 83)
04:00.0 VGA compatible controller: NVIDIA Corporation GK208M [GeForce
GT 730M] (rev a1)


Cheers,

Sylvain Munaut


(sorry if it's a dupe, seems first message didn't go through because I
wasn't subscribed to the list)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/int

Re: [Intel-gfx] [PULL] drm-intel-fixes

[Linus Torvalds]

On Wed, Dec 23, 2015 at 2:40 AM, Jani Nikula  wrote:
>
>
>   git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-12-23

Btw, since you're already using tags, mind using *signed* tags
instead? It's just good housekeeping..

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

[Intel-gfx] [PATCH v2, 3/4] drm/i915: simplify allocation of driver-internal requests

[Dave Gordon]

There are a number of places where the driver needs a request, but isn't
working on behalf of any specific user or in a specific context. At
present, we associate them with the per-engine default context. A future
patch will abolish those per-engine context pointers; but we can already
eliminate a lot of the references to them just by funneling all such
allocations through a single function, which will mean that the callers
don't need to know anything about how an appropriate context is found.

So this patch provides a shorthand for doing such request allocations
and changes all such calls to use the new function.

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/i915_gem.c  | 39 +++-
 drivers/gpu/drm/i915/intel_display.c |  6 --
 drivers/gpu/drm/i915/intel_overlay.c | 24 +++---
 4 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4fb15f5..56d149e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2269,6 +2269,8 @@ struct drm_i915_gem_request {
 int i915_gem_request_alloc(struct intel_engine_cs *ring,
   struct intel_context *ctx,
   struct drm_i915_gem_request **req_out);
+struct drm_i915_gem_request * __must_check
+   i915_gem_request_alloc_anon(struct intel_engine_cs *ring);
 void i915_gem_request_cancel(struct drm_i915_gem_request *req);
 void i915_gem_request_free(struct kref *req_ref);
 int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d9a3304..927ebc8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2751,6 +2751,22 @@ err:
return ret;
 }
 
+/*
+ * Allocate a request associated with the default context for the given
+ * ring. This can be used where the driver needs a request for internal
+ * purposes not directly related to a user batch submission.
+ */
+struct drm_i915_gem_request *
+i915_gem_request_alloc_anon(struct intel_engine_cs *ring)
+{
+   struct drm_i915_private *dev_priv = to_i915(ring->dev);
+   struct drm_i915_gem_request *req;
+   int err;
+
+   err = i915_gem_request_alloc(ring, ring->default_context, &req);
+   return err ? ERR_PTR(err) : req;
+}
+
 void i915_gem_request_cancel(struct drm_i915_gem_request *req)
 {
intel_ring_reserved_space_cancel(req->ringbuf);
@@ -3168,9 +3184,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj,
return 0;
 
if (*to_req == NULL) {
-   ret = i915_gem_request_alloc(to, to->default_context, 
to_req);
-   if (ret)
-   return ret;
+   struct drm_i915_gem_request *req;
+
+   req = i915_gem_request_alloc_anon(to);
+   if (IS_ERR(req))
+   return PTR_ERR(req);
+
+   *to_req = req;
}
 
trace_i915_gem_ring_sync_to(*to_req, from, from_req);
@@ -3370,9 +3390,9 @@ int i915_gpu_idle(struct drm_device *dev)
if (!i915.enable_execlists) {
struct drm_i915_gem_request *req;
 
-   ret = i915_gem_request_alloc(ring, 
ring->default_context, &req);
-   if (ret)
-   return ret;
+   req = i915_gem_request_alloc_anon(ring);
+   if (IS_ERR(req))
+   return PTR_ERR(req);
 
ret = i915_switch_context(req);
if (ret) {
@@ -4867,10 +4887,9 @@ i915_gem_init_hw(struct drm_device *dev)
for_each_ring(ring, dev_priv, i) {
struct drm_i915_gem_request *req;
 
-   WARN_ON(!ring->default_context);
-
-   ret = i915_gem_request_alloc(ring, ring->default_context, &req);
-   if (ret) {
+   req = i915_gem_request_alloc_anon(ring);
+   if (IS_ERR(req)) {
+   ret = PTR_ERR(req);
i915_gem_cleanup_ringbuffer(dev);
goto out;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index abd2d29..5716f4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
obj->last_write_req);
} else {
if (!request) {
-   ret = i915_gem_request_alloc(ring, 
ring->default_context, &request);
-   if (ret)
+   request = i915_gem_request_alloc_anon(ring);
+   

[Intel-gfx] [PATCH v2, 1/4] drm/i915: remove intel_context::file_priv, add flag for default context

[Dave Gordon]

The file_priv member was used in only one place, where we already had
the file_priv and therefore didn't need to derive it from the context
(in fact, we just found the context by reference to the file_priv).
So by fixing up that one use, we can drop file_priv entirely.

OTOH sometimes we need to identify the global (default) context, which
belongs to the driver as whole and is not associated with any user file
descriptor, so here we add a flag for that specific purpose. This will
then be used instead of the current unsafe practice of check each engine
to see whether its default_context pointer refers back to the context
being operated on. In a subsequent patch, these backpointers will go
away too :)

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_drv.h |  3 ++-
 drivers/gpu/drm/i915/i915_gem_context.c | 22 --
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9b82c45..4fb15f5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -854,6 +854,7 @@ struct i915_ctx_hang_stats {
  * @ref: reference count.
  * @user_handle: userspace tracking identity for this context.
  * @remap_slice: l3 row remapping information.
+ * @is_default: true iff this is the global default context
  * @flags: context specific flags:
  * CONTEXT_NO_ZEROMAP: do not allow mapping things to page 0.
  * @file_priv: filp associated with this context (NULL for global default
@@ -872,9 +873,9 @@ struct intel_context {
struct kref ref;
int user_handle;
uint8_t remap_slice;
+   uint8_t is_global_default;
struct drm_i915_private *i915;
int flags;
-   struct drm_i915_file_private *file_priv;
struct i915_ctx_hang_stats hang_stats;
struct i915_hw_ppgtt *ppgtt;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 900ffd0..43437af 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -236,16 +236,17 @@ __create_hw_context(struct drm_device *dev,
}
 
/* Default context will never have a file_priv */
-   if (file_priv != NULL) {
+   if (file_priv == NULL) {
+   ctx->is_global_default = true;
+   ret = DEFAULT_CONTEXT_HANDLE;
+   } else {
ret = idr_alloc(&file_priv->context_idr, ctx,
DEFAULT_CONTEXT_HANDLE, 0, GFP_KERNEL);
if (ret < 0)
goto err_out;
-   } else
-   ret = DEFAULT_CONTEXT_HANDLE;
-
-   ctx->file_priv = file_priv;
+   }
ctx->user_handle = ret;
+
/* NB: Mark all slices as needing a remap so that when the context first
 * loads it will restore whatever remap state already exists. If there
 * is no remap info, it will be a NOP. */
@@ -269,7 +270,6 @@ static struct intel_context *
 i915_gem_create_context(struct drm_device *dev,
struct drm_i915_file_private *file_priv)
 {
-   const bool is_global_default_ctx = file_priv == NULL;
struct intel_context *ctx;
int ret = 0;
 
@@ -279,8 +279,9 @@ i915_gem_create_context(struct drm_device *dev,
if (IS_ERR(ctx))
return ctx;
 
-   if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
-   /* We may need to do things with the shrinker which
+   if (ctx->is_global_default && ctx->legacy_hw_ctx.rcs_state) {
+   /*
+* We may need to do things with the shrinker which
 * require us to immediately switch back to the default
 * context. This can cause a problem as pinning the
 * default context also requires GTT space which may not
@@ -313,7 +314,7 @@ i915_gem_create_context(struct drm_device *dev,
return ctx;
 
 err_unpin:
-   if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
+   if (ctx->is_global_default && ctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
 err_destroy:
idr_remove(&file_priv->context_idr, ctx->user_handle);
@@ -381,6 +382,7 @@ int i915_gem_context_init(struct drm_device *dev)
}
}
 
+   /* NULL second parameter indicates this is the global default context */
ctx = i915_gem_create_context(dev, NULL);
if (IS_ERR(ctx)) {
DRM_ERROR("Failed to create default global context (error 
%ld)\n",
@@ -896,7 +898,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, 
void *data,
return PTR_ERR(ctx);
}
 
-   idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
+   idr_remove(&file_priv->context_idr, ctx->user_handle);
i915_gem_context_unreference(ctx);
mutex_unlock(&dev->struct_mutex);
 
-- 
1.9.1

__

[Intel-gfx] [PATCH v2, 2/4] drm/i915: simplify testing for the global default context

[Dave Gordon]

There are quite a number of places where the driver tests whether
a given context is or is not the global default context, usually by
checking whether an engine's default_pointer points to the context.
Now that we have a 'is_global_default' flag in the context itself,
all these tests these can be rewritten to use it. This makes the
logic more obvious, and usually saves at least one memory reference.
In addition, with these uses eliminated, a future patch will be able
to get rid of engine::default_context entirely.

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 15 +-
 drivers/gpu/drm/i915/i915_gem.c |  6 ++
 drivers/gpu/drm/i915/intel_lrc.c| 40 +
 3 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0fc38bb..5e6e02c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1942,11 +1942,8 @@ static int i915_context_status(struct seq_file *m, void 
*unused)
 
seq_puts(m, "HW context ");
describe_ctx(m, ctx);
-   for_each_ring(ring, dev_priv, i) {
-   if (ring->default_context == ctx)
-   seq_printf(m, "(default context %s) ",
-  ring->name);
-   }
+   if (ctx->is_global_default)
+   seq_printf(m, "(default context) ");
 
if (i915.enable_execlists) {
seq_putc(m, '\n');
@@ -2037,13 +2034,11 @@ static int i915_dump_lrc(struct seq_file *m, void 
*unused)
if (ret)
return ret;
 
-   list_for_each_entry(ctx, &dev_priv->context_list, link) {
-   for_each_ring(ring, dev_priv, i) {
-   if (ring->default_context != ctx)
+   list_for_each_entry(ctx, &dev_priv->context_list, link)
+   if (!ctx->is_global_default)
+   for_each_ring(ring, dev_priv, i)
i915_dump_lrc_obj(m, ring,
  ctx->engine[i].state);
-   }
-   }
 
mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6c60e04..d9a3304 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2677,10 +2677,8 @@ void i915_gem_request_free(struct kref *req_ref)
i915_gem_request_remove_from_client(req);
 
if (ctx) {
-   if (i915.enable_execlists) {
-   if (ctx != req->ring->default_context)
-   intel_lr_context_unpin(req);
-   }
+   if (i915.enable_execlists && !ctx->is_global_default)
+   intel_lr_context_unpin(req);
 
i915_gem_context_unreference(ctx);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3aa6147..cf76d28 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -571,7 +571,7 @@ static int execlists_context_queue(struct 
drm_i915_gem_request *request)
struct drm_i915_gem_request *cursor;
int num_elements = 0;
 
-   if (request->ctx != ring->default_context)
+   if (!request->ctx->is_global_default)
intel_lr_context_pin(request);
 
i915_gem_request_reference(request);
@@ -660,17 +660,14 @@ static int execlists_move_to_gpu(struct 
drm_i915_gem_request *req,
 
 int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request 
*request)
 {
-   int ret;
+   int ret = 0;
 
request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
 
-   if (request->ctx != request->ring->default_context) {
+   if (!request->ctx->is_global_default)
ret = intel_lr_context_pin(request);
-   if (ret)
-   return ret;
-   }
 
-   return 0;
+   return ret;
 }
 
 static int logical_ring_wait_for_space(struct drm_i915_gem_request *req,
@@ -967,7 +964,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs 
*ring)
struct drm_i915_gem_object *ctx_obj =
ctx->engine[ring->id].state;
 
-   if (ctx_obj && (ctx != ring->default_context))
+   if (ctx_obj && !ctx->is_global_default)
intel_lr_context_unpin(req);
list_del(&req->execlist_link);
i915_gem_request_unreference(req);
@@ -2367,22 +2364,21 @@ void intel_lr_context_free(struct intel_context *ctx)
 {
int i;
 
-   for (i = 0; i < I915_NUM_RINGS; i++) {
+   for (i = I915_NUM_RINGS; --i >= 0; ) {
+   struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
struct drm_i915_gem_object *ctx_obj = c

[Intel-gfx] [PATCH v2, 0/4] improve handling of the driver's default context

[Dave Gordon]

A reworking of the previous patchset, incorporating Daniel Vetter's
point that we don't need intel_context::file_priv and Chris Wilson's
wish to abolish engine::default_context.

Patch 1/4 starts the process by eliminating file_priv, which was only used
in one place. Patch 2/4 removes lots of references to default_context,
wherever it was used in a comparison. Patch 3/4 continues by removing
more references to default_context, wherever it was used for allocation.
And at the last, patch 4/4 replaces the remaining per-engine uses with a
single per-device pointer, thus finally making the refcounting sane.

Enjoy :)


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

[Intel-gfx] [PATCH v2, 4/4] drm/i915: abolish separate per-engine default_context pointers

[Dave Gordon]

Now that we've eliminated a lot of uses of engine::default_context,
we can eliminate the pointer itself.

All the engines share the same default intel_context, so we can just
keep a single reference to it in the dev_priv structure rather than one
in each of the engine[] elements. This make refcounting more sensible
too, as we now have a refcount of one for the one pointer, rather than
a refcount of one but multiple pointers.

From an idea by Chris Wilson.

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_drv.h|  2 ++
 drivers/gpu/drm/i915/i915_gem.c|  4 ++--
 drivers/gpu/drm/i915/i915_gem_context.c| 22 --
 drivers/gpu/drm/i915/i915_gpu_error.c  |  2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  6 +++---
 drivers/gpu/drm/i915/intel_lrc.c   | 14 --
 drivers/gpu/drm/i915/intel_ringbuffer.h|  1 -
 7 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56d149e..8ed89a1f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1946,6 +1946,8 @@ struct drm_i915_private {
void (*stop_ring)(struct intel_engine_cs *ring);
} gt;
 
+   struct intel_context *kernel_context;
+
bool edp_low_vswing;
 
/* perform PHY state sanity checks? */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 927ebc8..01b1eea 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2763,7 +2763,7 @@ i915_gem_request_alloc_anon(struct intel_engine_cs *ring)
struct drm_i915_gem_request *req;
int err;
 
-   err = i915_gem_request_alloc(ring, ring->default_context, &req);
+   err = i915_gem_request_alloc(ring, dev_priv->kernel_context, &req);
return err ? ERR_PTR(err) : req;
 }
 
@@ -4850,7 +4850,7 @@ i915_gem_init_hw(struct drm_device *dev)
 */
init_unused_rings(dev);
 
-   BUG_ON(!dev_priv->ring[RCS].default_context);
+   BUG_ON(!dev_priv->kernel_context);
 
ret = i915_ppgtt_init_hw(dev);
if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 43437af..c0efec3 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -355,11 +355,10 @@ int i915_gem_context_init(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_context *ctx;
-   int i;
 
/* Init should only be called once per module load. Eventually the
 * restriction on the context_disabled check can be loosened. */
-   if (WARN_ON(dev_priv->ring[RCS].default_context))
+   if (WARN_ON(dev_priv->kernel_context))
return 0;
 
if (intel_vgpu_active(dev) && HAS_LOGICAL_RING_CONTEXTS(dev)) {
@@ -390,12 +389,7 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
 
-   for (i = 0; i < I915_NUM_RINGS; i++) {
-   struct intel_engine_cs *ring = &dev_priv->ring[i];
-
-   /* NB: RCS will hold a ref for all rings */
-   ring->default_context = ctx;
-   }
+   dev_priv->kernel_context = ctx;
 
DRM_DEBUG_DRIVER("%s context support initialized\n",
i915.enable_execlists ? "LR" :
@@ -406,7 +400,7 @@ int i915_gem_context_init(struct drm_device *dev)
 void i915_gem_context_fini(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_context *dctx = dev_priv->ring[RCS].default_context;
+   struct intel_context *dctx = dev_priv->kernel_context;
int i;
 
if (dctx->legacy_hw_ctx.rcs_state) {
@@ -433,17 +427,17 @@ void i915_gem_context_fini(struct drm_device *dev)
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
}
 
-   for (i = 0; i < I915_NUM_RINGS; i++) {
+   for (i = I915_NUM_RINGS; --i >= 0;) {
struct intel_engine_cs *ring = &dev_priv->ring[i];
 
-   if (ring->last_context)
+   if (ring->last_context) {
i915_gem_context_unreference(ring->last_context);
-
-   ring->default_context = NULL;
-   ring->last_context = NULL;
+   ring->last_context = NULL;
+   }
}
 
i915_gem_context_unreference(dctx);
+   dev_priv->kernel_context = NULL;
 }
 
 int i915_gem_context_enable(struct drm_i915_gem_request *req)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 06ca408..7eeb244 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1050,7 +1050,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
if (request)
rbuf = 

Re: [Intel-gfx] [PATCH v2, 2/4] drm/i915: simplify testing for the global default context

[Chris Wilson]

On Wed, Dec 23, 2015 at 07:33:53PM +, Dave Gordon wrote:
> There are quite a number of places where the driver tests whether
> a given context is or is not the global default context, usually by
> checking whether an engine's default_pointer points to the context.
> Now that we have a 'is_global_default' flag in the context itself,
> all these tests these can be rewritten to use it. This makes the
> logic more obvious, and usually saves at least one memory reference.
> In addition, with these uses eliminated, a future patch will be able
> to get rid of engine::default_context entirely.

All the execlists use of ctx != ring->default_context stems from a
misstep in execlists - if you stop treating that default_context as
special during request processing and just take the pin/unpin at
init/fini of the ring, they all disappear.

And please stop conflating is_global_context when we have already a very
good expression for when the context is owned by no file.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/2] mm: Export nr_swap_pages

[Johannes Weiner]

On Thu, Dec 10, 2015 at 10:32:42AM +0100, Daniel Vetter wrote:
> On Fri, Dec 04, 2015 at 11:09:52AM -0500, Johannes Weiner wrote:
> > On Fri, Dec 04, 2015 at 03:58:53PM +, Chris Wilson wrote:
> > > Some modules, like i915.ko, use swappable objects and may try to swap
> > > them out under memory pressure (via the shrinker). Before doing so, they
> > > want to check using get_nr_swap_pages() to see if any swap space is
> > > available as otherwise they will waste time purging the object from the
> > > device without recovering any memory for the system. This requires the
> > > nr_swap_pages counter to be exported to the modules.
> > > 
> > > Signed-off-by: Chris Wilson 
> > > Cc: "Goel, Akash" 
> > > Cc: Johannes Weiner 
> > > Cc: linux...@kvack.org
> > 
> > Acked-by: Johannes Weiner 
> 
> Ack for merging this through drm-intel trees for 4.5? I'm a bit unclear
> who's ack I need for that for linux-mm topics ...

Andrew would be the -mm maintainer. CC'd.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/2] mm: Export nr_swap_pages

[Andrew Morton]

On Wed, 23 Dec 2015 17:04:27 -0500 Johannes Weiner  wrote:

> On Thu, Dec 10, 2015 at 10:32:42AM +0100, Daniel Vetter wrote:
> > On Fri, Dec 04, 2015 at 11:09:52AM -0500, Johannes Weiner wrote:
> > > On Fri, Dec 04, 2015 at 03:58:53PM +, Chris Wilson wrote:
> > > > Some modules, like i915.ko, use swappable objects and may try to swap
> > > > them out under memory pressure (via the shrinker). Before doing so, they
> > > > want to check using get_nr_swap_pages() to see if any swap space is
> > > > available as otherwise they will waste time purging the object from the
> > > > device without recovering any memory for the system. This requires the
> > > > nr_swap_pages counter to be exported to the modules.
> > > > 
> > > > Signed-off-by: Chris Wilson 
> > > > Cc: "Goel, Akash" 
> > > > Cc: Johannes Weiner 
> > > > Cc: linux...@kvack.org
> > > 
> > > Acked-by: Johannes Weiner 
> > 
> > Ack for merging this through drm-intel trees for 4.5? I'm a bit unclear
> > who's ack I need for that for linux-mm topics ...
> 
> Andrew would be the -mm maintainer. CC'd.

yup, please go ahead and merge that via the DRM tree.

nr_swap_pages is a crappy name.  That means "number of pages in swap",
which isn't the case.  Something like "swap_pages_available" would be
better.

And your swap_available() isn't good either ;) It can mean "is any swap
online" or "what is the amount of free swap space (in unknown units!)".
I'd call it "swap_is_full()" and put a ! in the caller.  But it's
hardly important for a wee little static helper.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on ec0382c73cb1adc972bebdd94afad3f0ea117114 drm-intel-nightly: 
2015y-12m-23d-22h-28m-25s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS   (skl-i5k-2)
dmesg-warn -> PASS   (bdw-nuci7)
dmesg-warn -> PASS   (skl-i7k-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS   (ilk-hp8440p)
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (bsw-nuc-2)
pass   -> DMESG-WARN (bdw-nuci7)
pass   -> DMESG-WARN (skl-i7k-2)
Subgroup basic-plain-flip:
pass   -> DMESG-WARN (bdw-ultra)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b-frame-sequence:
pass   -> DMESG-WARN (byt-nuc)
Test kms_psr_sink_crc:
Subgroup psr_basic:
dmesg-warn -> PASS   (bdw-ultra)
Test pm_rpm:
Subgroup basic-rte:
pass   -> DMESG-WARN (byt-nuc)

bdw-nuci7total:132  pass:121  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultratotal:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc  total:135  pass:119  dwarn:3   dfail:0   fail:0   skip:13 
ilk-hp8440p  total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430stotal:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
skl-i7k-2total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
snb-x220ttotal:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_818/

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