Re: [Intel-gfx] [PATCH] i915: Drop legacy execbuffer support

[Keith Packard]

Jason Ekstrand  writes:

> libdrm has supported the newer execbuffer2 ioctl and using it by default
> when it exists since libdrm commit b50964027bef249a0cc3d511de05c2464e0a1e22
> which landed Mar 2, 2010.  The i915 and i965 drivers in Mesa at the time
> both used libdrm and so did the Intel X11 back-end.  The SNA back-end
> for X11 has always used execbuffer2.

All execbuffer users in the past that I'm aware of used libdrm, which
now uses the execbuffer2 ioctl for this API. That means these
applications will remain ABI compatible through this change.

Acked-by: Keith Packard 

-- 
-keith


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

Re: [Intel-gfx] [PATCH] drm/lease: Send a distinct uevent

[Keith Packard]

Daniel Vetter  writes:

> Cc: Keith Packard 

Reviewed-by: Keith Packard 

-- 
-keith


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

Re: [Intel-gfx] [PATCH 3/3] drm/lease: look at ->universal_planes only once

[Keith Packard]

Daniel Vetter  writes:

> @@ -359,7 +359,8 @@ void drm_lease_revoke(struct drm_master *top)
>  static int validate_lease(struct drm_device *dev,
> struct drm_file *lessor_priv,
> int object_count,
> -   struct drm_mode_object **objects)
> +   struct drm_mode_object **objects,
> +   bool universal_planes)

It looks like you can remove the lessor_priv argument here, which would
ensure that we didn't reference any fields in it in the future.

-- 
-keith


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

Re: [Intel-gfx] [PATCH 1/3] drm/lease: debug output for lease creation

[Keith Packard]

Daniel Vetter  writes:

> I spent a bit of time scratching heads and figuring out why the igts
> don't work. Probably useful to keep this work.

Acked-by: Keith Packard 

>   /* Do not allow sub-leases */
> - if (lessor->lessor)
> + if (lessor->lessor) {
> + DRM_DEBUG_LEASE("recursive leasing not allowed\n");
>   return -EINVAL;
> + }

(not review about this patch, just noticing that the relevant code is here:-)

We may want to revisit this restriction as it looks like multi-seat may
end up using leases from a manager to multiple X servers and letting
those X servers support leases would be a good thing.

-- 
-keith


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

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Keith Packard]

Rodrigo Vivi  writes:

> I didn't checked the drm one close enough yet to decide for that.
> DK or Keith? do you guys see anyone suitable for fixes?

Yeah, we should probably get 1, 3 and 7 into fixes. 2, 4, 5 and 6 add
explicit casts where the compiler would already introduce equivalent
implicit casts.

-- 
-keith


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

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Keith Packard]

Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com> writes:

> From: "Pandiyan, Dhinakaran" <dhinakaran.pandi...@intel.com>
>
> drm_vblank_count() has an u32 type returning what is a 64-bit vblank count.
> The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
> space requested vblank sequence using this clipped 32-bit count(when the
> value is >= 2^32) as reference, the requested sequence remains a 32-bit
> value and gets queued like that. However, the code that checks if the
> requested sequence has passed compares this against the 64-bit vblank
> count.

For patches 1-7:

Reviewed-by: Keith Packard <kei...@keithp.com>

-- 
-keith


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

Re: [Intel-gfx] [PATCH 2/5] drm/vblank: Fix data type width for drm_crtc_arm_vblank_event()

[Keith Packard]

Dhinakaran Pandiyan  writes:

> Now that drm_vblank_count() returns all bits of the vblank count, update
> drm_crtc_arm_vblank_event() so that it queues the correct sequence.
> Otherwise, this leads to prolonged waits for a vblank sequence when the
> current count is >=2^32.

The summary for this patch is incorrect; the function being fixed is
drm_crtc_accurate_vblank_event.

And, I'm afraid you've uncovered a rabbit hole here -- there are a
couple of users of this function outside of the core, including i915 in
a couple of places and nouveau. We should at least review the whole call
graph starting here and make sure it does what we think it should.

Thanks for finding these bugs!

-- 
-keith


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

Re: [Intel-gfx] [PATCH 1/5] drm/vblank: Fix return type for drm_vblank_count()

[Keith Packard]

Dhinakaran Pandiyan  writes:

> drm_vblank_count() has a u32 type returning what is a 64-bit vblank
> count.

It looks like a general review of the 64-bit widening patch is needed.

 * drm_crtc_accurate_vblank_count has a 32-bit return, and uses a 32-bit 
temporary
 * drm_wait_one_vblank uses a 32-bit temporary.

I looked at every 'u32' in drm_vblank.c; it would be good to have more
eyes check this.

Thanks for finding the first one.

-- 
-keith


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

[Intel-gfx] [PATCH i-g-t 1/2] tests/kms_sequence: Add tests for new CRTC get/queue sequence ioctls [v2]

[Keith Packard]

These ioctls replace drmWaitVBlank and add ns time resolution and
64-bit sequence numbers to comply with the Vulkan API specifications.

The tests were derived from the existing kms_vblank tests with the
'wait' variant elided as the new API doesn't provide a mechanism for
blocking in the kernel.

v2: from Dave Airlie <airl...@redhat.com>

 * Add local definitions of new ioctls to avoid requiring latest
   libdrm.

 * Remove FIRST_PIXEL_OUT as that has been removed from the proposed
   kernel patches.

Signed-off-by: Keith Packard <kei...@keithp.com>
---
 lib/igt_kms.c  |   2 +-
 lib/igt_kms.h  |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_sequence.c   | 317 +
 tests/meson.build  |   1 +
 5 files changed, 321 insertions(+), 1 deletion(-)
 create mode 100644 tests/kms_sequence.c

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 1242324e..f6ad3565 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2079,7 +2079,7 @@ report_dup:
}
 }
 
-static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
+igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 {
igt_display_t *display = output->display;
enum pipe pipe;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index cfe646ae..8f270bfd 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -377,6 +377,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, int 
plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
 drmModeConnector *connector);
+igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output);
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
 
 void igt_pipe_request_out_fence(igt_pipe_t *pipe);
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index e4e06d01..485ed429 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -211,6 +211,7 @@ TESTS_progs = \
kms_tv_load_detect \
kms_universal_plane \
kms_vblank \
+   kms_sequence \
meta_test \
perf \
perf_pmu \
diff --git a/tests/kms_sequence.c b/tests/kms_sequence.c
new file mode 100644
index ..783dda25
--- /dev/null
+++ b/tests/kms_sequence.c
@@ -0,0 +1,317 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/** @file kms_sequence.c
+ *
+ * This is a test of drmCrtcGetSequence and drmCrtcQueueSequence
+ */
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test CrtcGetSequence and CrtcQueueSequence.");
+
+typedef struct {
+   igt_display_t display;
+   struct igt_fb primary_fb;
+   igt_output_t *output;
+   uint32_t crtc_id;
+   enum pipe pipe;
+   unsigned int flags;
+#define IDLE 1
+#define BUSY 2
+#define FORKED 4
+} data_t;
+
+struct local_drm_crtc_get_sequence {
+   __u32 crtc_id;
+   __u32 active;
+   __u64 sequence;
+   __u64 sequence_ns;
+};
+
+struct local_drm_crtc_queue_sequence {
+   __u32 crtc_id;
+   __u32 flags;
+   __u64 sequence;
+   __u64 user_data;
+};
+
+#define LOCAL_DRM_IOCTL_CRTC_GET_SEQUENCE DRM_IOWR(0x3b, struct 
local_drm_crtc_get_sequence)
+#define LOCAL_DRM_IOCTL_CRTC_QUEUE_SEQUENCE   DRM_IOWR(0x3c, struct 
local_drm_crtc_queue_sequence)
+
+#define LOCAL_DRM_CRTC_SEQUENCE_RELATIVE  0x0001  /* 
sequence is relative to current */
+#define LOCAL_DRM_CRTC_SEQUENCE_NEXT_ON_MISS  0x0002  /* Use 
next sequence if we've missed */
+
+struct local_drm_event_crtc_sequence {
+struct drm_event   

[Intel-gfx] [PATCH i-g-t 0/2] tests: Add tests for new sequence API and leases

[Keith Packard]

Both of these APIs are now in the kernel; it would be nice to have the
tests integrated into the test suite.

This includes an updated version of the kms_lease patch (v3) which
adapts to the final kernel API changes.

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

[Intel-gfx] [PATCH i-g-t 2/2] tests/kms_lease: add tests for lease ioctls [v3]

[Keith Packard]

Validate that the leasing API creates leases that allow access to a
subset of the available resources and that lease revocation works.

v2: from Dave Airlie <airl...@redhat.com>

 * Update ioctl numbers to latest proposed values.
 * Fix commit message
 * Add tests for get_lease and list_lessees

v3: adapt to final kernel API

* Disable CAP_UNIVERSAL_PLANES before leasing so that we don't need to
  also include a plane.

* Zero out all of the ioctl structures so that any padding fields are
  set correctly.

* When listing a lease, expect 3 resources -- crtc, connector and plane.

Signed-off-by: Keith Packard <kei...@keithp.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_lease.c  | 606 +
 tests/meson.build  |   1 +
 3 files changed, 608 insertions(+)
 create mode 100644 tests/kms_lease.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 485ed429..2bef7d68 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -189,6 +189,7 @@ TESTS_progs = \
kms_frontbuffer_tracking \
kms_hdmi_inject \
kms_invalid_dotclock \
+   kms_lease \
kms_legacy_colorkey \
kms_mmap_write_crc \
kms_mmio_vs_cs_flip \
diff --git a/tests/kms_lease.c b/tests/kms_lease.c
new file mode 100644
index ..2a6d82fb
--- /dev/null
+++ b/tests/kms_lease.c
@@ -0,0 +1,606 @@
+/*
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/** @file kms_lease.c
+ *
+ * This is a test of DRM leases
+ */
+
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+IGT_TEST_DESCRIPTION("Test of CreateLease.");
+
+struct local_drm_mode_create_lease {
+/** Pointer to array of object ids (__u32) */
+__u64 object_ids;
+/** Number of object ids */
+__u32 object_count;
+/** flags for new FD (O_CLOEXEC, etc) */
+__u32 flags;
+
+/** Return: unique identifier for lessee. */
+__u32 lessee_id;
+/** Return: file descriptor to new drm_master file */
+__u32 fd;
+};
+
+struct local_drm_mode_list_lessees {
+/** Number of lessees.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+__u32 count_lessees;
+__u32 pad;
+
+/** Pointer to lessees.
+ * pointer to __u64 array of lessee ids
+ */
+__u64 lessees_ptr;
+};
+
+struct local_drm_mode_get_lease {
+/** Number of leased objects.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+__u32 count_objects;
+__u32 pad;
+
+/** Pointer to objects.
+ * pointer to __u32 array of object ids
+ */
+__u64 objects_ptr;
+};
+
+/**
+ * Revoke lease
+ */
+struct local_drm_mode_revoke_lease {
+/** Unique ID of lessee
+ */
+__u32 lessee_id;
+};
+
+
+#define LOCAL_DRM_IOCTL_MODE_CREATE_LEASE DRM_IOWR(0xC6, struct 
local_drm_mode_create_lease)
+#define LOCAL_DRM_IOCTL_MODE_LIST_LESSEES DRM_IOWR(0xC7, struct 
local_drm_mode_list_lessees)
+#define LOCAL_DRM_IOCTL_MODE_GET_LEASEDRM_IOWR(0xC8, struct 
local_drm_mode_get_lease)
+#define LOCAL_DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct 
local_drm_mode_revoke_lease)
+
+typedef struct {
+   int fd;
+   uint32_t lessee_id;
+   igt_display_t display;
+   st

[Intel-gfx] [PATCH] drm: Update edid-derived drm_display_info fields at edid property set [v2]

[Keith Packard]

There are a set of values in the drm_display_info structure for each
connector which hold information derived from EDID. These are computed
in drm_add_display_info. Before this patch, that was only called in
drm_add_edid_modes. This meant that they were only set when EDID was
present and never reset when EDID was not, as happened when the
display was disconnected.

One of these fields, non_desktop, is used from
drm_mode_connector_update_edid_property, the function responsible for
assigning the new edid value to the application-visible property.

Various drivers call these two functions (drm_add_edid_modes and
drm_mode_connector_update_edid_property) in different orders. This
means that even when EDID is present, the drm_display_info fields may
not have been computed at the time that
drm_mode_connector_update_edid_property used the non_desktop value to
set the non_desktop property.

I've added a public function (drm_reset_display_info) that resets the
drm_display_info field values to default values and then made the
drm_add_display_info function public. These two functions are now
called directly from drm_mode_connector_update_edid_property so that
the drm_display_info fields are always computed from the current EDID
information before being used in that function.

This means that the drm_display_info values are often computed twice,
once when the EDID property it set and a second time when EDID is used
to compute modes for the device. The alternative would be to uniformly
ensure that the values were computed once before being used, which
would require that all drivers reliably invoke the two paths in the
same order. The computation is inexpensive enough that it seems more
maintainable in the long term to simply compute them in both paths.

The API to drm_add_display_info has been changed so that it no longer
takes the set of edid-based quirks as a parameter. Rather, it now
computes those quirks itself and returns them for further use by
drm_add_edid_modes.

This patch also includes a number of 'const' additions caused by
drm_mode_connector_update_edid_property taking a 'const struct edid *'
parameter and wanting to pass that along to drm_add_display_info.

v2: after review by Daniel Vetter <daniel.vet...@ffwll.ch>

Removed EXPORT_SYMBOL_GPL for drm_reset_display_info and
drm_add_display_info.

Added FIXME in drm_mode_connector_update_edid_property about
potentially merging that with drm_add_edid_modes to avoid
the need for two driver calls.

Signed-off-by: Keith Packard <kei...@keithp.com>
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
---
 drivers/gpu/drm/drm_connector.c | 13 ++
 drivers/gpu/drm/drm_edid.c  | 53 ++---
 include/drm/drm_edid.h  |  2 ++
 3 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 25f4b2e9a44f..83c01f359d55 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1207,6 +1207,19 @@ int drm_mode_connector_update_edid_property(struct 
drm_connector *connector,
if (edid)
size = EDID_LENGTH * (1 + edid->extensions);
 
+   /* Set the display info, using edid if available, otherwise
+* reseting the values to defaults. This duplicates the work
+* done in drm_add_edid_modes, but that function is not
+* consistently called before this one in all drivers and the
+* computation is cheap enough that it seems better to
+* duplicate it rather than attempt to ensure some arbitrary
+* ordering of calls.
+*/
+   if (edid)
+   drm_add_display_info(connector, edid);
+   else
+   drm_reset_display_info(connector);
+
drm_object_property_set_value(>base,
  dev->mode_config.non_desktop_property,
  connector->display_info.non_desktop);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 524eace3d460..365901c1c33c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1731,7 +1731,7 @@ EXPORT_SYMBOL(drm_edid_duplicate);
  *
  * Returns true if @vendor is in @edid, false otherwise
  */
-static bool edid_vendor(struct edid *edid, const char *vendor)
+static bool edid_vendor(const struct edid *edid, const char *vendor)
 {
char edid_vendor[3];
 
@@ -1749,7 +1749,7 @@ static bool edid_vendor(struct edid *edid, const char 
*vendor)
  *
  * This tells subsequent routines what fixes they need to apply.
  */
-static u32 edid_get_quirks(struct edid *edid)
+static u32 edid_get_quirks(const struct edid *edid)
 {
const struct edid_quirk *quirk;
int i;
@@ -2813,7 +2813,7 @@ add_detailed_modes(struct drm_connector *connector, 
struct edid *edid,
 /*
  * Search EDID for CEA extension block.
  */
-static u8 *drm_find_ed

[Intel-gfx] [PATCH i-g-t 2/2] tests/kms_lease: add tests for lease ioctls [v2]

[Keith Packard]

Validate that the leasing API creates leases that allow access to a
subset of the available resources and that lease revocation works.

v2: from Dave Airlie <airl...@redhat.com>

 * Update ioctl numbers to latest proposed values.
 * Fix commit message
 * Add tests for get_lease and list_lessees

Signed-off-by: Keith Packard <kei...@keithp.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_lease.c  | 597 +
 tests/meson.build  |   1 +
 3 files changed, 599 insertions(+)
 create mode 100644 tests/kms_lease.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 9dc1d250..1ca6e104 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -193,6 +193,7 @@ TESTS_progs = \
kms_frontbuffer_tracking \
kms_hdmi_inject \
kms_invalid_dotclock \
+   kms_lease \
kms_legacy_colorkey \
kms_mmap_write_crc \
kms_mmio_vs_cs_flip \
diff --git a/tests/kms_lease.c b/tests/kms_lease.c
new file mode 100644
index ..49226163
--- /dev/null
+++ b/tests/kms_lease.c
@@ -0,0 +1,597 @@
+/*
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/** @file kms_lease.c
+ *
+ * This is a test of DRM leases
+ */
+
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+IGT_TEST_DESCRIPTION("Test of CreateLease.");
+
+struct local_drm_mode_create_lease {
+/** Pointer to array of object ids (__u32) */
+__u64 object_ids;
+/** Number of object ids */
+__u32 object_count;
+/** flags for new FD (O_CLOEXEC, etc) */
+__u32 flags;
+
+/** Return: unique identifier for lessee. */
+__u32 lessee_id;
+/** Return: file descriptor to new drm_master file */
+__u32 fd;
+};
+
+struct local_drm_mode_list_lessees {
+/** Number of lessees.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+__u32 count_lessees;
+__u32 pad;
+
+/** Pointer to lessees.
+ * pointer to __u64 array of lessee ids
+ */
+__u64 lessees_ptr;
+};
+
+struct local_drm_mode_get_lease {
+/** Number of leased objects.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+__u32 count_objects;
+__u32 pad;
+
+/** Pointer to objects.
+ * pointer to __u32 array of object ids
+ */
+__u64 objects_ptr;
+};
+
+/**
+ * Revoke lease
+ */
+struct local_drm_mode_revoke_lease {
+/** Unique ID of lessee
+ */
+__u32 lessee_id;
+};
+
+
+#define LOCAL_DRM_IOCTL_MODE_CREATE_LEASE DRM_IOWR(0xC6, struct 
local_drm_mode_create_lease)
+#define LOCAL_DRM_IOCTL_MODE_LIST_LESSEES DRM_IOWR(0xC7, struct 
local_drm_mode_list_lessees)
+#define LOCAL_DRM_IOCTL_MODE_GET_LEASEDRM_IOWR(0xC8, struct 
local_drm_mode_get_lease)
+#define LOCAL_DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct 
local_drm_mode_revoke_lease)
+
+typedef struct {
+   int fd;
+   uint32_t lessee_id;
+   igt_display_t display;
+   struct igt_fb primary_fb;
+   igt_output_t *output;
+   drmModeModeInfo *mode;
+} lease_t;
+
+typedef struct {
+   lease_t master;
+   enum pipe pipe;
+   uint32_t crtc_id;
+   uint32_t connector_id;
+} data_t;
+
+static uint32_t pipe_to_crtc_id(igt_display_t *display, e

[Intel-gfx] [PATCH i-g-t 0/2] Add new vblank and lease tests [v2]

[Keith Packard]

Changes since last version:

 * Add local definitions of new ioctls to avoid dependency on proposed
   libdrm bits.

 * Remove FIRST_PIXEL_OUT as that has been removed from the proposed
   kernel interface

 * Add tests for get_lease and list_lessees

 * Fix commit message on the lease test patch

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

[Intel-gfx] [PATCH i-g-t 1/2] tests/kms_sequence: Add tests for new CRTC get/queue sequence ioctls [v2]

[Keith Packard]

These ioctls replace drmWaitVBlank and add ns time resolution and
64-bit sequence numbers to comply with the Vulkan API specifications.

The tests were derived from the existing kms_vblank tests with the
'wait' variant elided as the new API doesn't provide a mechanism for
blocking in the kernel.

v2: from Dave Airlie <airl...@redhat.com>

 * Add local definitions of new ioctls to avoid requiring latest
   libdrm.

 * Remove FIRST_PIXEL_OUT as that has been removed from the proposed
   kernel patches.

Signed-off-by: Keith Packard <kei...@keithp.com>
---
 lib/igt_kms.c  |   2 +-
 lib/igt_kms.h  |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_sequence.c   | 317 +
 tests/meson.build  |   1 +
 5 files changed, 321 insertions(+), 1 deletion(-)
 create mode 100644 tests/kms_sequence.c

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 379bd0c3..44285baa 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1983,7 +1983,7 @@ report_dup:
}
 }
 
-static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
+igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 {
igt_display_t *display = output->display;
enum pipe pipe;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 8dc118c9..92b66676 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -397,6 +397,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, int 
plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
 drmModeConnector *connector);
+igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output);
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
 bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
   uint32_t *prop_id, uint64_t *value,
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index bb6652e2..9dc1d250 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -215,6 +215,7 @@ TESTS_progs = \
kms_tv_load_detect \
kms_universal_plane \
kms_vblank \
+   kms_sequence \
meta_test \
perf \
pm_backlight \
diff --git a/tests/kms_sequence.c b/tests/kms_sequence.c
new file mode 100644
index ..783dda25
--- /dev/null
+++ b/tests/kms_sequence.c
@@ -0,0 +1,317 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/** @file kms_sequence.c
+ *
+ * This is a test of drmCrtcGetSequence and drmCrtcQueueSequence
+ */
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test CrtcGetSequence and CrtcQueueSequence.");
+
+typedef struct {
+   igt_display_t display;
+   struct igt_fb primary_fb;
+   igt_output_t *output;
+   uint32_t crtc_id;
+   enum pipe pipe;
+   unsigned int flags;
+#define IDLE 1
+#define BUSY 2
+#define FORKED 4
+} data_t;
+
+struct local_drm_crtc_get_sequence {
+   __u32 crtc_id;
+   __u32 active;
+   __u64 sequence;
+   __u64 sequence_ns;
+};
+
+struct local_drm_crtc_queue_sequence {
+   __u32 crtc_id;
+   __u32 flags;
+   __u64 sequence;
+   __u64 user_data;
+};
+
+#define LOCAL_DRM_IOCTL_CRTC_GET_SEQUENCE DRM_IOWR(0x3b, struct 
local_drm_crtc_get_sequence)
+#define LOCAL_DRM_IOCTL_CRTC_QUEUE_SEQUENCE   DRM_IOWR(0x3c, struct 
local_drm_crtc_queue_sequence)
+
+#define LOCAL_DRM_CRTC_SEQUENCE_RELATIVE  0x0001  /* 
sequence is relative to current */
+#define LOCAL_DRM_CRTC_SEQUENCE_NEXT_ON_MISS  0x0002  /* Use 
next sequence if we've missed */
+

Re: [Intel-gfx] [PATCH] drm/dp: Do not prune the last mode on the connector

[Keith Packard]

Manasi Navare  writes:

> This patch fixes this problem by checking if the mode being pruned
> is the last mode on that connector and if so doesnt prune it.

I think you want to stop pruning when you've gotten to a single mode on
the list, not at the last mode in the list (which may well need to be
pruned).

This is entirely untested, but perhaps

+   !(list_is_singular(mode_list))) {

is what you want?

-- 
-keith


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

Re: [Intel-gfx] [PATCH] drm: Document code of conduct

[Keith Packard]

Daniel Vetter <daniel.vet...@ffwll.ch> writes:

> freedesktop.org has adopted a formal code of conduct:

Acked-by: Keith Packard <kei...@keithp.com>

-- 
-keith


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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Drop reference to current state wait req as soon as it goes unused

[Keith Packard]

Daniel Vetter  writes:

> We still need to clean up the reference in case of failure, which
> means latest in intel_plane_destroy_state(). Also hanging onto a
> request isn't that evil really, why can't we just only clean up in the
> destroy function?

Sticking a cleanup there as well is good insurance, but it seems like a
reasonable policy to drop references when values go 'out of scope' as
they do in cleanup_plane_fb. But, you're right, we only *need* to drop
the reference in the destroy function, it's just that the state hangs
around until the next mode setting operation, which is likely to be days
away.

-- 
-keith


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

Re: [Intel-gfx] [PATCH] drm/i915: cleanup_plane_fb: also drop reference to current state wait_req

[Keith Packard]

Daniel Vetter  writes:

> Hm, I think we should just clean up wiat_req in ->atomic_destroy_state
> instead of littering cleanup code all over. But this gets the job done, so
> applied.

Thanks. It's required for the DRM patch I posted that makes moving the
cursor not block on rendering. I'm hoping Dave will get that merged for
4.8; it's really annoying to have the server moving the cursor in a
separate thread and then have the motion blocked in the kernel for a few
seconds worth of rendering.

-- 
-keith


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

[Intel-gfx] [PATCH] drm/i915: cleanup_plane_fb: also drop reference to current state wait_req

[Keith Packard]

There are two paths into intel_cleanup_plane_fb, the normal completion
path and the failure path.

In the failure case, intel_cleanup_plane_fb is called before
drm_atomic_helper_swap_state, so any wait_req reference made in
intel_prepare_plane_fb will be in old_intel_state->wait_req.

In the normal completion path, drm_atomic_helper_swap_state has
already been called, so the plane state holding the just-used wait_req
will not be in old_intel_state->wait_req, rather it will be in the
state associated with the plane itself.

Clearing this reference ensures that the wait_req will be freed as
soon as it the related mode setting operation is complete, rather than
waiting for some future mode setting operation to eventually
dereference it.

The existing dereference of old_intel_state->wait_req is still
required as that will hold the wait_req when the mode setting
operation fails.

cc: Daniel Vetter <daniel.vet...@intel.com>
cc: David Airlie <airl...@linux.ie>
cc: intel-gfx@lists.freedesktop.org
cc: dri-de...@lists.freedesktop.org
Signed-off-by: Keith Packard <kei...@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3074c56..dbabaf3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13924,6 +13924,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
struct drm_device *dev = plane->dev;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_plane_state *old_intel_state;
+   struct intel_plane_state *intel_state = 
to_intel_plane_state(plane->state);
struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
 
@@ -13941,6 +13942,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
(obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit)))
i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
 
+   i915_gem_request_assign(_state->wait_req, NULL);
i915_gem_request_assign(_intel_state->wait_req, NULL);
 }
 
-- 
2.8.1

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

Re: [Intel-gfx] [PATCH 0/2 xf86-video-intel] Two DRI3/Present bug fixes for UXA

[Keith Packard]

Chris Wilson ch...@chris-wilson.co.uk writes:

 commit d21d781466785c317131a8a57606925867265dc8
 Author: Daniel Vetter daniel.vet...@ffwll.ch
 Date:   Tue Feb 22 18:31:44 2011 +0100

 Fix relaxed tiling on gen2

This one matches libdrm in using 16 for the tile height alignment on
gen2.

 Try enabling relaxed fencing again.

 No. The clearest requirement is that the ddx (or other display server)
 must treat incoming surfaces as tainted and validate them to be sure
 that they work with its code paths. If it can't we have a choice of
 either rejecting them outright, or staging them.

If there's a stricter alignment requirement, then we must fix both the
2D driver and libdrm. Otherwise, the user's session will simply crash at
startup.

However, I still see absolutely no evidence that gen2 requires tile
alignment to 32 rows, or that gen3+ requires tile alignment to 16
rows in any software configuration at all.

-- 
keith.pack...@intel.com


pgpB1HedFzRjW.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/2 xf86-video-intel] Two DRI3/Present bug fixes for UXA

[Keith Packard]

Chris Wilson ch...@chris-wilson.co.uk writes:

 That extra alignment is due to gen2 and early gen3 (if
 (!intel-has_relaxed_fencing) covers them).

Here's the patch which changed the alignment requirment:

commit 736b89504a32239a0c7dfb5961c1b8292dd744bd
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Sun Dec 30 10:32:18 2012 +

uxa: Align surface allocations to even tile rows

Align surface sizes to an even number of tile rows to cater for 
sampler
prefetch. If we read beyond the last page we may catch the PTE in a
state of flux and trigger a GPU hang. Also detected by enabling 
invalid
PTE access checking.

References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Both of these bugs report regressions found past the 3.6 kernel, one on
965gm and the other on Ironlake. Are there additional bug reports on UXA
which actually relate to this patch as it affects gen2 and gen3
hardware?

Here's the patch that added the additional alignment restriction to SNA:

commit 1b6c1a30723b1d13e9bd3df0b59a8d75639c89be
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Fri Nov 30 09:27:57 2012 +

sna: Increase tiling alignment to an even tile

Seems to help g4x.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Note that this does not reference gen2 or gen3 either.

From the above two patches, all that I can learn is that both of these
larger alignments were introduced to fix bugs on newer hardware, and
that the larger alignment is now specifically disabled on that same
hardware in the SNA code. Reading only this history, I felt reasonably
confident that changing UXA back to what libdrm does was the best plan.

If you have specific bug reports that were resolved by this patch, or
specific hardware documentation which indicates that this patch is
required, especially as it relates to gen2 and gen3 hardware, I'd love
to see them.

In any case, we've now got four versions of the pixmap alignment code
(libdrm, uxa and sna in two varieties). They're all subtly different;
one suspects that each one works on some set of problems and fails on
others...

Here's what the height alignment requirements are:

libdrm  uxa uxa sna sna
  +keithp=2.6.38 2.6.38

gen2 none 2  2   2   1   2
gen3 none 2  2   2   1   2
gen4+ none2  2   2   1   1

gen2 X   16 16  32  16  32
gen3 X8  8  16   8  16
gen4+ X   8  8  16   8   8

gen2 Y   16 16  32  16  32
i915 Y8 32  64   8  16
i945 Y   32 32  64   8  16
gen4+ Y  32 32  64  32  32

It looks like the SNA alignment for untiled buffers is incorrect? 965
hardware is documented to read buffers in 2x2 chunks, so a failure to
height align allocations to 2 can result in reads off the end of the
buffer.

For uxa's intel_set_pixmap_bo, and sna's sna_dri3_pixmap_from_fd,
there's a clear requirement that the 2D driver impose no stricter
alignment than libdrm, so that, buffer passing from Mesa to X will work.

For pixmap allocations within the X server, we should ensure that the
alignment requirements are at least as strict as those in libdrm so that
pixmap passing from X to Mesa will work. Not that Mesa actually checks
the provided buffer size at all, although it probably should.x

It seems obvious to me that we should be doing this work in one place
and sharing the code across the 2D and 3D drivers, and yet we never
have.

-- 
keith.pack...@intel.com


pgpGkYFBBdj8Y.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 0/2 xf86-video-intel] Two DRI3/Present bug fixes for UXA

[Keith Packard]

Here are a couple of small bug fixes which make DRI3/Present work
better with UXA.

 [PATCH 1/2] Do not clear pending kernel events on mode switch

This patch prevents GL-based compositing managers from wedging when
performing video mode setting. The problem was that DIX was never
receiving notification about page flips being completed when one was
pending across a mode switch.

 [PATCH 2/2] Correct BO allocation alignment

This patch makes UXA and Mesa agree about how buffers are allocated
for images. Without this, UXA was requiring larger padding, which
meant that converting some textures into pixmaps using DRI3 would
fail.

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

[Intel-gfx] [PATCH 2/2] Correct BO allocation alignment

[Keith Packard]

BO allocations for pixmaps must be aligned to the tile height, but at
some point the code was changed to align them to twice the tile
height. This overallocates pixmaps, wasting memory, but more
importantly, for buffers allocated by DRM and shared through DRI3, the
stricter alignment check causes sharing to fail.

From reading through the history of the code and related bugs, it
seems like this change was part of a set of changes trying to address
what turned out to be a kernel regression. Reverting this change
solves the DRI3 problem and saves a bit of memory for pixmap
allocations.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel_uxa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/uxa/intel_uxa.c b/src/uxa/intel_uxa.c
index d33eca5..4ce6eae 100644
--- a/src/uxa/intel_uxa.c
+++ b/src/uxa/intel_uxa.c
@@ -206,7 +206,7 @@ intel_uxa_compute_size(struct intel_screen_private *intel,
tile_height = 8;
else
tile_height = 32;
-   aligned_h = ALIGN(h, 2*tile_height);
+   aligned_h = ALIGN(h, tile_height);
 
*stride = intel_get_fence_pitch(intel,
ALIGN(pitch, 512),
@@ -768,7 +768,7 @@ free_priv:
else
height = 32;
 
-   height = ALIGN(pixmap-drawable.height, 2*height);
+   height = ALIGN(pixmap-drawable.height, height);
size = intel_get_fence_size(intel, priv-stride * 
height);
} else
size = priv-stride * pixmap-drawable.height;
-- 
2.0.1

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

[Intel-gfx] [PATCH 1/2] Do not clear pending kernel events on mode switch

[Keith Packard]

Let the kernel send these back to us so that DIX hears about them in
the usual way.

Mode setting while Present has a flip active will trigger an unflip
before the mode is changed. The event from that unflip will not get
processed before the mode switch is executed. Clearing the driver
queue at mode switch time will discard the connection between the
kernel event and the present callback so that DIX will never know that
the flip pixmap is idle.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel_display.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 97af76d..8c43ae9 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -71,9 +71,6 @@ struct intel_drm_queue {
 intel_drm_abort_proc abort;
 };
 
-static void
-intel_drm_abort_scrn(ScrnInfoPtr scrn);
-
 static uint32_t intel_drm_seq;
 static struct list intel_drm_queue;
 
@@ -398,7 +395,6 @@ intel_crtc_apply(xf86CrtcPtr crtc)
 
if (scrn-pScreen)
xf86_reload_cursors(scrn-pScreen);
-intel_drm_abort_scrn(scrn);
 
 done:
free(output_ids);
-- 
2.0.1

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

Re: [Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 OK, but isn't the problem with the code you're #if 0ing (which, really?
 #if 0 in a patch being submitted for review?) that it's aligning to
 2*height instead of height?

I went and did a bit of archaeology to figure out why it's using
2*height instead of just height. The initial change to pixmap
allocation, which was then copied to the BO validation logic, was here:

commit 736b89504a32239a0c7dfb5961c1b8292dd744bd
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Sun Dec 30 10:32:18 2012 +

uxa: Align surface allocations to even tile rows

Align surface sizes to an even number of tile rows to cater for 
sampler
prefetch. If we read beyond the last page we may catch the PTE in a
state of flux and trigger a GPU hang. Also detected by enabling 
invalid
PTE access checking.

References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index f5ac0a6..2f14173 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -209,7 +209,7 @@ intel_uxa_pixmap_compute_size(PixmapPtr pixmap,
tile_height = 8;
else
tile_height = 32;
-   aligned_h = ALIGN(h, tile_height);
+   aligned_h = ALIGN(h, 2*tile_height);
 
*stride = intel_get_fence_pitch(intel,
ALIGN(pitch, 512),

Look at the referenced bugs, what I found was a long list of random GPU
hangs on ILK and SNB hardware that appear to have been caused by a
kernel change. Daniel and Chris created a number of scatter shot fixes
across the kernel, and this patch to the 2D driver. However, this patch
doesn't appear to have actually solved anything; Norbert Preining was
still crashing with this patch applied:

https://bugs.freedesktop.org/show_bug.cgi?id=55984#c129

Furthermore, SNA has some similar code, but it applies it conditionally
for hardware which doesn't have 'relaxed fencing', which is only
hardware older than i965 when running on a older kernel that doesn't
recognize the HAS_RELAXED_FENCING parameter (the current kernel always
returns 'true').

So, as near as I can tell, this fix should never be necessary as the
reported bug wasn't fixed by it, and SNA does not apply this rule to any
hardware on which either bug was reproduced.

If this fix is actually useful, wouldn't we want it in Mesa as well?

-- 
keith.pack...@intel.com


pgpyPVU4ToK8H.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/12] Do more checks for proposed flip pixmaps

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 Make sure the pitch and tiling are correct.
 Make sure there's a BO we can get at.

 I thought we couldn't change these parameters, but now I can't find what
 prevents them from changing.  Can you cite sources?

Looks like we *can* change tiling format. That actually makes me kinda
happy as that explains why we were able to allocate a linear frame
buffer for the X front buffer (due to a bug) and page flip to DRI3
buffers which are always tiled.

However, we can't change the pitch. From the kernel driver:

/*
 * TILEOFF/LINOFF registers can't be changed via MI display flips.
 * Note that pitch changes could also affect these register.
 */
if (INTEL_INFO(dev)-gen  3 
(fb-offsets[0] != crtc-primary-fb-offsets[0] ||
 fb-pitches[0] != crtc-primary-fb-pitches[0]))
return -EINVAL;

I'll remove the tiling check.

-- 
keith.pack...@intel.com


pgpuPa0GqaUNw.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/12] Add glamor back into the driver

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 This adds glamor support back into the driver, but instad of going
 through UXA, this uses it directly instead.

 This is hard to read with the conditionalizing all of the UXA code in
 the same commit as adding the glamor code.  Then there are a bunch of
 unrelated whitespace changes, or flattening of a bunch of nested
 conditionals.

I'm not sure how to make the patch easier to review; I guess I could
make UXA conditional first? That would be 'crazy' in that the driver
would fail to ever work if you didn't ask for UXA, but might make the
patch easier to read?

 The only substantive problem I see is intel_glamor_set_pixmap_bo().

 The extra enum and temporary variable introduced here seems pretty
 pointless (even if that pattern had happened before).

Agreed. The problem is that 'DEFAULT_ACCEL_METHOD' is defined as
'GLAMOR', 'UXA' or 'NONE' by configure.ac. This seemed like the cleanest
solution in some ways. I also liked having the accel_type enum *not*
define acceleration types which weren't compiled into the driver; that
caught a few missing #ifdefs

 I don't think this will work -- glamor_egl uses a different fd from
 intel-bufmgr, so you're attaching some unrelated BO, if you're even
 that lucky.

This API uses the same FD as the intel bufmgr.

From intel_glamor.c:

if (!glamor_egl_init(scrn, intel-drmSubFD)) {

From glamor_egl.c:

Bool
glamor_egl_init(ScrnInfoPtr scrn, int fd)
...

glamor_egl-fd = fd;
...
glamor_egl-has_gem = glamor_egl_check_has_gem(fd);

...
Bool
glamor_egl_create_textured_pixmap(PixmapPtr pixmap, int handle, int 
stride)
...
if (glamor_egl-has_gem) {
if (!glamor_get_flink_name(glamor_egl-fd, handle, name)) {

So, you pass in a GEM handle for the intel driver bufmgr and glamor
does the flink to get a global name.

We should be using an FD passing mechanism here instead, to avoid
creating more global names...

 No need to check for initiailization -- double-calling it is safe (as
 long as the size is consistent).

Thanks. Will fix.

  void
 @@ -258,18 +240,52 @@ intel_glamor_flush(intel_screen_private * intel)
  ScreenPtr screen;
  
  screen = xf86ScrnToScreen(intel-scrn);
 -if (intel-uxa_flags  UXA_USE_GLAMOR)
 -glamor_block_handler(screen);
 +glamor_block_handler(screen);
  }

 glamor_block_handler() is pretty awfully named.  We should do something
 about that.

Suggestions welcome :-)

Thanks for the careful review.

-- 
keith.pack...@intel.com


pgpCJ7Bv7BEdF.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] Rework intel 2D driver glamor support

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 I spent the day just cleaning up this patch series and testing. I
 think it's ready for others to use and review. I've been running it on
 two machines for a couple of days now and it's been solid.

 Patches 2, 4 are:

 Reviewed-by: Eric Anholt e...@anholt.net

 Patch 5 is:

 Acked-by: Eric Anholt e...@anholt.net

Thanks.

-- 
keith.pack...@intel.com


pgp7IBtW8_yBv.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] Rework intel 2D driver glamor support

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 I spent the day just cleaning up this patch series and testing. I
 think it's ready for others to use and review. I've been running it on
 two machines for a couple of days now and it's been solid.

 Patches 2, 4 are:

 Reviewed-by: Eric Anholt e...@anholt.net

 Patch 5 is:

 Acked-by: Eric Anholt e...@anholt.net

I've pushed a new version of the tree with your review marked and a few
changes:

 * Split the GetScratchPixmapHeader change into two pieces; the first
   just removes the redundant calls to drm_intel_bo_disable_reuse and
   the second contains the scratch pixmap changes and the other misc
   stuff. Yes, I could split the misc changes out into another cleanup
   patch if you want.

 * Removed the tiling check from intel_present when flipping. The kernel
   doesn't appear to ever require matching tiling.

 * Removed call to dixPrivateKeyRegistered in the 'Add glamor back' patch.

What I didn't do is clean up the 'remove glamor' patch so that it would
compile even with --enable-glamor. Fixing that seems like noise to me;
all of the changes needed to make it work would be immediately un-done
when adding glamor back.

-- 
keith.pack...@intel.com


pgpeiubNu3VUv.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/12] Do more checks for proposed flip pixmaps

[Keith Packard]

Ville Syrjälä ville.syrj...@linux.intel.com writes:

 Now that we have mmio flips in the kernel we can start to relax that
 restriction. That still needs a bit more work in the mmio flip code
 but I believe some people working on just that.

I couldn't find any tiling restrictions in the current (ring-based) flip
code; did I just miss them?

 We could even change the pixel format, except a check was added to
 drm_mode_page_flip_ioctl() to prevent that, so I guess it was
 deemed that the API isn't meant to allow that.

Yeah, not sure I care about this; 32bpp is pretty much the only format I
want.

-- 
keith.pack...@intel.com


pgp6ASLSyQNPR.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 I don't see anything indicating that this code path is only used by
 glamor.

True. It's a fix for DRI3 for either UXA or none. Mesa allocates a
single page for a 1x1 texture, but this code thinks that should take two
pages causing a texture-to-pixmap operation to fail.

In particular, gnome-shell fails without this patch.

It's useful independent of this series, which is why I stuck it first.

-- 
keith.pack...@intel.com


pgpnuxq1KsyiP.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 This change appears to be unrelated, and possibly harmful (if X has
 dropped the last ref to the BO, but it's still the scanout buffer, a new
 allocation would now reuse the BO and scribble on scanout until the next
 modeset happens).

Yeah, it's unrelated. intel_allocate_framebuffer calls disable_reuse, so
there's no need to call it from these two other places. I'll split that
change out into a separate patch with separate comment.

 Unrelated whitespace.

There are a bunch of whitespace fixups; should I pull those into a
separate patch or just leave them scattered in the first patch to change
a file?

-- 
keith.pack...@intel.com


pgpius4ilGE8T.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/12] Remove glamor support from UXA acceleration

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Something weird happened in this commit.

 uxa-glamor.h is removed from the repo, but the makefile only drops
 intel_glamor.h, which keeps getting used.  I suspect make distcheck is
 broken.

Yeah, almost certainly. In fact, after this patch, the driver does *not*
build if you ask for glamor.

 intel_glamor.c is no longer built, but USE_GLAMOR might still be defined
 I think, and if so intel_glamor_flush() from it is still getting
 called.

Right, the driver explicitly doesn't build with --enable-glamor after
this patch. Cleaning it up so that it would build makes more of a mess
though, so I just didn't worry about it as I was re-adding glamor when
all of the USE_GLAMOR bits would be needed again.

I can make it work if you like, but this patch and the re-enable glamor
patch would both get larger.

-- 
keith.pack...@intel.com


pgp1tUlqhlD6b.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/12] Get rid of glamor stubs in intel_glamor.h

[Keith Packard]

Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 This eliminates the stubs in intel_glamor.h and replaces them with
 ifdefs instead.

 I don't feel strongly about this either way -- ifdefs are more
 traditional userland style, while stubs are more kernel style.

Right, just trying to keep things all the same and #ifdef USE_GLAMOR
seemed more common.

 Acked-by: Eric Anholt e...@anholt.net

-- 
keith.pack...@intel.com


pgp1ME88gbxeA.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 00/12] Rework intel 2D driver glamor support

[Keith Packard]

I spent the day just cleaning up this patch series and testing. I
think it's ready for others to use and review. I've been running it on
two machines for a couple of days now and it's been solid.

I ran three different desktop environments (current Debian unstable versions):

 XFCE   4.10
 Gnome  3.12
 KDE4.13.3

I have not merged DRI2 support when running with Glamor; I've got that
working locally, but if you accidentally try indirect rendering,
you'll crash the X server with an assert failure. So, this is DRI3
only, at least for now.

This patch series also adds a none acceleration mode. It's different
from other unaccelerated drivers in offering DRI2 and DRI3 support so
that you can run direct rendering.

A brief synopsis of the series

1-4 cleanup patches

5   Identify and isolate UXA code; UXA-specific functions are
renamed and moved into uxa-specific files.

6   Remove UXA-based Glamor support. This pulls out all of the
Glamor calls from UXA rendering paths. This patch only builds
with glamor disabled; I didn't worry about the existing glamor
code or support for glamor within the rest of the driver.

7-9 Prepare for glamor support. Creates a couple of abstract
functions for accel-dependent functionality needed by the
initialization and modesetting code. Gets rid of the
glamor stubs in intel_glamor.h

10  Add glamor support back in, using the regular glamor
API.

11  Add an unaccelerated option (none). This offers fb-only
support and is always compiled in to the driver.

12  Delay initial mode set operation until the root window
is painted and the server is ready to go. This includes
potentially copying an existing fbcon frame buffer to the
root window in background none mode, providing support
for this in all three acceleration modes. The driver used
to support this by copying the fbcon buffer to the screen
buffer during early server initialization; this change allows
the driver to use regular GC-based CopyArea instead of needing
custom rendering code.

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

[Intel-gfx] [PATCH 04/12] Move intel_alloc_framebuffer to intel_memory.c

[Keith Packard]

UXA and Glamor both share this function, so move it out of the UXA file.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel.h|   5 ++
 src/uxa/intel_memory.c | 138 ++
 src/uxa/intel_uxa.c| 158 -
 3 files changed, 154 insertions(+), 147 deletions(-)

diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 409635d..8110c2e 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -539,6 +539,11 @@ unsigned long intel_get_fence_pitch(intel_screen_private 
*intel, unsigned long p
 Bool intel_check_display_stride(ScrnInfoPtr scrn, int stride, Bool tiling);
 void intel_set_gem_max_sizes(ScrnInfoPtr scrn);
 
+unsigned int
+intel_compute_size(struct intel_screen_private *intel,
+   int w, int h, int bpp, unsigned usage,
+   uint32_t *tiling, int *stride);
+
 drm_intel_bo *intel_allocate_framebuffer(ScrnInfoPtr scrn,
 int width, int height, int cpp,
 int *out_stride,
diff --git a/src/uxa/intel_memory.c b/src/uxa/intel_memory.c
index 809b636..188a557 100644
--- a/src/uxa/intel_memory.c
+++ b/src/uxa/intel_memory.c
@@ -190,3 +190,141 @@ void intel_set_gem_max_sizes(ScrnInfoPtr scrn)
 */
intel-max_bo_size = intel-max_gtt_map_size;
 }
+
+unsigned int
+intel_compute_size(struct intel_screen_private *intel,
+   int w, int h, int bpp, unsigned usage,
+   uint32_t *tiling, int *stride)
+{
+   int pitch, size;
+
+   if (*tiling != I915_TILING_NONE) {
+   /* First check whether tiling is necessary. */
+   pitch = (w * bpp  + 7) / 8;
+   pitch = ALIGN(pitch, 64);
+   size = pitch * ALIGN (h, 2);
+   if (INTEL_INFO(intel)-gen  040) {
+   /* Gen 2/3 has a maximum stride for tiling of
+* 8192 bytes.
+*/
+   if (pitch  KB(8))
+   *tiling = I915_TILING_NONE;
+
+   /* Narrower than half a tile? */
+   if (pitch  256)
+   *tiling = I915_TILING_NONE;
+
+   /* Older hardware requires fences to be pot size
+* aligned with a minimum of 1 MiB, so causes
+* massive overallocation for small textures.
+*/
+   if (size  1024*1024/2  !intel-has_relaxed_fencing)
+   *tiling = I915_TILING_NONE;
+   } else if (!(usage  INTEL_CREATE_PIXMAP_DRI2)  size = 4096) 
{
+   /* Disable tiling beneath a page size, we will not see
+* any benefit from reducing TLB misses and instead
+* just incur extra cost when we require a fence.
+*/
+   *tiling = I915_TILING_NONE;
+   }
+   }
+
+   pitch = (w * bpp + 7) / 8;
+   if (!(usage  INTEL_CREATE_PIXMAP_DRI2)  pitch = 256)
+   *tiling = I915_TILING_NONE;
+
+   if (*tiling != I915_TILING_NONE) {
+   int aligned_h, tile_height;
+
+   if (IS_GEN2(intel))
+   tile_height = 16;
+   else if (*tiling == I915_TILING_X)
+   tile_height = 8;
+   else
+   tile_height = 32;
+   aligned_h = ALIGN(h, 2*tile_height);
+
+   *stride = intel_get_fence_pitch(intel,
+   ALIGN(pitch, 512),
+   *tiling);
+
+   /* Round the object up to the size of the fence it will live in
+* if necessary.  We could potentially make the kernel allocate
+* a larger aperture space and just bind the subset of pages in,
+* but this is easier and also keeps us out of trouble (as much)
+* with drm_intel_bufmgr_check_aperture().
+*/
+   size = intel_get_fence_size(intel, *stride * aligned_h);
+
+   if (size  intel-max_tiling_size)
+   *tiling = I915_TILING_NONE;
+   }
+
+   if (*tiling == I915_TILING_NONE) {
+   /* We only require a 64 byte alignment for scanouts, but
+* a 256 byte alignment for sharing with PRIME.
+*/
+   *stride = ALIGN(pitch, 256);
+   /* Round the height up so that the GPU's access to a 2x2 aligned
+* subspan doesn't address an invalid page offset beyond the
+* end of the GTT.
+*/
+   size = *stride * ALIGN(h, 2);
+   }
+
+   return size;
+}
+
+drm_intel_bo *intel_allocate_framebuffer(ScrnInfoPtr scrn

[Intel-gfx] [PATCH 02/12] Fix present debug output

[Keith Packard]

Was using 'low_msc', which isn't defined anymore.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel_present.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/uxa/intel_present.c b/src/uxa/intel_present.c
index 18fcbf8..5f98da8 100644
--- a/src/uxa/intel_present.c
+++ b/src/uxa/intel_present.c
@@ -193,7 +193,8 @@ intel_present_queue_vblank(RRCrtcPtr
crtc,
if (errno != EBUSY || !intel_present_flush_drm_events(screen))
return BadAlloc;
}
-   DebugPresent((\t\tiq %lld seq %u msc %u (hw msc %u)\n, event_id, seq, 
low_msc, vbl.request.sequence));
+   DebugPresent((\t\tiq %lld seq %u msc %llu (hw msc %u)\n,
+  (long long) event_id, seq, (long long) msc, 
vbl.request.sequence));
return Success;
 }
 
-- 
2.0.1

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

[Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

[Keith Packard]

---
 src/uxa/intel_uxa.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/uxa/intel_uxa.c b/src/uxa/intel_uxa.c
index b396188..717754f 100644
--- a/src/uxa/intel_uxa.c
+++ b/src/uxa/intel_uxa.c
@@ -758,6 +758,7 @@ free_priv:
goto free_priv;
}
 
+#if 0
if (tiling != I915_TILING_NONE) {
int height;
 
@@ -780,6 +781,7 @@ free_priv:
bo = NULL;
goto free_priv;
}
+#endif
}
 
   BAIL:
-- 
2.0.1

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

[Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps

[Keith Packard]

GetScratchPixmapHeader should only be used for local memory pixmaps,
as used by PutImage and friends. That's because when you free the
scratch pixmap header, it doesn't actually free the pixmap; instead,
it gets stuffed in pScreen-pScratchPixmap and any private data stored
on it will be left hanging around forever.

In the case of glamor, that private data includes all of the GL
state. Using that scratch pixmap later results in glamor getting
mightily confused as the pixmap and underlying objects do not match.

Avoid this by allocating pixmap headers explicitly for this purpose.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel_display.c | 44 ++--
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 0b83140..bcaafaa 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -545,13 +545,31 @@ intel_crtc_shadow_allocate(xf86CrtcPtr crtc, int width, 
int height)
return NULL;
}
 
-   drm_intel_bo_disable_reuse(intel_crtc-rotate_bo);
-
intel_crtc-rotate_pitch = rotate_pitch;
return intel_crtc-rotate_bo;
 }
 
 static PixmapPtr
+intel_create_pixmap_header(ScreenPtr pScreen, int width, int height, int depth,
+   int bitsPerPixel, int devKind, void *pPixData)
+{
+PixmapPtr pixmap;
+
+/* width and height of 0 means don't allocate any pixmap data */
+pixmap = (*pScreen-CreatePixmap) (pScreen, 0, 0, depth, 0);
+
+if (pixmap) {
+if ((*pScreen-ModifyPixmapHeader) (pixmap, width, height, 
depth,
+bitsPerPixel, devKind, 
pPixData))
+{
+return pixmap;
+}
+(*pScreen-DestroyPixmap) (pixmap);
+}
+return NullPixmap;
+}
+
+static PixmapPtr
 intel_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height)
 {
ScrnInfoPtr scrn = crtc-scrn;
@@ -573,12 +591,12 @@ intel_crtc_shadow_create(xf86CrtcPtr crtc, void *data, 
int width, int height)
return NULL;
}
 
-   rotate_pixmap = GetScratchPixmapHeader(scrn-pScreen,
-  width, height,
-  scrn-depth,
-  scrn-bitsPerPixel,
-  intel_crtc-rotate_pitch,
-  NULL);
+   rotate_pixmap = intel_create_pixmap_header(scrn-pScreen,
+   width, height,
+   scrn-depth,
+   scrn-bitsPerPixel,
+   intel_crtc-rotate_pitch,
+   NULL);
 
if (rotate_pixmap == NULL) {
xf86DrvMsg(scrn-scrnIndex, X_ERROR,
@@ -602,8 +620,8 @@ intel_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr 
rotate_pixmap, void *data)
struct intel_mode *mode = intel_crtc-mode;
 
if (rotate_pixmap) {
-   intel_set_pixmap_bo(rotate_pixmap, NULL);
-   FreeScratchPixmapHeader(rotate_pixmap);
+intel_set_pixmap_bo(rotate_pixmap, NULL);
+rotate_pixmap-drawable.pScreen-DestroyPixmap(rotate_pixmap);
}
 
if (data) {
@@ -1415,7 +1433,6 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int 
height)
int i, old_width, old_height, old_pitch;
int pitch;
uint32_t tiling;
-   ScreenPtr screen;
 
if (scrn-virtualX == width  scrn-virtualY == height)
return TRUE;
@@ -1430,8 +1447,7 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int 
height)
old_front = intel-front_buffer;
 
if (intel-back_pixmap) {
-   screen = intel-back_pixmap-drawable.pScreen;
-   screen-DestroyPixmap(intel-back_pixmap);
+   scrn-pScreen-DestroyPixmap(intel-back_pixmap);
intel-back_pixmap = NULL;
}
 
@@ -1454,7 +1470,6 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int 
height)
if (ret)
goto fail;
 
-   drm_intel_bo_disable_reuse(intel-front_buffer);
intel-front_pitch = pitch;
intel-front_tiling = tiling;
 
@@ -2204,6 +2219,7 @@ Bool intel_crtc_on(xf86CrtcPtr crtc)
return ret;
 }
 
+
 static PixmapPtr
 intel_create_pixmap_for_bo(ScreenPtr pScreen, dri_bo *bo,
   int width, int height,
-- 
2.0.1

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

[Intel-gfx] [PATCH 10/12] Add glamor back into the driver

[Keith Packard]

This adds glamor support back into the driver, but instad of going
through UXA, this uses it directly instead.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel.h |  17 +++
 src/uxa/intel_display.c |  30 -
 src/uxa/intel_dri.c |  20 +++
 src/uxa/intel_driver.c  | 247 -
 src/uxa/intel_glamor.c  | 316 +---
 src/uxa/intel_glamor.h  |  18 ++-
 src/uxa/intel_present.c |   5 -
 src/uxa/intel_uxa.c |   5 +
 src/uxa/intel_video.c   |  75 +++-
 9 files changed, 480 insertions(+), 253 deletions(-)

diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 64f573b..5fbb2c7 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -111,10 +111,21 @@ enum dri_type {
DRI_ACTIVE
 };
 
+enum accel_type {
+#if USE_GLAMOR
+ACCEL_GLAMOR,
+#endif
+#if USE_UXA
+ACCEL_UXA,
+#endif
+};
+
 typedef struct intel_screen_private {
ScrnInfoPtr scrn;
int cpp;
 
+enum accel_type accel;
+
 #define RENDER_BATCH   I915_EXEC_RENDER
 #define BLT_BATCH  I915_EXEC_BLT
unsigned int current_batch;
@@ -127,6 +138,7 @@ typedef struct intel_screen_private {
 
dri_bufmgr *bufmgr;
 
+#if USE_UXA
uint32_t batch_ptr[4096];
/** Byte offset in batch_ptr for the next dword to be emitted. */
unsigned int batch_used;
@@ -142,6 +154,7 @@ typedef struct intel_screen_private {
struct list batch_pixmaps;
drm_intel_bo *wa_scratch_bo;
OsTimerPtr cache_expire;
+#endif
 
/* For Xvideo */
Bool use_overlay;
@@ -178,8 +191,10 @@ typedef struct intel_screen_private {
void (*batch_flush) (struct intel_screen_private *intel);
void (*batch_commit_notify) (struct intel_screen_private *intel);
 
+#if USE_UXA
struct _UxaDriver *uxa_driver;
int uxa_flags;
+#endif
Bool need_sync;
int accel_pixmap_offset_alignment;
int accel_max_x;
@@ -212,6 +227,7 @@ typedef struct intel_screen_private {
drm_intel_bo *gen6_depth_stencil_bo;
} video;
 
+#if USE_UXA
/* Render accel state */
float scale_units[2][2];
/** Transform pointers for src/mask, or NULL if identity */
@@ -269,6 +285,7 @@ typedef struct intel_screen_private {
 
/* 965 render acceleration state */
struct gen4_render_state *gen4_render_state;
+#endif
 
/* DRI enabled this generation. */
enum dri_type dri2, dri3;
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index add09b0..a4c4b82 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -56,6 +56,9 @@
 #include xf86DDC.h
 #include fb.h
 #include uxa.h
+#if USE_GLAMOR
+#include intel_glamor.h
+#endif
 
 #define KNOWN_MODE_FLAGS ((114)-1)
 
@@ -182,7 +185,7 @@ intel_output_backlight_init(xf86OutputPtr output)
 {
struct intel_output *intel_output = output-driver_private;
intel_screen_private *intel = intel_get_screen_private(output-scrn);
-   char *str;
+   const char *str;
 
 #if !USE_BACKLIGHT
return;
@@ -683,7 +686,10 @@ intel_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
return TRUE;
}
 
-   bo = intel_get_pixmap_bo(ppix);
+bo = intel_get_pixmap_bo(ppix);
+if (!bo)
+return FALSE;
+
if (intel-front_buffer) {
ErrorF(have front buffer\n);
}
@@ -1472,8 +1478,20 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int 
height)
scrn-virtualX = width;
scrn-virtualY = height;
 
-   if (!intel_uxa_create_screen_resources(scrn-pScreen))
-   goto fail;
+switch (intel-accel) {
+#if USE_GLAMOR
+case ACCEL_GLAMOR:
+if (!intel_glamor_create_screen_resources(scrn-pScreen))
+goto fail;
+break;
+#endif
+#if USE_UXA
+case ACCEL_UXA:
+if (!intel_uxa_create_screen_resources(scrn-pScreen))
+goto fail;
+break;
+#endif
+}
 
for (i = 0; i  xf86_config-num_crtc; i++) {
xf86CrtcPtr crtc = xf86_config-crtc[i];
@@ -1532,7 +1550,7 @@ intel_do_pageflip(intel_screen_private *intel,
xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
struct intel_crtc *crtc = config-crtc[0]-driver_private;
struct intel_mode *mode = crtc-mode;
-   unsigned int pitch = scrn-displayWidth * intel-cpp;
+unsigned int pitch = scrn-displayWidth * intel-cpp;
struct intel_pageflip *flip;
uint32_t new_fb_id;
uint32_t flags;
@@ -2235,7 +2253,7 @@ intel_create_pixmap_for_bo(ScreenPtr pScreen, dri_bo *bo,
return NullPixmap;
}
 
-   intel_set_pixmap_bo(pixmap, bo);
+intel_set_pixmap_bo(pixmap, bo);
return pixmap;
 }
 
diff --git a/src/uxa/intel_dri.c b/src/uxa

[Intel-gfx] [PATCH 06/12] Remove glamor support from UXA acceleration

[Keith Packard]

Makes UXA no longer include calls to glamor acceleration functions.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/Makefile.am|   9 --
 src/uxa/intel_dri.c|  60 +
 src/uxa/intel_driver.c |   1 -
 src/uxa/intel_uxa.c|  49 ---
 src/uxa/intel_uxa.h|   1 -
 src/uxa/uxa-accel.c| 230 -
 src/uxa/uxa-glamor.h   |  65 --
 src/uxa/uxa-glyphs.c   |  28 --
 src/uxa/uxa-render.c   |  79 -
 src/uxa/uxa.c  |   7 --
 src/uxa/uxa.h  |  16 
 11 files changed, 2 insertions(+), 543 deletions(-)
 delete mode 100644 src/uxa/uxa-glamor.h

diff --git a/src/uxa/Makefile.am b/src/uxa/Makefile.am
index 23b074d..b9618c9 100644
--- a/src/uxa/Makefile.am
+++ b/src/uxa/Makefile.am
@@ -34,7 +34,6 @@ libuxa_la_SOURCES = \
intel_batchbuffer.h \
intel_display.c \
intel_driver.c \
-   intel_glamor.h \
intel_memory.c \
intel_uxa.c \
intel_video.c \
@@ -65,14 +64,6 @@ libuxa_la_SOURCES = \
uxa-unaccel.c
$(NULL)
 
-if GLAMOR
-AM_CFLAGS += $(LIBGLAMOR_CFLAGS)
-libuxa_la_LIBADD += $(LIBGLAMOR_LIBS)
-libuxa_la_SOURCES += \
-intel_glamor.c \
-$(NULL)
-endif
-
 if DRI2
 AM_CFLAGS += $(DRI2_CFLAGS)
 libuxa_la_SOURCES += \
diff --git a/src/uxa/intel_dri.c b/src/uxa/intel_dri.c
index 65612f7..79978ab 100644
--- a/src/uxa/intel_dri.c
+++ b/src/uxa/intel_dri.c
@@ -793,38 +793,6 @@ I830DRI2ExchangeBuffers(struct intel_screen_private 
*intel, DRI2BufferPtr front,
dri_bo_reference (intel-front_buffer);
 }
 
-static PixmapPtr
-intel_glamor_create_back_pixmap(ScreenPtr screen,
-   PixmapPtr front_pixmap,
-   drm_intel_bo *back_bo)
-{
-   PixmapPtr back_pixmap;
-
-   back_pixmap = screen-CreatePixmap(screen,
-  0,
-  0,
-  front_pixmap-drawable.depth,
-  0);
-   if (back_pixmap == NULL)
-   return NULL;
-
-   screen-ModifyPixmapHeader(back_pixmap,
-  front_pixmap-drawable.width,
-  front_pixmap-drawable.height,
-  0, 0,
-  front_pixmap-devKind,
-  0);
-   intel_set_pixmap_bo(back_pixmap, back_bo);
-   if (!intel_glamor_create_textured_pixmap(back_pixmap)) {
-   ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-   xf86DrvMsg(scrn-scrnIndex, X_WARNING,
-  Failed to create textured back pixmap.\n);
-   screen-DestroyPixmap(back_pixmap);
-   return NULL;
-   }
-   return back_pixmap;
-}
-
 static drm_intel_bo *get_pixmap_bo(I830DRI2BufferPrivatePtr priv)
 {
drm_intel_bo *bo = intel_get_pixmap_bo(priv-pixmap);
@@ -883,10 +851,6 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
}
 
if (intel-back_buffer == NULL) {
-   I830DRI2BufferPrivatePtr drvpriv;
-   PixmapPtr front_pixmap, back_pixmap;
-   ScreenPtr screen;
-
new_back = drm_intel_bo_alloc(intel-bufmgr, front buffer,
  intel-front_buffer-size, 0);
if (new_back == NULL)
@@ -903,21 +867,6 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
 
drm_intel_bo_disable_reuse(new_back);
dri_bo_flink(new_back, intel-back_name);
-
-   if ((intel-uxa_flags  UXA_USE_GLAMOR)) {
-   screen = draw-pScreen;
-   drvpriv = info-front-driverPrivate;
-   front_pixmap = drvpriv-pixmap;
-
-   back_pixmap = intel_glamor_create_back_pixmap(screen,
- 
front_pixmap,
- new_back);
-   if (back_pixmap == NULL) {
-   drm_intel_bo_unreference(new_back);
-   return FALSE;
-   }
-   intel-back_pixmap = back_pixmap;
-   }
} else {
new_back = intel-back_buffer;
intel-back_buffer = NULL;
@@ -937,13 +886,8 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
 
intel-back_buffer = intel-front_buffer;
drm_intel_bo_reference(intel-back_buffer);
-   if (!(intel-uxa_flags  UXA_USE_GLAMOR)) {
-   intel_set_pixmap_bo(priv-pixmap, new_back);
-   drm_intel_bo_unreference(new_back);
-   }
-   else
-   intel_exchange_pixmap_buffers(intel, priv-pixmap

[Intel-gfx] [PATCH 07/12] Add intel_flush to abstract flushing pending acceleration operations

[Keith Packard]

intel_flush flushes any pending acceleration operations to the
hardware, just like intel_uxa_batch_submit does today except that it is
not uxa-specific.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel.h |  3 +++
 src/uxa/intel_display.c | 11 +++
 src/uxa/intel_driver.c  | 14 +-
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index ade8742..64f573b 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -535,4 +535,7 @@ intel_get_pixmap_bo(PixmapPtr pixmap);
 void
 intel_set_pixmap_bo(PixmapPtr pixmap, dri_bo *bo);
 
+void
+intel_flush(intel_screen_private *intel);
+
 #endif /* _I830_H_ */
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index a4cbf00..add09b0 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -57,8 +57,6 @@
 #include fb.h
 #include uxa.h
 
-#include intel_glamor.h
-
 #define KNOWN_MODE_FLAGS ((114)-1)
 
 struct intel_drm_queue {
@@ -437,8 +435,7 @@ intel_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr 
mode,
crtc-y = y;
crtc-rotation = rotation;
 
-   intel_glamor_flush(intel);
-   intel_batch_submit(crtc-scrn);
+intel_flush(intel);
 
mode_to_kmode(crtc-scrn, intel_crtc-kmode, mode);
ret = intel_crtc_apply(crtc);
@@ -1437,8 +1434,7 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int 
height)
if (scrn-virtualX == width  scrn-virtualY == height)
return TRUE;
 
-   intel_glamor_flush(intel);
-   intel_batch_submit(scrn);
+intel_flush(intel);
 
old_width = scrn-virtualX;
old_height = scrn-virtualY;
@@ -1552,8 +1548,7 @@ intel_do_pageflip(intel_screen_private *intel,
goto error_out;
 
drm_intel_bo_disable_reuse(new_front);
-   intel_glamor_flush(intel);
-   intel_batch_submit(scrn);
+intel_flush(intel);
 
mode-pageflip_data = pageflip_data;
mode-pageflip_handler = pageflip_handler;
diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
index d9f918b..a764b97 100644
--- a/src/uxa/intel_driver.c
+++ b/src/uxa/intel_driver.c
@@ -181,6 +181,12 @@ intel_get_pixmap_bo(PixmapPtr pixmap)
 return intel_uxa_get_pixmap_bo(pixmap);
 }
 
+void
+intel_flush(intel_screen_private *intel)
+{
+intel_batch_submit(intel-scrn);
+}
+
 static void PreInitCleanup(ScrnInfoPtr scrn)
 {
if (!scrn || !scrn-driverPrivate)
@@ -647,7 +653,7 @@ redisplay_dirty(ScreenPtr screen, PixmapDirtyUpdatePtr 
dirty)
PixmapSyncDirtyHelper(dirty, pixregion);
RegionUninit(pixregion);
 
-   intel_batch_submit(scrn);
+intel_flush(intel);
if (!intel-has_prime_vmap_flush) {
drm_intel_bo *bo = 
intel_uxa_get_pixmap_bo(dirty-slave_dst-master_pixmap);
was_blocked = xf86BlockSIGIO();
@@ -732,10 +738,8 @@ intel_flush_callback(CallbackListPtr *list,
 pointer user_data, pointer call_data)
 {
ScrnInfoPtr scrn = user_data;
-   if (scrn-vtSema) {
-   intel_batch_submit(scrn);
-   intel_glamor_flush(intel_get_screen_private(scrn));
-   }
+   if (scrn-vtSema)
+intel_flush(intel_get_screen_private(scrn));
 }
 
 #if HAVE_UDEV
-- 
2.0.1

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

[Intel-gfx] [PATCH 08/12] Get rid of glamor stubs in intel_glamor.h

[Keith Packard]

This eliminates the stubs in intel_glamor.h and replaces them with
ifdefs instead.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel_dri.c | 20 ++--
 src/uxa/intel_driver.c  |  6 ++
 src/uxa/intel_glamor.h  | 21 -
 src/uxa/intel_present.c |  2 ++
 src/uxa/intel_video.c   |  4 
 5 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/src/uxa/intel_dri.c b/src/uxa/intel_dri.c
index 79978ab..78734cb 100644
--- a/src/uxa/intel_dri.c
+++ b/src/uxa/intel_dri.c
@@ -66,7 +66,9 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include dri2.h
 
+#if USE_GLAMOR
 #include intel_glamor.h
+#endif
 #include uxa.h
 
 typedef struct {
@@ -107,6 +109,7 @@ static PixmapPtr get_front_buffer(DrawablePtr drawable)
return pixmap;
 }
 
+#if USE_GLAMOR
 static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
 {
ScreenPtr screen = drawable-pScreen;
@@ -158,6 +161,7 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable, 
PixmapPtr pixmap)
intel_get_screen_private(xf86ScreenToScrn(screen))-needs_flush = TRUE;
return old;
 }
+#endif
 
 #if DRI2INFOREC_VERSION  2
 static DRI2BufferPtr
@@ -170,7 +174,9 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int 
*attachments,
DRI2BufferPtr buffers;
I830DRI2BufferPrivatePtr privates;
PixmapPtr pixmap, pDepthPixmap;
+#if USE_GLAMOR
Bool is_glamor_pixmap = FALSE;
+#endif
int i;
 
buffers = calloc(count, sizeof *buffers);
@@ -187,11 +193,12 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int 
*attachments,
pixmap = NULL;
if (attachments[i] == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
-
+#if USE_GLAMOR
if (pixmap == NULL) {
drawable = 
(get_drawable_pixmap(drawable)-drawable);
is_glamor_pixmap = TRUE;
}
+#endif
} else if (attachments[i] == DRI2BufferStencil  pDepthPixmap) 
{
pixmap = pDepthPixmap;
pixmap-refcnt++;
@@ -230,8 +237,10 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int 
*attachments,
goto unwind;
}
 
+#if USE_GLAMOR
if (is_glamor_pixmap)
pixmap = fixup_glamor(drawable, pixmap);
+#endif
}
 
if (attachments[i] == DRI2BufferDepth)
@@ -292,7 +301,9 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int 
attachment,
DRI2Buffer2Ptr buffer;
I830DRI2BufferPrivatePtr privates;
PixmapPtr pixmap;
+#if USE_GLAMOR
Bool is_glamor_pixmap = FALSE;
+#endif
 
buffer = calloc(1, sizeof *buffer);
if (buffer == NULL)
@@ -306,11 +317,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int 
attachment,
pixmap = NULL;
if (attachment == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
-
+#if USE_GLAMOR
if (pixmap == NULL) {
drawable = (get_drawable_pixmap(drawable)-drawable);
is_glamor_pixmap = TRUE;
}
+#endif
}
 
if (pixmap == NULL) {
@@ -383,8 +395,10 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int 
attachment,
free(buffer);
return NULL;
}
+#if USE_GLAMOR
if (is_glamor_pixmap)
pixmap = fixup_glamor(drawable, pixmap);
+#endif
}
 
buffer-attachment = attachment;
@@ -762,7 +776,9 @@ intel_exchange_pixmap_buffers(struct intel_screen_private 
*intel, PixmapPtr fron
new_front-busy = 1;
new_back-busy = -1;
 
+#if USE_GLAMOR
intel_glamor_exchange_buffers(intel, front, back);
+#endif
 
DamageRegionProcessPending(front-drawable);
 
diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
index a764b97..6d38cfe 100644
--- a/src/uxa/intel_driver.c
+++ b/src/uxa/intel_driver.c
@@ -72,7 +72,9 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include i915_drm.h
 
+#if USE_GLAMOR
 #include intel_glamor.h
+#endif
 #include intel_options.h
 
 static void i830AdjustFrame(ADJUST_FRAME_ARGS_DECL);
@@ -610,12 +612,14 @@ static Bool I830PreInit(ScrnInfoPtr scrn, int flags)
return FALSE;
}
 
+#if USE_GLAMOR
if (!intel_glamor_pre_init(scrn)) {
PreInitCleanup(scrn);
xf86DrvMsg(scrn-scrnIndex, X_ERROR,
Failed to pre init glamor display.\n);
return FALSE;
}
+#endif
 
/* Load the dri modules if requested. */
 #if HAVE_DRI2
@@ -1125,7 +1129,9 @@ static Bool I830CloseScreen(CLOSE_SCREEN_ARGS_DECL)
 
DeleteCallback(FlushCallback, intel_flush_callback, scrn);
 
+#if USE_GLAMOR

[Intel-gfx] [PATCH 11/12] Add none acceleration option

[Keith Packard]

This just uses fb directly

Signed-off-by: Keith Packard kei...@keithp.com
---
 configure.ac|  24 -
 src/Makefile.am |   9 +-
 src/intel_module.c  |  12 ++-
 src/uxa/Makefile.am |  46 ++---
 src/uxa/intel.h |   5 +
 src/uxa/intel_display.c |   6 ++
 src/uxa/intel_dri3.c|  49 ++
 src/uxa/intel_driver.c  | 245 ++--
 src/uxa/intel_glamor.c  |  10 ++
 src/uxa/intel_none.c| 189 +
 src/uxa/intel_none.h|  38 
 src/uxa/intel_uxa.c |   9 ++
 12 files changed, 504 insertions(+), 138 deletions(-)
 create mode 100644 src/uxa/intel_none.c
 create mode 100644 src/uxa/intel_none.h

diff --git a/configure.ac b/configure.ac
index 5c3dd20..46c3143 100644
--- a/configure.ac
+++ b/configure.ac
@@ -396,7 +396,6 @@ if test x$UXA = xauto; then
 fi
 if test x$UXA != xno; then
AC_DEFINE(USE_UXA, 1, [Enable UXA support])
-   PKG_CHECK_MODULES(DRMINTEL, [libdrm_intel = $uxa_requires_libdrm])
required_pixman_version=0.24
UXA=yes
 fi
@@ -407,9 +406,6 @@ AC_ARG_ENABLE(glamor,
  [GLAMOR=$enableval],
  [GLAMOR=no])
 if test x$GLAMOR != xno; then
-   if test x$UXA != xyes; then
-   AC_MSG_ERROR([Glamor acceleration requested but UXA is not 
enabled])
-   fi
if pkg-config --exists xorg-server = 1.15.99.901; then
GLAMOR=yes (using Xorg glamor module)
else
@@ -417,9 +413,12 @@ if test x$GLAMOR != xno; then
PKG_CHECK_MODULES(LIBGLAMOR_EGL, [glamor-egl])
GLAMOR=yes (using libglamor)
fi
+   GLAMOR=yes
AC_DEFINE(USE_GLAMOR, 1, [Enable glamor acceleration])
 fi
 
+PKG_CHECK_MODULES(DRMINTEL, [libdrm_intel = $uxa_requires_libdrm])
+
 PKG_CHECK_MODULES(XORG, [xorg-server = $required_xorg_server_version xproto 
fontsproto pixman-1 = $required_pixman_version $REQUIRED_MODULES])
 ABI_VERSION=`$PKG_CONFIG --variable=abi_videodrv xorg-server`
 
@@ -661,6 +660,12 @@ if test x$accel = xauto; then
else
if test x$UXA != xno; then
accel=uxa
+   else
+   if test x$GLAMOR != xno; then
+   accel=glamor
+   else
+   accel=none
+   fi
fi
fi
if test x$accel = xauto -a x$KMS = xyes; then
@@ -696,6 +701,11 @@ if test x$accel = xglamor; then
fi
 fi
 
+if test x$accel = xnone; then
+   AC_DEFINE(DEFAULT_ACCEL_METHOD, NONE, [Default acceleration method])
+   have_accel=yes
+fi
+
 if test x$have_accel = xnone; then
if test x$KMS = xyes; then
AC_MSG_ERROR([Invalid default acceleration option])
@@ -886,6 +896,12 @@ if test x$GLAMOR != xno; then
fi
 fi
 
+if test $accel = none; then
+   accel_msg=$accel_msg *none
+else
+   accel_msg=$accel_msg none
+fi
+
 if test x$dri_msg = x; then
dri_msg= none
 fi
diff --git a/src/Makefile.am b/src/Makefile.am
index aa8b47e..ce6f836 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -18,7 +18,7 @@
 #  IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
 #  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-SUBDIRS = render_program legacy
+SUBDIRS = render_program legacy uxa
 
 # this is obnoxious:
 # -module lets us name the module exactly how we want
@@ -41,18 +41,13 @@ endif
 intel_drv_la_LTLIBRARIES = intel_drv.la
 intel_drv_la_LDFLAGS = -module -avoid-version
 intel_drv_ladir = $(moduledir)/drivers
-intel_drv_la_LIBADD = legacy/liblegacy.la $(PCIACCESS_LIBS) $(XORG_LIBS)
+intel_drv_la_LIBADD = legacy/liblegacy.la uxa/libuxa.la $(PCIACCESS_LIBS) 
$(XORG_LIBS)
 
 if SNA
 SUBDIRS += sna
 intel_drv_la_LIBADD += sna/libsna.la
 endif
 
-if UXA
-SUBDIRS += uxa
-intel_drv_la_LIBADD += uxa/libuxa.la
-endif
-
 NULL:=#
 
 intel_drv_la_SOURCES = \
diff --git a/src/intel_module.c b/src/intel_module.c
index d6a0d3c..b8d7f91 100644
--- a/src/intel_module.c
+++ b/src/intel_module.c
@@ -489,7 +489,7 @@ _xf86findDriver(const char *ident, XF86ConfDevicePtr p)
return NULL;
 }
 
-static enum accel_method { SNA, UXA, GLAMOR } get_accel_method(void)
+static enum accel_method { NONE, SNA, UXA, GLAMOR } get_accel_method(void)
 {
enum accel_method accel_method = DEFAULT_ACCEL_METHOD;
XF86ConfDevicePtr dev;
@@ -509,6 +509,8 @@ static enum accel_method { SNA, UXA, GLAMOR } 
get_accel_method(void)
accel_method = UXA;
else if (strcasecmp(s, glamor) == 0)
accel_method = GLAMOR;
+else if (strcasecmp(s, none) == 0)
+ accel_method = NONE;
}
}
 
@@ -564,12 +566,14 @@ intel_scrn_create(DriverPtr   driver,
 #if USE_SNA
case SNA: return

[Intel-gfx] [PATCH 12/12] Delay initial modeset until root window contents are prepared

[Keith Packard]

Wait until the root window has been painted for the first time before
doing the modeset. This avoids flashing black while the root window
gets set up.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel.h |  2 ++
 src/uxa/intel_display.c | 39 ++-
 src/uxa/intel_driver.c  | 40 +++-
 3 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index ae33938..fa2aa64 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -194,6 +194,8 @@ typedef struct intel_screen_private {
void (*batch_flush) (struct intel_screen_private *intel);
void (*batch_commit_notify) (struct intel_screen_private *intel);
 
+Bool need_entervt;
+
 #if USE_UXA
struct _UxaDriver *uxa_driver;
int uxa_flags;
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 1d4ba3c..758f01f 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -2319,7 +2319,7 @@ void intel_copy_fb(ScrnInfoPtr scrn)
ScreenPtr pScreen = xf86ScrnToScreen(scrn);
intel_screen_private *intel = intel_get_screen_private(scrn);
PixmapPtr src, dst;
-   unsigned int pitch = scrn-displayWidth * intel-cpp;
+GCPtr gc;
struct intel_crtc *intel_crtc;
int i, fbcon_id;
 
@@ -2339,30 +2339,27 @@ void intel_copy_fb(ScrnInfoPtr scrn)
if (src == NULL)
return;
 
-   /* We dont have a screen Pixmap yet */
-   dst = intel_create_pixmap_for_bo(pScreen, intel-front_buffer,
-scrn-virtualX, scrn-virtualY,
-scrn-depth, scrn-bitsPerPixel,
-pitch);
+dst = (*pScreen-GetScreenPixmap)(pScreen);
+
if (dst == NullPixmap)
goto cleanup_src;
 
-   if (!intel-uxa_driver-prepare_copy(src, dst,
--1, -1,
-GXcopy, FB_ALLONES))
-   goto cleanup_dst;
-
-   intel-uxa_driver-copy(dst,
-   0, 0,
-   0, 0,
-   scrn-virtualX, scrn-virtualY);
-   intel-uxa_driver-done_copy(dst);
-#if ABI_VIDEODRV_VERSION = SET_ABI_VERSION(10, 0)
-   pScreen-canDoBGNoneRoot = TRUE;
-#endif
+gc = GetScratchGC(scrn-depth, pScreen);
+
+if (!gc)
+goto cleanup_src;
+
+ValidateGC(dst-drawable, gc);
+
+(*gc-ops-CopyArea)(src-drawable,
+ dst-drawable,
+ gc,
+ 0, 0,
+ scrn-virtualX, scrn-virtualY,
+ 0, 0);
+
+FreeScratchGC(gc);
 
-cleanup_dst:
-   (*pScreen-DestroyPixmap)(dst);
 cleanup_src:
(*pScreen-DestroyPixmap)(src);
 }
diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
index 316de64..0475a17 100644
--- a/src/uxa/intel_driver.c
+++ b/src/uxa/intel_driver.c
@@ -170,13 +170,19 @@ static Bool i830CreateScreenResources(ScreenPtr screen)
 case ACCEL_GLAMOR:
 if (!intel_glamor_create_screen_resources(screen))
 return FALSE;
+#if ABI_VIDEODRV_VERSION = SET_ABI_VERSION(10, 0)
+screen-canDoBGNoneRoot = TRUE;
+#endif
 break;
 #endif
 #if USE_UXA
 case ACCEL_UXA:
 if (!intel_uxa_create_screen_resources(screen))
 return FALSE;
-intel_copy_fb(scrn);
+#if ABI_VIDEODRV_VERSION = SET_ABI_VERSION(10, 0)
+screen-canDoBGNoneRoot = TRUE;
+#endif
+break;
 #endif
 case ACCEL_NONE:
 if (!intel_none_create_screen_resources(screen))
@@ -830,6 +836,29 @@ I830BlockHandler(BLOCKHANDLER_ARGS_DECL)
 
screen-BlockHandler = intel-BlockHandler;
 
+/* At server init time, get the root window bits from fbcon if
+ * available and then update the protocol-level information
+ * for both RandR and XINERAMA
+ */
+if (intel-need_entervt) {
+intel-need_entervt = FALSE;
+
+if (screen-root-backgroundState == None)
+intel_copy_fb(scrn);
+
+/* Must force it before EnterVT, so we are in control of VT and
+ * later memory should be bound when allocating, e.g 
rotate_mem */
+scrn-vtSema = TRUE;
+
+if (!I830EnterVT(VT_FUNC_ARGS(0))) {
+xf86DrvMsg(scrn-scrnIndex, X_ERROR,
+   Failed to perform initial modeset.\n);
+FatalError(Mode set failed\n);
+}
+
+xf86RandR12CreateScreenResources(screen);
+}
+
(*screen-BlockHandler) (BLOCKHANDLER_ARGS);
 
intel

[Intel-gfx] [PATCH 09/12] Do more checks for proposed flip pixmaps

[Keith Packard]

Make sure the pitch and tiling are correct.
Make sure there's a BO we can get at.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel_present.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/uxa/intel_present.c b/src/uxa/intel_present.c
index c53d71d..b901fb1 100644
--- a/src/uxa/intel_present.c
+++ b/src/uxa/intel_present.c
@@ -248,6 +248,8 @@ intel_present_check_flip(RRCrtcPtr  crtc,
ScreenPtr   screen = window-drawable.pScreen;
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
intel_screen_private*intel = intel_get_screen_private(scrn);
+dri_bo  *bo;
+uint32_ttiling, swizzle;
 
if (!scrn-vtSema)
return FALSE;
@@ -261,6 +263,22 @@ intel_present_check_flip(RRCrtcPtr  crtc,
if (crtc  !intel_crtc_on(crtc-devPrivate))
return FALSE;
 
+/* Check stride, can't change that on flip */
+if (pixmap-devKind != intel-front_pitch)
+return FALSE;
+
+/* Make sure there's a bo we can get to */
+bo = intel_get_pixmap_bo(pixmap);
+if (!bo)
+return FALSE;
+
+/* Check tiling, can't change that on flip */
+if (drm_intel_bo_get_tiling((drm_intel_bo *) bo, tiling, swizzle) != 
0)
+return FALSE;
+
+if (tiling != intel-front_tiling)
+return FALSE;
+
return TRUE;
 }
 
-- 
2.0.1

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

[Intel-gfx] [PATCH] load_cursor_argb is supposed to return a Bool, not void

[Keith Packard]

By mis-declaring this function, we ended up using software cursors
because the value seen by the caller was 0.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/sna/sna_display.c  | 8 ++--
 src/sna/sna_display_fake.c | 3 ++-
 src/uxa/intel_display.c| 7 +--
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 6e2d118..a3441e1 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -1681,11 +1681,12 @@ sna_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, 
int y)
(void)drmIoctl(to_sna(crtc-scrn)-kgem.fd, DRM_IOCTL_MODE_CURSOR, 
arg);
 }
 
-static void
+static Bool
 sna_crtc_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *image)
 {
struct sna *sna = to_sna(crtc-scrn);
struct drm_i915_gem_pwrite pwrite;
+int ret;
 
__DBG((%s: CRTC:%d\n, __FUNCTION__, to_sna_crtc(crtc)-id));
 
@@ -1694,7 +1695,10 @@ sna_crtc_load_cursor_argb(xf86CrtcPtr crtc, CARD32 
*image)
pwrite.offset = 0;
pwrite.size = sna-mode.cursor_width*sna-mode.cursor_height*4;
pwrite.data_ptr = (uintptr_t)image;
-   (void)drmIoctl(sna-kgem.fd, DRM_IOCTL_I915_GEM_PWRITE, pwrite);
+   ret = drmIoctl(sna-kgem.fd, DRM_IOCTL_I915_GEM_PWRITE, pwrite);
+if (ret != 0)
+return FALSE;
+return TRUE;
 }
 
 static void
diff --git a/src/sna/sna_display_fake.c b/src/sna/sna_display_fake.c
index c709d99..3cf0042 100644
--- a/src/sna/sna_display_fake.c
+++ b/src/sna/sna_display_fake.c
@@ -115,9 +115,10 @@ sna_crtc_show_cursor(xf86CrtcPtr crtc)
 {
 }
 
-static void
+static Bool
 sna_crtc_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *image)
 {
+return TRUE;
 }
 
 static void
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 0f06793..f755cb6 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -432,16 +432,19 @@ intel_crtc_set_cursor_position (xf86CrtcPtr crtc, int x, 
int y)
drmModeMoveCursor(mode-fd, crtc_id(intel_crtc), x, y);
 }
 
-static void
+static Bool
 intel_crtc_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *image)
 {
struct intel_crtc *intel_crtc = crtc-driver_private;
int ret;
 
ret = dri_bo_subdata(intel_crtc-cursor, 0, 64*64*4, image);
-   if (ret)
+   if (ret) {
xf86DrvMsg(crtc-scrn-scrnIndex, X_ERROR,
   failed to set cursor: %s\n, strerror(-ret));
+return FALSE;
+}
+return TRUE;
 }
 
 static void
-- 
1.9.1

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

Re: [Intel-gfx] REGRESSION 3.14 i915 warning mouse cursor vanishing

[Keith Packard]

Steven Noonan ste...@uplinklabs.net writes:

 Was using my machine normally, then my mouse cursor vanished. After switching
 to a VT and back to X11, my cursor came back. But I did notice a nasty trace 
 in
 dmesg (below).

I don't think the trace below is related to the cursor disappearing.

I found a pair of bugs (one in the intel driver, one in the X server)
which can cause cursor disappearances. I just sent an intel driver patch
to the intel-gfx list with the subject:

[PATCH] load_cursor_argb is supposed to return a Bool, not void

I've posted the X server patch once, and will respond to some review
comments. Either is sufficient to get a cursor back, the intel driver
one means you get a working hardware cursor again, rather than using a
software cursor by mistake.

-- 
keith.pack...@intel.com


pgpelSNxaVj1A.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] load_cursor_argb is supposed to return a Bool, not void

[Keith Packard]

Julien Cristau jcris...@debian.org writes:

 Only since
 http://cgit.freedesktop.org/xorg/xserver/commit/?id=901fbfbbbd71c0d82080957f8ba09eebbc786f2b

 Which could probably have used a different name to avoid silent
 breakage.

Yeah, that probably would have been a better change.

-- 
keith.pack...@intel.com


pgp0vcYNAo_xV.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] REGRESSION 3.14 i915 warning mouse cursor vanishing

[Keith Packard]

Steven Noonan ste...@uplinklabs.net writes:

 OK, good to know. Thanks for pointing those out!

As Julien points out, this bug only affects people running master from
the X server though...

-- 
keith.pack...@intel.com


pgpHmhnPO7B4K.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Allow for 5.4Gbps for Haswell.

[Keith Packard]

Ville Syrjälä ville.syrj...@linux.intel.com writes:

 Todd already implemented 5.4Gbps support a while back. So it seems your
 tree is a bit out of date.

I didn't find it on drm-intel-fixes-2014-02-14; can you explain which
tree it is present in?

-- 
keith.pack...@intel.com


pgpDBs_8j9uhj.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/intel: Only smash VGA SR01 register if intel is default VGA device

[Keith Packard]

We want to disable the (unused) VGA plane on the intel hardware, which
should be a simple matter of writing the vga control register. However,
in 2009 (commit 24f119c769bacac5729297b682fec7811a983cc6), that simple
code was changed to also smash the SR01 VGA register to fix random
crash and lockups.

When running efifb on an nVidia card, executing this store to SR01
ends up causing the screen to go black.

I'm not sure if this store is still required; this patch limits it to
when the intel card is primary.

Signed-off-by: Keith Packard kei...@keithp.com
---

 drivers/gpu/drm/i915/intel_display.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c714d4d..545b271 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9038,12 +9038,14 @@ static void i915_disable_vga(struct drm_device *dev)
u8 sr1;
u32 vga_reg = i915_vgacntrl_reg(dev);
 
-   vga_get_uninterruptible(dev-pdev, VGA_RSRC_LEGACY_IO);
-   outb(SR01, VGA_SR_INDEX);
-   sr1 = inb(VGA_SR_DATA);
-   outb(sr1 | 15, VGA_SR_DATA);
-   vga_put(dev-pdev, VGA_RSRC_LEGACY_IO);
-   udelay(300);
+   if (dev-pdev == vga_default_device()) {
+   vga_get_uninterruptible(dev-pdev, VGA_RSRC_LEGACY_IO);
+   outb(SR01, VGA_SR_INDEX);
+   sr1 = inb(VGA_SR_DATA);
+   outb(sr1 | 15, VGA_SR_DATA);
+   vga_put(dev-pdev, VGA_RSRC_LEGACY_IO);
+   udelay(300);
+   }
 
I915_WRITE(vga_reg, VGA_DISP_DISABLE);
POSTING_READ(vga_reg);
-- 
1.8.5.1

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

Re: [Intel-gfx] [PATCH] drm/intel: Only smash VGA SR01 register if intel is default VGA device

[Keith Packard]

Chris Wilson ch...@chris-wilson.co.uk writes:

 The bspec still says we must assert SR01 bit5 prior to disabling the VGA
 plane.

 Perhaps the test should be whether (vga_reg  VGA_DISP_DISABLE) == 0 and
 do nothing if the plane is already off.

The problem is that for some reason we're smashing *some other video
card* when it's being used via efifb.

I'm wondering if vgaarb just doesn't work because efifb isn't telling
vgaarb that it's using those registers (I mean, how would it even know?)

The other simple option is to just not disable VGA if the card isn't
primary; presumably it wasn't ever enabled.

And, yes, I know that the card probably won't work at all if it isn't
primary because so much currently depends on the BIOS setting up bits of
the card that we can't autodetect. So, another simple option would be to
just refuse to load the driver if the card is secondary...

-- 
keith.pack...@intel.com


pgpLLmW0aJ7ID.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/intel: Only smash VGA SR01 register if intel is default VGA device

[Keith Packard]

Chris Wilson ch...@chris-wilson.co.uk writes:

 Ok, so as no vgaarb_clients have yet been registered and so the call to
 grab the IO resource does not actually disable VGA IO routing to the
 nvidia card.

Yikes! This explains a lot.

 If you care to update the changelog to explain the problem is that
 vgaarb is ineffective before all clients are registered, then I think
 this is a good temporary hack. It should be possible for vgaarb to mark
 resources as locked if the device is interpretting IO access and has no
 method for disabling the IO grab (and then a vga_tryget() check here).

Sounds like vgaarb should not assume that all devices using VGA are
registered drivers. That also sounds like a significantly harder fix.

If SR01 is required before disabling VGA, then perhaps we should not
disable VGA at all in this case though. That sounds safer, although if
VGA was enabled, it will suck a bit more power?

-- 
keith.pack...@intel.com


pgpvHdV8z2b5h.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/4] xf86-video-intel DRI3 and Present patch series

[Keith Packard]

Chris Wilson ch...@chris-wilson.co.uk writes:

  [PATCH 1/4] Support 64-bit MSC values. Handle kernel vageries about

 Some spurious assignments that appear to intentially drop the error code
 could be clarified,

I can't find any dropped error codes in this patch to add comments to,
please provide patch excerpts for this review.

 and intel_crtc_msc_to_sequence() is always called
 with a derived current_msc already to hand. The latter present path
 obfuscates its derived current_msc.

Present always computes absolute MSC values and provides those to the
driver, instead of expecting every driver to duplicate that logic.

  [PATCH 2/4] Restructure DRM event handling.

 This won't compile against older Xorg due to xorg_list in the common
 code.

Can switch to intel_list, but that would need list_for_each_entry_safe
added. How many versions back is this supposed to compile against?

  [PATCH 3/4] Add DRI3 and miSyncShm support

 O_CLOEXEC needs protecting, also would appear to be candidate for a
 render-node.

Yes, obviously this wants to use render-node. I haven't had complaints
about O_CLOEXEC from BSD or Solaris developers for libxshmfence; what
systems do not have support for this?

 The imported and exported DRI3 pixmaps need to be pinned
 to prevent the driver using BO exchanges on that pixmap.

I don't understand this comment.

 DRI3 doesn't respect the xorg.conf Option for disabling.

Ok, it should check intel-directRenderingType == DRI_DISABLED.

 A fence is only tied to a
 screen and no XID or Client in particular?

DRI3 fences are screen-specific (otherwise you'd have no way of hooking
the fence to a specific driver).

 So it is a global operation
 akin to intel_flush_callback() which would be called before the Sync
 reply was sent.

Yes, the hardware queue is to be flushed before the Sync event is sent
(and before the xshmfence object is triggered, of course). Note that
this is just the mi version of sync fences, which use libxshmfence; the
driver is free to use different code there. If we find that the code for
handling these xshmfence objects is common across drivers, we can move
that into the X server to share.

  [PATCH 4/4] Add Present extension support

 Yikes. The patch is itself fairly innoculous, but only because the Present
 extension in the server appears to be repeating the worst of DRI2,
 including its original bugs.

Please provide more specific comments here.

 The fallback/non-fullscreen case is not
 synchronised to screen refresh (if the Client so desired), and should
 be passed through to the driver.

The fallback case is synchronized as the Present code triggers the
CopyArea call from the vblank hook. In practice, this has proven
sufficient to get images onto the screen without tearing and without
requiring a huge amount of driver and kernel infrastructure.

 The whole driver interface seems to be too low a level, baking in many
 assumptions, rather the usual approach of providing a set of mi
 routines that the driver can plug into or not as the case may be.

Patches to the X server to change the API for better hardware support
are welcome, of course.

 That the WindowPixmap no longer points to the actual bo leads
 to a few problems, such as the CRTC misconfiguration and GetImage being
 broken after a PresentFlip.

A patch for the X server to fix that has been posted.

 After a vblank_event, Present must check that
 the flip is still valid before execution.

The flip proc may return FALSE to indicate failure of any kind. Present
will then fall-back to a simple blt.

 In the backend it is not clear whether the RRCrtc should be the
 primary CRTC or the only CRTC to flip.

There is only one screen pixmap, so of course every CRTC must flip
together. The CRTC provided indicates which one the MSC is from.

 Damage is processed after the fallback but not the Flip path, the lack
 of Damage notification would upset Prime amongst others.

Sounds easy to fix in the X server.

Thanks for your review!

-- 
keith.pack...@intel.com


pgpVceKq_Q16e.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Keith Packard]

Kristian Høgsberg hoegsb...@gmail.com writes:

 I already explained to Keith why we use different sets of format codes
 in the DRI interface, but it's always fun to slam other peoples code.

As we discussed, my complaint isn't so much about __DRI_IMAGE_FOURCC,
but the fact that the __DRIimage interfaces use *both*
__DRI_IMAGE_FOURCC and __DRI_IMAGE_FORMAT at different times.

Ok, here's a fine thing we can actually fix -- the pattern that mesa
uses all over the place in converting formats looks like this (not to
pick on anyone, it's repeated everywhere, this is just the first one I
found in gbm_dri.c):

static uint32_t
gbm_dri_to_gbm_format(uint32_t dri_format)
{
   uint32_t ret = 0;

   switch (dri_format) {
   case __DRI_IMAGE_FORMAT_RGB565:
  ret = GBM_FORMAT_RGB565;
  break;
   case __DRI_IMAGE_FORMAT_XRGB:
  ret = GBM_FORMAT_XRGB;
  break;
   case __DRI_IMAGE_FORMAT_ARGB:
  ret = GBM_FORMAT_ARGB;
  break;
   case __DRI_IMAGE_FORMAT_ABGR:
  ret = GBM_FORMAT_ABGR;
  break;
   default:
  ret = 0;
  break;
   }

   return ret;
}

The problem here is that any unknown incoming formats get translated to
garbage (0) outgoing. With fourcc codes, there is the slight advantage
that 0 is never a legal value, but it sure would be nice to print a
warning or even abort if you get a format code you don't understand as
there's no way 0 is ever going to do what you want.

Anyone have a preference? Abort? Print an error? Silently continue to do
the wrong thing?

-- 
keith.pack...@intel.com


pgpwoFH8RsOlL.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Keith Packard]

Ville Syrjälä ville.syrj...@linux.intel.com writes:

 What is this format anyway? -ENODOCS

Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)

 If its just an srgb version of ARGB, then I wouldn't really want it
 in drm_fourcc.h. I expect colorspacy stuff will be handled by various
 crtc/plane properties in the kernel so we don't need to encode that
 stuff into the fb format.

It's not any different from splitting YUV codes from RGB codes; a
different color encoding with the same bitfields. And, for mesa, it's
necessary to differentiate between ARGB and SARGB within the same format
code given how the API is structured. So, we have choices:

 1) Let Mesa define it's own fourcc codes and risk future accidental
collisions
 
 2) Rewrite piles of mesa code to split out the color encoding from the
bitfield information and pass it separately.

 3) Add reasonable format codes like this to the kernel fourcc list.

-- 
keith.pack...@intel.com


pgpu6mWttBLty.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 0/4] xf86-video-intel DRI3 and Present patch series

[Keith Packard]

Here's a series of patches which provide DRI3 and Present support in
the Intel 2D driver. The first two patches pave the way by
synthesizing 64-bit vblank counters and extending the DRM event
handling to allow for both DRI2 and DRI3 events. Then there's a patch
to add DRI2 and miSyncShm support followed by a patch to add Present
support.

 [PATCH 1/4] Support 64-bit MSC values. Handle kernel vageries about
 [PATCH 2/4] Restructure DRM event handling.
 [PATCH 3/4] Add DRI3 and miSyncShm support
 [PATCH 4/4] Add Present extension support

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

[Intel-gfx] [PATCH 4/4] Add Present extension support

[Keith Packard]

Signed-off-by: Keith Packard kei...@keithp.com
---
 configure.ac|  15 ++
 src/uxa/Makefile.am |   6 +
 src/uxa/intel.h |  15 ++
 src/uxa/intel_driver.c  |   4 +
 src/uxa/intel_present.c | 406 
 5 files changed, 446 insertions(+)
 create mode 100644 src/uxa/intel_present.c

diff --git a/configure.ac b/configure.ac
index 13b9970..8d881a8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -277,6 +277,7 @@ XORG_DRIVER_CHECK_EXT(RENDER, renderproto)
 XORG_DRIVER_CHECK_EXT(XF86DRI, xextproto x11)
 XORG_DRIVER_CHECK_EXT(DPMSExtension, xextproto)
 XORG_DRIVER_CHECK_EXT(DRI3, dri3proto)
+XORG_DRIVER_CHECK_EXT(PRESENT, presentproto)
 
 # Obtain compiler/linker options for the driver dependencies
 PKG_CHECK_MODULES(DRM, [libdrm = 2.4.20]) # libdrm_intel is checked separately
@@ -477,6 +478,19 @@ AC_MSG_CHECKING([whether to include DRI3 support])
 AM_CONDITIONAL(DRI3, test x$DRI3 = xyes)
 AC_MSG_RESULT([$DRI3])
 
+if test x$PRESENT != xno; then
+   save_CFLAGS=$CFLAGS
+   CFLAGS=$XORG_CFLAGS $PRESENT_CFLAGS
+   AC_CHECK_DECL(PRESENT,
+ [PRESENT=yes], [PRESENT=no],
+ [#include xorg-server.h])
+   CFLAGS=$save_CFLAGS
+fi
+echo 'PRESENT is now ' $PRESENT
+AC_MSG_CHECKING([whether to include Present support])
+AM_CONDITIONAL(PRESENT, test x$PRESENT = xyes)
+AC_MSG_RESULT([$PRESENT])
+
 AC_CHECK_HEADERS([X11/extensions/dpmsconst.h])
 
 AC_MSG_CHECKING([whether to include UXA support])
@@ -731,6 +745,7 @@ echo   Additional debugging support?$debug_msg
 echo   Support for Kernel Mode Setting? $KMS
 echo   Support for legacy User Mode Setting (for i810)? $UMS
 echo   Support for Direct Rendering Infrastructure:$dri_msg
+echo   Support for Present extension? $PRESENT
 echo   Support for Xv motion compensation (XvMC and libXvMC):$xvmc_msg
 echo   Build additional tools and utilities?$tools_msg
 if test -n $xp_msg; then
diff --git a/src/uxa/Makefile.am b/src/uxa/Makefile.am
index 3c9e693..1f6f942 100644
--- a/src/uxa/Makefile.am
+++ b/src/uxa/Makefile.am
@@ -87,6 +87,12 @@ libuxa_la_SOURCES += \
$(NULL)
 endif
 
+if PRESENT
+libuxa_la_SOURCES += \
+   intel_present.c \
+   $(NULL)
+endif
+
 if XVMC
 AM_CFLAGS += -I$(top_srcdir)/xvmc
 libuxa_la_SOURCES += \
diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index c3d00f4..48711e4 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -742,4 +742,19 @@ intel_sync_init(ScreenPtr screen);
 void
 intel_sync_close(ScreenPtr screen);
 
+/*
+ * intel_present.c
+ */
+
+#if 0
+#define DebugPresent(x) ErrorF x
+#else
+#define DebugPresent(x)
+#endif
+
+#if PRESENT
+Bool
+intel_present_screen_init(ScreenPtr screen);
+#endif
+
 #endif /* _I830_H_ */
diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
index 12c7b34..c760ff6 100644
--- a/src/uxa/intel_driver.c
+++ b/src/uxa/intel_driver.c
@@ -1050,6 +1050,10 @@ I830ScreenInit(SCREEN_INIT_ARGS_DECL)
if (intel-XvEnabled)
I830InitVideo(screen);
 
+#if PRESENT
+intel_present_screen_init(screen);
+#endif
+
 #if DRI3
 intel_dri3_screen_init(screen);
 #endif
diff --git a/src/uxa/intel_present.c b/src/uxa/intel_present.c
new file mode 100644
index 000..297497b
--- /dev/null
+++ b/src/uxa/intel_present.c
@@ -0,0 +1,406 @@
+/*
+ * Copyright © 2013 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided as
+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include stdio.h
+#include string.h
+#include assert.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include unistd.h
+#include fcntl.h
+#include sys/time.h
+#include time.h
+#include errno.h
+
+#include xf86.h
+#include xf86_OSproc.h
+
+#include xf86Pci.h
+#include xf86drm.h
+
+#include windowstr.h
+#include shadow.h
+#include fb.h
+
+#include intel.h
+#include i830_reg.h
+
+#include

[Intel-gfx] [PATCH 2/4] Restructure DRM event handling.

[Keith Packard]

This refactors the drm interrupt handling logic quite a bit, both to
allow for either DRI2 or Present handlers, but also to eliminate
passing pointers through the kernel. Passing pointers left the kernel
holding the only reference to some internal X server data structures.

After a server reset, the X server would end up using stale pointers
stored in those structures. Using simple integers makes it possible to
empty the queue of pending interrupt data and then ignore the stale
kernel data.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel.h |  31 -
 src/uxa/intel_display.c | 316 ++--
 src/uxa/intel_dri.c |  99 ---
 3 files changed, 390 insertions(+), 56 deletions(-)

diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index f05b160..922b208 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -395,6 +395,25 @@ extern void 
intel_mode_disable_unused_functions(ScrnInfoPtr scrn);
 extern void intel_mode_remove_fb(intel_screen_private *intel);
 extern void intel_mode_close(intel_screen_private *intel);
 extern void intel_mode_fini(intel_screen_private *intel);
+extern int intel_mode_read_drm_events(intel_screen_private *intel);
+
+typedef void (*intel_drm_handler_proc)(ScrnInfoPtr scrn,
+   xf86CrtcPtr crtc,
+   uint64_t seq,
+   uint64_t usec,
+   void *data);
+
+typedef void (*intel_drm_abort_proc)(ScrnInfoPtr scrn,
+ xf86CrtcPtr crtc,
+ void *data);
+
+extern uint32_t intel_drm_queue_alloc(ScrnInfoPtr scrn, xf86CrtcPtr crtc, void 
*data, intel_drm_handler_proc handler, intel_drm_abort_proc abort);
+extern void intel_drm_abort(ScrnInfoPtr scrn, Bool (*match)(void *data, void 
*match_data), void *match_data);
+
+/* struct intel_mode *
+   intel_page_flip_handler(void *event_data); */
+
+
 
 extern int intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, xf86CrtcPtr 
crtc);
 extern int intel_crtc_id(xf86CrtcPtr crtc);
@@ -422,6 +441,12 @@ typedef void (*DRI2SwapEventPtr)(ClientPtr client, void 
*data, int type,
 CARD64 ust, CARD64 msc, CARD64 sbc);
 #endif
 
+typedef void (*intel_pageflip_handler_proc) (uint64_t frame,
+ uint64_t usec,
+ void *data);
+
+typedef void (*intel_pageflip_abort_proc) (void *data);
+
 typedef struct _DRI2FrameEvent {
struct intel_screen_private *intel;
 
@@ -444,7 +469,11 @@ typedef struct _DRI2FrameEvent {
 
 extern Bool intel_do_pageflip(intel_screen_private *intel,
  dri_bo *new_front,
- DRI2FrameEventPtr flip_info, int ref_crtc_hw_id);
+  int ref_crtc_hw_id,
+  Bool async,
+  void *pageflip_data,
+  intel_pageflip_handler_proc pageflip_handler,
+  intel_pageflip_abort_proc pageflip_abort);
 
 static inline intel_screen_private *
 intel_get_screen_private(ScrnInfoPtr scrn)
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 09cd48f..e6cc07a 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -61,6 +61,23 @@
 
 #define KNOWN_MODE_FLAGS ((114)-1)
 
+struct intel_drm_queue {
+struct xorg_listlist;
+xf86CrtcPtr crtc;
+uint32_tseq;
+void*data;
+ScrnInfoPtr scrn;
+intel_drm_handler_proc  handler;
+intel_drm_abort_procabort;
+};
+
+static void
+intel_drm_abort_scrn(ScrnInfoPtr scrn);
+
+static uint32_t intel_drm_seq;
+
+static struct xorg_list intel_drm_queue;
+
 struct intel_mode {
int fd;
uint32_t fb_id;
@@ -68,7 +85,6 @@ struct intel_mode {
int cpp;
 
drmEventContext event_context;
-   DRI2FrameEventPtr flip_info;
int old_fb_id;
int flip_count;
uint64_t fe_msc;
@@ -76,6 +92,10 @@ struct intel_mode {
 
struct list outputs;
struct list crtcs;
+
+void *pageflip_data;
+intel_pageflip_handler_proc pageflip_handler;
+intel_pageflip_abort_proc pageflip_abort;
 };
 
 struct intel_pageflip {
@@ -536,6 +556,7 @@ intel_crtc_apply(xf86CrtcPtr crtc)
 
if (scrn-pScreen)
xf86_reload_cursors(scrn-pScreen);
+intel_drm_abort_scrn(scrn);
 
 done:
free(output_ids);
@@ -1138,11 +1159,23 @@ intel_output_dpms(xf86OutputPtr output, int dpms)
dpms);
intel_output-dpms_mode = dpms;
drmModeFreeProperty(props);
-   return;
+break

[Intel-gfx] [PATCH 3/4] Add DRI3 and miSyncShm support

[Keith Packard]

Signed-off-by: Keith Packard kei...@keithp.com
---
 configure.ac   |  14 
 src/uxa/Makefile.am|   7 ++
 src/uxa/intel.h|  17 +
 src/uxa/intel_dri3.c   | 184 +
 src/uxa/intel_driver.c |  13 
 src/uxa/intel_sync.c   | 109 +
 src/uxa/intel_uxa.c|   1 +
 7 files changed, 345 insertions(+)
 create mode 100644 src/uxa/intel_dri3.c
 create mode 100644 src/uxa/intel_sync.c

diff --git a/configure.ac b/configure.ac
index 0783d61..13b9970 100644
--- a/configure.ac
+++ b/configure.ac
@@ -276,6 +276,7 @@ XORG_DRIVER_CHECK_EXT(RANDR, randrproto)
 XORG_DRIVER_CHECK_EXT(RENDER, renderproto)
 XORG_DRIVER_CHECK_EXT(XF86DRI, xextproto x11)
 XORG_DRIVER_CHECK_EXT(DPMSExtension, xextproto)
+XORG_DRIVER_CHECK_EXT(DRI3, dri3proto)
 
 # Obtain compiler/linker options for the driver dependencies
 PKG_CHECK_MODULES(DRM, [libdrm = 2.4.20]) # libdrm_intel is checked separately
@@ -463,6 +464,19 @@ else
UXA=no
 fi
 
+if test x$DRI3 != xno; then
+   save_CFLAGS=$CFLAGS
+   CFLAGS=$XORG_CFLAGS $DRM_CFLAGS $DRI_CFLAGS $DRI3_CFLAGS
+   AC_CHECK_DECL(DRI3,
+ [DRI3=yes], [DRI3=no],
+ [#include xorg-server.h])
+   CFLAGS=$save_CFLAGS
+   dri_msg=$dri_msg DRI3
+fi
+AC_MSG_CHECKING([whether to include DRI3 support])
+AM_CONDITIONAL(DRI3, test x$DRI3 = xyes)
+AC_MSG_RESULT([$DRI3])
+
 AC_CHECK_HEADERS([X11/extensions/dpmsconst.h])
 
 AC_MSG_CHECKING([whether to include UXA support])
diff --git a/src/uxa/Makefile.am b/src/uxa/Makefile.am
index 971ac21..3c9e693 100644
--- a/src/uxa/Makefile.am
+++ b/src/uxa/Makefile.am
@@ -80,6 +80,13 @@ libuxa_la_LIBADD += \
$(NULL)
 endif
 
+if DRI3
+libuxa_la_SOURCES += \
+   intel_dri3.c \
+   intel_sync.c \
+   $(NULL)
+endif
+
 if XVMC
 AM_CFLAGS += -I$(top_srcdir)/xvmc
 libuxa_la_SOURCES += \
diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 922b208..c3d00f4 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -59,6 +59,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include xf86xv.h
 #include xf86Crtc.h
 #include xf86RandR12.h
+#include misync.h
 
 #include xorg-server.h
 #include pciaccess.h
@@ -352,6 +353,11 @@ typedef struct intel_screen_private {
InputHandlerProc uevent_handler;
 #endif
Bool has_prime_vmap_flush;
+
+SyncScreenFuncsRec save_sync_screen_funcs;
+
+void (*flush_rendering)(struct intel_screen_private *intel);
+
 } intel_screen_private;
 
 #define INTEL_INFO(intel) ((intel)-info)
@@ -519,6 +525,9 @@ void I830DRI2FrameEventHandler(unsigned int frame, unsigned 
int tv_sec,
 void I830DRI2FlipEventHandler(unsigned int frame, unsigned int tv_sec,
  unsigned int tv_usec, DRI2FrameEventPtr 
flip_info);
 
+/* intel_dri3.c */
+Bool intel_dri3_screen_init(ScreenPtr screen);
+
 extern Bool intel_crtc_on(xf86CrtcPtr crtc);
 int intel_crtc_to_pipe(xf86CrtcPtr crtc);
 
@@ -725,4 +734,12 @@ static inline Bool intel_pixmap_is_offscreen(PixmapPtr 
pixmap)
return priv  priv-offscreen;
 }
 
+#if DRI3
+Bool
+intel_sync_init(ScreenPtr screen);
+#endif
+
+void
+intel_sync_close(ScreenPtr screen);
+
 #endif /* _I830_H_ */
diff --git a/src/uxa/intel_dri3.c b/src/uxa/intel_dri3.c
new file mode 100644
index 000..99ac9d5
--- /dev/null
+++ b/src/uxa/intel_dri3.c
@@ -0,0 +1,184 @@
+/*
+ * Copyright © 2013 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided as
+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include stdio.h
+#include string.h
+#include assert.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include unistd.h
+#include fcntl.h
+#include sys/time.h
+#include time.h
+#include errno.h
+
+#include xf86.h
+#include xf86_OSproc.h
+
+#include xf86Pci.h
+#include xf86drm.h
+
+#include

[Intel-gfx] [PATCH 1/4] Support 64-bit MSC values. Handle kernel vageries about MSC reporting

[Keith Packard]

The kernel sometimes reports bogus MSC values, especially when
suspending and resuming the machine. Deal with this by tracking an
offset to ensure that the MSC seen by applications increases
monotonically, and at a reasonable pace.

Also, provide a full 64 bits of MSC value by noticing wrapping and
tracking the high 32-bits of MSC separately.

Signed-off-by: Keith Packard kei...@keithp.com
---
 src/uxa/intel.h |   9 
 src/uxa/intel_display.c | 118 -
 src/uxa/intel_dri.c | 125 
 3 files changed, 156 insertions(+), 96 deletions(-)

diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 131f18c..f05b160 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -401,6 +401,15 @@ extern int intel_crtc_id(xf86CrtcPtr crtc);
 extern int intel_output_dpms_status(xf86OutputPtr output);
 extern void intel_copy_fb(ScrnInfoPtr scrn);
 
+int
+intel_get_crtc_msc_ust(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint64_t *msc, 
uint64_t *ust);
+
+uint32_t
+intel_crtc_msc_to_sequence(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint64_t 
expect);
+
+uint64_t
+intel_sequence_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence);
+
 enum DRI2FrameEventType {
DRI2_SWAP,
DRI2_SWAP_CHAIN,
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index 3c2f964..09cd48f 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -71,9 +71,8 @@ struct intel_mode {
DRI2FrameEventPtr flip_info;
int old_fb_id;
int flip_count;
-   unsigned int fe_frame;
-   unsigned int fe_tv_sec;
-   unsigned int fe_tv_usec;
+   uint64_t fe_msc;
+uint64_t fe_usec;
 
struct list outputs;
struct list crtcs;
@@ -97,6 +96,9 @@ struct intel_crtc {
struct list link;
PixmapPtr scanout_pixmap;
uint32_t scanout_fb_id;
+int32_t vblank_offset;
+uint32_t msc_prev;
+uint64_t msc_high;
 };
 
 struct intel_property {
@@ -1647,9 +1649,8 @@ intel_do_pageflip(intel_screen_private *intel,
 * Also, flips queued on disabled or incorrectly configured displays
 * may never complete; this is a configuration error.
 */
-   mode-fe_frame = 0;
-   mode-fe_tv_sec = 0;
-   mode-fe_tv_usec = 0;
+   mode-fe_msc = 0;
+   mode-fe_usec = 0;
 
for (i = 0; i  config-num_crtc; i++) {
if (!intel_crtc_on(config-crtc[i]))
@@ -1705,6 +1706,102 @@ static const xf86CrtcConfigFuncsRec 
intel_xf86crtc_config_funcs = {
intel_xf86crtc_resize
 };
 
+static uint32_t pipe_select(int pipe)
+{
+   if (pipe  1)
+   return pipe  DRM_VBLANK_HIGH_CRTC_SHIFT;
+   else if (pipe  0)
+   return DRM_VBLANK_SECONDARY;
+   else
+   return 0;
+}
+
+/*
+ * Get the current msc/ust value from the kernel
+ */
+static int
+intel_get_msc_ust(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint32_t *msc, uint64_t 
*ust)
+{
+   intel_screen_private*intel = intel_get_screen_private(scrn);
+drmVBlank   vbl;
+int ret;
+
+/* Get current count */
+vbl.request.type = DRM_VBLANK_RELATIVE | 
pipe_select(intel_crtc_to_pipe(crtc));
+vbl.request.sequence = 0;
+vbl.request.signal = 0;
+ret = drmWaitVBlank(intel-drmSubFD, vbl);
+if (ret) {
+*msc = 0;
+*ust = 0;
+return BadMatch;
+} else {
+*msc = vbl.reply.sequence;
+*ust = (CARD64) vbl.reply.tval_sec * 100 + 
vbl.reply.tval_usec;
+}
+return Success;
+}
+
+/*
+ * Convert a 32-bit kernel MSC sequence number to a 64-bit local sequence
+ * number, adding in the vblank_offset and high 32 bits, and dealing
+ * with 64-bit wrapping
+ */
+uint64_t
+intel_sequence_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence)
+{
+   struct intel_crtc   *intel_crtc = crtc-driver_private;
+sequence += intel_crtc-vblank_offset;
+
+if ((int32_t) (sequence - intel_crtc-msc_prev)  -0x4000)
+intel_crtc-msc_high += 0x1L;
+intel_crtc-msc_prev = sequence;
+return intel_crtc-msc_high + sequence;
+}
+
+/*
+ * Get the current 64-bit adjust MSC and UST value
+ */
+int
+intel_get_crtc_msc_ust(ScrnInfoPtr scrn, xf86CrtcPtr crtc, uint64_t *msc, 
uint64_t *ust)
+{
+uint32_tsequence;
+int ret;
+
+ret = intel_get_msc_ust(scrn, crtc, sequence, ust);
+*msc = intel_sequence_to_crtc_msc(crtc, sequence);
+return ret;
+}
+
+/*
+ * Convert a 64-bit adjusted MSC value into a 32-bit kernel sequence number,
+ * removing the high 32 bits and subtracting out the vblank_offset term.
+ *
+ * This also updates the vblank_offset when it notices that the value should
+ * change.
+ */
+uint32_t
+intel_crtc_msc_to_sequence(ScrnInfoPtr scrn, xf86CrtcPtr crtc

[Intel-gfx] vblank_count count jumps backwards and then forwards across system suspend

[Keith Packard]

I'm getting some weird results when using vblank_count on my Ivybridge
machine across suspend/resume.

With glxgears running, I suspend the machine. At resume, I see
vblank_count temporarily jump back by a fairly large amount (usually
between 1 and 2 frames). After a short while, it recovers and
jumps back to something that looks like it's back on track with values
From before the suspend.

I thought at first that the value was just getting set back and left
there, but after working around that kind of bug, I discovered that it
only stayed on the wrong sequence for a short while (looks like about
20ms) and then jumps back to the previous sequence. I hacked up the
vblank code to show where the errors occurred, and here's what I got
(the logged MSC values are bogus, but the sequence of events is
correct):

Vblank goes haywire here:

[ 30348.438] expect msc near 4294871473 got msc 4294860521
[ 30348.441] (II) intel(0): EDID vendor APP, prod id 40178
[ 30348.442] (II) intel(0): Printing DDC gathered Modelines:
[ 30348.442] (II) intel(0): Modeline 1366x768x0.0   72.00  1366 1380 1436 
1500  768 769 772 800 -hsync -vsync (48.0 kHz eP)

Vblank recovers here:

[ 30348.458] expect msc near 4294849570 got msc 4294860522

-- 
keith.pack...@intel.com


pgpaziv_IGOyF.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] compilation broken again

[Keith Packard]

Knut Petersen knut_peter...@t-online.de writes:

 /home/knut/fast/xorg/X11-h/usr/include/xorg/shmint.h:59:31: fatal
 error: protocol-versions.h: No such file or directory

Julien noticed this as well; I've posted a patch to the server to fix
it (by not including protocol-versions.h from shmint.h)

-- 
keith.pack...@intel.com


pgp2yEbJSaCqv.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915: Add async page flip support for IVB

[Keith Packard]

 Generally I think checking our current sw state instead of reading hw
 registers would be safer, e.g. in case we start to queue up more than
 one pageflip. For async pageflips in benchmark mode flip as fast as
 you can queue would be a sensible mode.

Ok, I've moved the tiling checks to the general code and removed the
stride checks as those are already present there. These were moved to
the general code because the pointer to the previous FB has already
been overwritten by the time the queue_flip functions are called.

This should be followed by replacements for the last to patches in the
series.

-keith

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

[Intel-gfx] [PATCH 1/2] drm/i915: Add async page flip support for IVB

[Keith Packard]

This adds the necesary register defines for async page flipping
through the command ring, and then hooks those up for Ivybridge (gen7)
page flipping.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_reg.h  |  6 ++
 drivers/gpu/drm/i915/intel_display.c | 37 
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc3d6a7..029cfb0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -209,6 +209,7 @@
 #define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
 #define MI_DISPLAY_FLIPMI_INSTR(0x14, 2)
 #define MI_DISPLAY_FLIP_I915   MI_INSTR(0x14, 1)
+#define   MI_DISPLAY_FLIP_ASYNC_INDICATOR  (1  22)
 #define   MI_DISPLAY_FLIP_PLANE(n) ((n)  20)
 /* IVB has funny definitions for which plane to flip. */
 #define   MI_DISPLAY_FLIP_IVB_PLANE_A  (0  19)
@@ -217,6 +218,11 @@
 #define   MI_DISPLAY_FLIP_IVB_SPRITE_B (3  19)
 #define   MI_DISPLAY_FLIP_IVB_PLANE_C  (4  19)
 #define   MI_DISPLAY_FLIP_IVB_SPRITE_C (5  19)
+/* These go in the bottom of the base address value */
+#define   MI_DISPLAY_FLIP_TYPE_SYNC(0  0)
+#define   MI_DISPLAY_FLIP_TYPE_ASYNC   (1  0)
+#define   MI_DISPLAY_FLIP_TYPE_STEREO  (2  0)
+#define   MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS (0  0)
 #define MI_ARB_ON_OFF  MI_INSTR(0x08, 0)
 #define   MI_ARB_ENABLE(10)
 #define   MI_ARB_DISABLE   (00)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bdb8854..166aa2c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1833,8 +1833,10 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
alignment = 64 * 1024;
break;
case I915_TILING_X:
-   /* pin() will align the object as required by fence */
-   alignment = 0;
+   /* Async page flipping requires X tiling and 32kB alignment, so 
just
+* make all X tiled frame buffers aligned for that
+*/
+   alignment = 32 * 1024;
break;
case I915_TILING_Y:
/* Despite that we check this in framebuffer_init userspace can
@@ -7514,6 +7516,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_ring_buffer *ring = dev_priv-ring[BCS];
uint32_t plane_bit = 0;
+   uint32_t cmd;
+   uint32_t base;
int ret;
 
ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
@@ -7536,13 +7540,21 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
goto err_unpin;
}
 
+   cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
+   base = i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
+
+   if (flags  DRM_MODE_PAGE_FLIP_ASYNC) {
+   cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
+   base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
+   }
+
ret = intel_ring_begin(ring, 4);
if (ret)
goto err_unpin;
 
-   intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
+   intel_ring_emit(ring, cmd);
intel_ring_emit(ring, (fb-pitches[0] | obj-tiling_mode));
-   intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + 
intel_crtc-dspaddr_offset);
+   intel_ring_emit(ring, base);
intel_ring_emit(ring, (MI_NOOP));
 
intel_mark_page_flip_active(intel_crtc);
@@ -7591,6 +7603,19 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 fb-pitches[0] != crtc-fb-pitches[0]))
return -EINVAL;
 
+   /* Check tiling restrictions specific to asynchronous flips
+*/
+   if (page_flip_flags  DRM_MODE_PAGE_FLIP_ASYNC) {
+
+   /* New FB must be X tiled */
+   if (obj-tiling_mode != I915_TILING_X)
+   return -EINVAL;
+
+   /* Current FB must be X tiled */
+   if (to_intel_framebuffer(old_fb)-obj-tiling_mode != 
I915_TILING_X)
+   return -EINVAL;
+   }
+
work = kzalloc(sizeof *work, GFP_KERNEL);
if (work == NULL)
return -ENOMEM;
@@ -9705,6 +9730,10 @@ void intel_modeset_init(struct drm_device *dev)
dev-mode_config.max_width = 8192;
dev-mode_config.max_height = 8192;
}
+
+   if (IS_GEN7(dev))
+   dev-mode_config.async_page_flip = true;
+
dev-mode_config.fb_base = dev_priv-gtt.mappable_base;
 
DRM_DEBUG_KMS(%d display pipe%s available.\n,
-- 
1.8.3.2

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

[Intel-gfx] [PATCH 2/2] drm/i915: Add async page flip support for SNB

[Keith Packard]

Just copies the IVB code

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 166aa2c..4a118c3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7465,20 +7465,29 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_ring_buffer *ring = dev_priv-ring[RCS];
uint32_t pf, pipesrc;
+   uint32_t cmd;
+   uint32_t base;
int ret;
 
ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
if (ret)
goto err;
 
+   cmd = MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc-plane);
+   base = i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
+
+   if (flags  DRM_MODE_PAGE_FLIP_ASYNC) {
+   cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
+   base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
+   }
+
ret = intel_ring_begin(ring, 4);
if (ret)
goto err_unpin;
 
-   intel_ring_emit(ring, MI_DISPLAY_FLIP |
-   MI_DISPLAY_FLIP_PLANE(intel_crtc-plane));
+   intel_ring_emit(ring, cmd);
intel_ring_emit(ring, fb-pitches[0] | obj-tiling_mode);
-   intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + 
intel_crtc-dspaddr_offset);
+   intel_ring_emit(ring, base);
 
/* Contrary to the suggestions in the documentation,
 * Enable Panel Fitter does not seem to be required when page
@@ -9731,7 +9740,7 @@ void intel_modeset_init(struct drm_device *dev)
dev-mode_config.max_height = 8192;
}
 
-   if (IS_GEN7(dev))
+   if (IS_GEN6(dev) || IS_GEN7(dev))
dev-mode_config.async_page_flip = true;
 
dev-mode_config.fb_base = dev_priv-gtt.mappable_base;
-- 
1.8.3.2

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

Re: [Intel-gfx] [PATCH 1/4] drm/i915: don't block resume on fb console resume v2

[Keith Packard]

Jesse Barnes jbar...@virtuousgeek.org writes:

 v2: use console_trylock() to try to resume the console immediately
 (Chris)

This will cause other printks to stall if i915 grabs the lock
first. Seems like a way to get random resume delays to me.

-- 
keith.pack...@intel.com


pgpas36b6iooM.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: optionally check GTT checksum across suspend/resume

[Keith Packard]

Ben Widawsky b...@bwidawsk.net writes:

 Also, as a bikeshed you could probably get a much better detection with
 a CRC or something similar.

Wonder if you could use the existing MD5 code in the kernel; would avoid
all sorts of unfortunate false-positives.

-- 
keith.pack...@intel.com


pgpOfytF7Loyb.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/7] drm/i915: FDI B/C share 4 lanes on Ivybridge

[Keith Packard]

Lespiau, Damien damien.lesp...@intel.com writes:

 On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard kei...@keithp.com wrote:

 @@ -3728,7 +3728,8 @@ static inline bool intel_panel_use_ssc(struct
 drm_i915_private *dev_priv)
   */
  static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
  unsigned int *pipe_bpp,
 -struct drm_display_mode *mode)
 +struct drm_display_mode *mode,
 +int max_fdi_bpp)

 There's some kernel-doc for this function, maybe add a @max_fdi_bpp
 there?

Will do

 This chunk is being moved around in a later patch in the series,
 merging the two patches in one looks like a good idea?

Or at least move this into its final position in this patch.

 I guess this does not cover the case of pipe B using 3 lanes meaning
 pipe C can use 1?

It didn't look like that was a supported mode from the docs.

 This duplicates the code just that is just a few lines away, instead
 how about moving the logic to set target_clock up in front of this
 whole if()?

It's not the same, it's the inverse -- computing bpp from lanes+clock
clock instead of computing lanes from bpp+clock. But, yeah, it would be
nice to have these merged somehow. I couldn't figure out a good way though.

 This chunk is also reworked in a later commit in this series.

I'll see if I can't avoid that as it's confusing. Thanks for your review!

-- 
keith.pack...@intel.com


pgpXbVAz0RQqc.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Disable FDI RX before FDI TX

[Keith Packard]

Lespiau, Damien damien.lesp...@intel.com writes:

 I can't see anything in the docs about an order requirement for those.

Right, the docs don't say anything, which is a bit disconcerting.

 Not sure why the other way does not make sense. Somehow disabling TX
 before RX makes some sense to me (TX enabled without a ready RX looks
 weird?, no data should flow as the pipe is shutdown at that point
 anyway). Maybe it just does not matter?

And here I figured disabling RX before TX made more sense -- otherwise
the receiver wouldn't be seeing anything. In other areas of the driver,
we're careful to disable receivers before senders (disable CRTC before
PLL, etc).

 Another detail is that disabling the PLLs seem to have an order in the
 disabling sequence, TX, then RX.

 I.  Disable CPU FDI Transmitter PLL
 II. Disable PCH FDI Receiver PLL

That ordering doesn't matter as the FDI receiver and transmitter are
both disabled by that point, so they aren't talking at all.

-- 
keith.pack...@intel.com


pgpP7gjpbWilF.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix hsw uncached pte

[Keith Packard]

Paulo Zanoni przan...@gmail.com writes:

 +#define HSW_PTE_UNCACHED 0x

Are you sure this value should be zero? It seems pretty unlikely to me.

-- 
keith.pack...@intel.com


pgptzbyqd415W.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/7] drm/i915: Check display_bpc against max_fdi_bpp after display_bpc is set

[Keith Packard]

display_bpc might not have been set before comparing with the
requested mode, so wait until afterwards before comparing with the
supported fdi bandwidth. Not a significant change as any case that
mattered would have worked; this just makes the debug messages look nicer.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   39 --
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 95248bd..b099a17 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3804,15 +3804,6 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc 
*crtc,
display_bpc = 6;
}
 
-   if (display_bpc * 3  max_fdi_bpp) {
-   if (max_fdi_bpp  24)
-   display_bpc = 6;
-   else if (max_fdi_bpp  30)
-   display_bpc = 8;
-   else if (max_fdi_bpp  36)
-   display_bpc = 10;
-   DRM_DEBUG_KMS(Dithering FDI to %dbpc\n, display_bpc);
-   }
/*
 * We could just drive the pipe at the highest bpc all the time and
 * enable dithering as needed, but that costs bandwidth.  So choose
@@ -3845,8 +3836,20 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc 
*crtc,
 
display_bpc = min(display_bpc, bpc);
 
-   DRM_DEBUG_KMS(setting pipe bpc to %d (max display bpc %d)\n,
- bpc, display_bpc);
+   display_bpc = 6;
+
+   if (display_bpc * 3  max_fdi_bpp) {
+   if (max_fdi_bpp  24)
+   display_bpc = 6;
+   else if (max_fdi_bpp  30)
+   display_bpc = 8;
+   else if (max_fdi_bpp  36)
+   display_bpc = 10;
+   DRM_DEBUG_KMS(Dithering FDI to %dbpc\n, display_bpc);
+   }
+
+   DRM_DEBUG_KMS(setting pipe bpc to %d (max display bpc %d) (max_fdi_bpp 
%d)\n,
+ bpc, display_bpc, max_fdi_bpp);
 
*pipe_bpp = display_bpc * 3;
 
@@ -4737,8 +4740,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
max_fdi_bpp = 0;
max_lane = lane;
} else {
-   u32 fdi_bw;
-
+   u32 fdi_bw, pps;
/* [e]DP over FDI requires target mode clock
   instead of link clock */
if (is_dp)
@@ -4763,9 +4765,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 * Compute the available FDI bandwidth, use that
 * to compute the maximum supported BPP
 */
-   fdi_bw = link_bw * max_lane * 19 / 20;
-   max_fdi_bpp = fdi_bw / target_clock;
-   DRM_DEBUG_KMS(max lane %d yields max fdi bpp %d\n, max_lane, 
max_fdi_bpp);
+   fdi_bw = (link_bw * 8) * max_lane;
+   pps = target_clock * 21 / 20;
+
+   max_fdi_bpp = fdi_bw / pps;
+   DRM_DEBUG_KMS(link_bw %d max_lane %d fdi_bw %u pps %u 
max_fdi_bpp %d\n,
+ link_bw, max_lane, fdi_bw, pps, max_fdi_bpp);
}
 
/* [e]DP over FDI requires target mode clock instead of link clock. */
@@ -4809,6 +4814,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 */
u32 bps = target_clock * pipe_bpp * 21 / 20;
lane = bps / (link_bw * 8) + 1;
+   DRM_DEBUG_KMS(target_clock %u pipe_bpp %u bps %u link_bw %u 
lane %u\n,
+ target_clock, pipe_bpp, bps, link_bw, lane);
if (lane  max_lane) {
DRM_ERROR(Not enough lanes available for mode! (want 
%d have %d)\n,
  lane, max_lane);
-- 
1.7.10.4

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

[Intel-gfx] [PATCH 0/7] drm/i915: IVB FDI B/C fixes and misc cleanups

[Keith Packard]

This is the complete set of patches that yield a working 3-pipe mode
setting configuration on my test machines. It does not make DPMS work,
so I still need to figure that out. As the DPMS paths are almost
entirely different from mode setting (whose crazy idea was that,
anyway?), that may take a bit more time.

I've managed to reproduce this bug with only two monitors just by
forcing them onto FDI B/C, but only with 1920x1080 and larger modes:

 $ xrandr --output VGA1 --off --output DP1 --off
 $ xrandr --output VGA1 --auto --crtc 1
 $ xrandr --output DP1 --auto --crtc 2

That works about 50% of the time on the original kernel. If it works,
you can just try a couple of different modes to see if you can get it
to fail. Just use reasonably large modes -- 640x480 has never failed
for me. I'd love to know why

 $ xrandr --output DP1 --mode 1600x1200 --crtc 2

Finally, the X server has a terrible bug in the RandR code. It
computes a desired initial configuration, and if that fails in any
way, it refuses to start at all. So, if you connect three monitors
that all require separate PLLs, then you get two monitors lit up, the
third one fails and the X server aborts. Need to fix that so that if
*any* monitors light up, the X server will keep going.
 
   *** The patches: ***

 [PATCH 1/7] drm/i915: Allow VGA on CRTC 2

Silly hold-over from the bad-old PLL sharing code.

 [PATCH 2/7] drm/i915: FDI B/C share 4 lanes on Ivybridge

Here's the patch which rejects modes that the FDI B/C lane sharing
hardware can't support. This only affects modes larger than 1920x1200
though as that mode and smaller all fit in two FDI lanes. I ran across
this while testing with a 2560x1200 monitor.

 [PATCH 3/7] drm/i915: Delay between FDI link training tries. Clear

This seems like a sensible change in the FDI link training code --
gives the FDI hardware time to adapt to changes in the
signalling. But, as I've never seen FDI fail to train, I'm not sure
it's useful.

 [PATCH 4/7] drm/i915: Check display_bpc against max_fdi_bpp after

This mostly just cleans up some debug messages by moving various BPP
computations around. should have no effect on the hardware.

 [PATCH 5/7] drm/i915: Pipe-C only configurations would not get SR

Just happened across this obvious bug while reading through the
driver.

 [PATCH 6/7] drm/i915: Disable FDI RX before FDI TX

The specs don't say which order to do this in, but it doesn't make
sense to do these in the current order. With this in place, I saw mode
setting errors reduced quite a bit, but not gone.

 [PATCH 7/7] drm/i915: Merge FDI RX reg writes during training

This may be the only patch necessary to get mode setting working on
FDI-B/C, it ensures that error correction is always turned on during
link training. The old code left error correction disabled as there
was no posting read after setting that. I'm hoping this explains why
the problem wasn't reliably reproducible -- the problem depended on
how long the write waited to get to the hardware. I haven't done
enough testing of this patch in isolation to know if this is true or not.

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

[Intel-gfx] [PATCH 5/7] drm/i915: Pipe-C only configurations would not get SR

[Keith Packard]

These should probably all look like

enabled |= (1  pipe)

so that the intent is clear...

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_pm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 94aabca..1a84425 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1815,7 +1815,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
DRM_DEBUG_KMS(FIFO watermarks For pipe C -
   plane %d, cursor: %d\n,
  plane_wm, cursor_wm);
-   enabled |= 3;
+   enabled |= 4;
}
 
/*
-- 
1.7.10.4

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

[Intel-gfx] [PATCH 3/7] drm/i915: Delay between FDI link training tries. Clear FDI_RX_IIR before training

[Keith Packard]

Just a bit of cleanup; it appears to have no effect.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7106807..95248bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2391,6 +2391,7 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
temp |= FDI_LINK_TRAIN_PATTERN_1;
I915_WRITE(reg, temp | FDI_TX_ENABLE);
 
+   I915_WRITE(FDI_RX_IIR(pipe), FDI_RX_BIT_LOCK);
reg = FDI_RX_CTL(pipe);
temp = I915_READ(reg);
temp = ~FDI_LINK_TRAIN_NONE;
@@ -2398,10 +2399,10 @@ static void ironlake_fdi_link_train(struct drm_crtc 
*crtc)
I915_WRITE(reg, temp | FDI_RX_ENABLE);
 
POSTING_READ(reg);
-   udelay(150);
 
/* Ironlake workaround, enable clock pointer after FDI enable*/
if (HAS_PCH_IBX(dev)) {
+   udelay(150);
I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR);
I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR |
   FDI_RX_PHASE_SYNC_POINTER_EN);
@@ -2409,6 +2410,7 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 
reg = FDI_RX_IIR(pipe);
for (tries = 0; tries  5; tries++) {
+   udelay(150);
temp = I915_READ(reg);
DRM_DEBUG_KMS(FDI_RX_IIR 0x%x\n, temp);
 
@@ -2422,6 +2424,7 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
DRM_ERROR(FDI train 1 fail!\n);
 
/* Train 2 */
+   I915_WRITE(FDI_RX_IIR(pipe), FDI_RX_SYMBOL_LOCK);
reg = FDI_TX_CTL(pipe);
temp = I915_READ(reg);
temp = ~FDI_LINK_TRAIN_NONE;
@@ -2435,10 +2438,10 @@ static void ironlake_fdi_link_train(struct drm_crtc 
*crtc)
I915_WRITE(reg, temp);
 
POSTING_READ(reg);
-   udelay(150);
 
reg = FDI_RX_IIR(pipe);
for (tries = 0; tries  5; tries++) {
+   udelay(150);
temp = I915_READ(reg);
DRM_DEBUG_KMS(FDI_RX_IIR 0x%x\n, temp);
 
-- 
1.7.10.4

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

[Intel-gfx] [PATCH 6/7] drm/i915: Disable FDI RX before FDI TX

[Keith Packard]

Doesn't make sense to disable in the other order.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b099a17..754f10f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2776,17 +2776,17 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
u32 reg, temp;
 
/* disable CPU FDI tx and PCH FDI rx */
-   reg = FDI_TX_CTL(pipe);
-   temp = I915_READ(reg);
-   I915_WRITE(reg, temp  ~FDI_TX_ENABLE);
-   POSTING_READ(reg);
-
reg = FDI_RX_CTL(pipe);
temp = I915_READ(reg);
temp = ~(0x7  16);
temp |= (I915_READ(PIPECONF(pipe))  PIPE_BPC_MASK)  11;
I915_WRITE(reg, temp  ~FDI_RX_ENABLE);
+   POSTING_READ(reg);
+   udelay(100);
 
+   reg = FDI_TX_CTL(pipe);
+   temp = I915_READ(reg);
+   I915_WRITE(reg, temp  ~FDI_TX_ENABLE);
POSTING_READ(reg);
udelay(100);
 
-- 
1.7.10.4

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

[Intel-gfx] [PATCH 7/7] drm/i915: Merge FDI RX reg writes during training

[Keith Packard]

Need to turn on the error correction when enabling training or it
might not get enabled in time.

This seems to fix the FDI-B/FDI-C link training problem.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 754f10f..1d24d55 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2324,6 +2324,8 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
temp |= FDI_LINK_TRAIN_NONE | FDI_TX_ENHANCE_FRAME_ENABLE;
}
I915_WRITE(reg, temp);
+   POSTING_READ(reg);
+   udelay(100);
 
reg = FDI_RX_CTL(pipe);
temp = I915_READ(reg);
@@ -2334,16 +2336,15 @@ static void intel_fdi_normal_train(struct drm_crtc 
*crtc)
temp = ~FDI_LINK_TRAIN_NONE;
temp |= FDI_LINK_TRAIN_NONE;
}
+   /* IVB wants error correction enabled */
+   if (IS_IVYBRIDGE(dev))
+   temp |= FDI_FS_ERRC_ENABLE | FDI_FE_ERRC_ENABLE;
+
I915_WRITE(reg, temp | FDI_RX_ENHANCE_FRAME_ENABLE);
 
/* wait one idle pattern time */
POSTING_READ(reg);
udelay(1000);
-
-   /* IVB wants error correction enabled */
-   if (IS_IVYBRIDGE(dev))
-   I915_WRITE(reg, I915_READ(reg) | FDI_FS_ERRC_ENABLE |
-  FDI_FE_ERRC_ENABLE);
 }
 
 static void cpt_phase_pointer_enable(struct drm_device *dev, int pipe)
-- 
1.7.10.4

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

Re: [Intel-gfx] Output problems with displayport-hdmi kable on arrandale based hp elitebook 2540p

[Keith Packard]

On Thu, 19 Jul 2012 00:16:59 +0200, Clemens Eisserer linuxhi...@gmail.com 
wrote:

 I really hope I won't need to use VGA forever - the blurry fonts make
 my eyes bleed ;)

Yeah, just be aware that DP to HDMI/DVI isn't really a 'dongle' in the
usual sense, the whole internal hardware is running in HDMI mode, just
sending the data over the DP wires. The cable then just converts the
signal levels.

So, our driver sees a regular HDMI output, which is vastly simpler than
the DP to VGA dongles which look like DP to the computer and VGA to the
monitor.

I've tried a bunch of monitors using this mode and it's always worked
just fine; you might try a different cable, or test with a different
monitor if you can find one.

-- 
keith.pack...@intel.com


pgpZLuYTlMcBT.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: prefer wide slow to fast narrow in DP configs

[Keith Packard]

Chris Wilson ch...@chris-wilson.co.uk writes:

 On Thu, 21 Jun 2012 18:13:19 -0700, Keith Packard kei...@keithp.com wrote:

 It was structured to minimise lane count because certain chipsets did
 not wire up all the lanes, right? Is that still relevant as we are using
 the advertised max_lane_count from the DPCD now?

We've always used the max_lane_count from dpcd; has there been some
recent change that fixed usage of that? What I recall is one acer laptop
that advertised 4 lanes but had only wired up two of them.

-- 
keith.pack...@intel.com


pgpmKQkYSOOOJ.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: prefer wide slow to fast narrow in DP configs

[Keith Packard]

Jesse Barnes jbar...@virtuousgeek.org writes:

 In embedded applications, some of the lanes may not exist, but the DPCD
 should indicate that (though as Keith says, some lie about it).  But if
 we set aside eDP it may be safe...

Yeah, that's my thinking. We should probably include eDP hooked up to
the PCH DP lanes (for all-in-one systems) too.

-- 
keith.pack...@intel.com


pgp2R7zpW2Ra3.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: enable vdd when switching off the eDP panel

[Keith Packard]

Daniel Vetter dan...@ffwll.ch writes:

 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -1154,11 +1154,10 @@ static void ironlake_edp_panel_off(struct intel_dp 
 *intel_dp)
  
  DRM_DEBUG_KMS(Turn eDP power off\n);
  
 -WARN(intel_dp-want_panel_vdd, Cannot turn power off while VDD is 
 on\n);
 -ironlake_panel_vdd_off_sync(intel_dp); /* finish any pending work */
 +WARN(!intel_dp-want_panel_vdd, Need VDD to turn off panel\n);
  
  pp = ironlake_get_pp_control(dev_priv);
 -pp = ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | 
 EDP_BLC_ENABLE);
 +pp = ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);

Can you explain where the panel is getting turned off here?

-- 
keith.pack...@intel.com


pgpys5TelbnPm.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 19/24] drm/i915: detect digital outputs on Haswell

[Keith Packard]

On Tue, 1 May 2012 08:01:08 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 I had wanted to avoid every HSW system looking like it had a bunch of
 HDMI and DP ports, when it really only has one of each or something.

Every DP port is also an HDMI port when a DP to HDMI converter is
plugged in.

-- 
keith.pack...@intel.com


pgpJ41qlOy7cv.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Google MapsGL causing X crashes on machines with Sandybridge graphics

[Keith Packard]

#part sign=pgpmime
On Thu, 12 Apr 2012 19:34:12 + (UTC), Eric Appleman erapple...@gmail.com 
wrote:

 It's a known issue, but I've yet to find a comprehensive explanation for 
 what's
 going on.

Appears to be caused by the Hi-Z code. No known solution at this
point, aside from disabling Hi-Z. Here's a script that I use to launch
google-earth with Hi-Z disabled (and enough core fonts to make the
broken Qt version included work):

#!/bin/sh
export INTEL_HIZ=0
MISCFONTS=/usr/share/fonts/X11/misc
if xset q | grep -q $MISCFONTS; then
echo 'fonts already present'
else
xset fp+ /usr/share/fonts/X11/misc
fi
/usr/bin/google-earth $@


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

Re: [Intel-gfx] [PATCH] drm/i915: properly compute dp dithering for user-created modes

[Keith Packard]

#part sign=pgpmime
On Tue, 10 Apr 2012 09:46:53 -0400, Adam Jackson a...@redhat.com wrote:

 Certainly an improvement.
 
 Reviewed-by: Adam Jackson a...@redhat.com

I'd like to know if this actually helps someone before I stick it in
drm-intel-fixes...

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

Re: [Intel-gfx] [PATCH] drm/i915/crt: Only support DPMS On/Off with the PCH registers

[Keith Packard]

#part sign=pgpmime
On Tue, 10 Apr 2012 09:20:38 -0400, Adam Jackson a...@redhat.com wrote:
 On 4/10/12 4:35 AM, Chris Wilson wrote:
  As part of the PCH split, the ability to control CRT standby/suspend
  states was defeatured; the bits are now marked reserved and apparently
  have no effect.
 
 Are the intermediate states even tested?  I have vague memories of them 
 not working.  I'd be just as happy to see them aliased to Off.

It's worse than that -- iirc, there were patent issues with the
intermediate states which caused them to be removed from some newer
display signaling standards...

As far as I know, only CRTs could usefully use them anyways.

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

Re: [Intel-gfx] Scanline wait hack for IVB

[Keith Packard]

#part sign=pgpmime
On Fri, 30 Mar 2012 14:45:16 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 So it looks like we can't use this on the BLT ring in SNB or IVB.  So
 we'll need a render ring BLT routine to use here instead (hello SNA).

Or a semaphore to block the render ring?

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

Re: [Intel-gfx] Broken LVDS output at mode changes

[Keith Packard]

#part sign=pgpmime
On Thu, 29 Mar 2012 13:44:28 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:

 In conjunction with bits Power Sequence Progress field and Power Cycle
 Delay Active, this bit set to a one indicates that the panel is
 currently powered up or is currently in the power down sequence and it
 is unsafe to change the timing, port, and DPLL registers for the pipe or
 transcoder that is assigned to the panel output.

The theory was that as we don't touch the DPLL and only modify the
scaler, that the panel wouldn't care. I wonder what's confusing this one...

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

Re: [Intel-gfx] Broken LVDS output at mode changes

[Keith Packard]

#part sign=pgpmime
On Thu, 29 Mar 2012 18:12:56 +0200, Takashi Iwai ti...@suse.de wrote:

 The strange thing is that, although you can recover the display by
 turning off LVDS and on again once when the problem happens, but then
 the display starts flickering.  And, the flickering continues even
 after reboot on BIOS boot screen.

Yeah, it's clearly the panel which is confused. It would be nice to know
what happened to the LVDS signal which caused this problem with the
panel.

 BTW, now I tested to disable LVDS unconditionally on different
 platforms as Daniel suggested.  All worked fine, and I saw no
 regressions.  Tested on IVY, SNB, ILK, GM45 and PineView.

That's not surprising -- pre-PCH platforms disabled the LVDS
anyways.

I wonder if we could fix this by being more careful about the order of
register operations during LVDS mode setting...

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

Re: [Intel-gfx] Broken LVDS output at mode changes

[Keith Packard]

#part sign=pgpmime
On Thu, 29 Mar 2012 20:02:01 +0200, Takashi Iwai ti...@suse.de wrote:

 Sure, that'd be a preferred option.
 If you have any easy test in mind, let me know.

The way I'd love to see it tested would be to capture signal traces on
the LVDS link across mode set and see what glitches appear. That sounds
hard to do without an LVDS signal analyser...

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

[Keith Packard]

#part sign=pgpmime
On Tue, 20 Mar 2012 13:04:41 +0100, Takashi Iwai ti...@suse.de wrote:

 Since checking the lid state is tricky and unreliable, the practical
 check would be simply reading the first LVDS reg and seeing whether it
 was initialized or not.  It seems that it reads to 0x02 when booted
 with the lid close, which is LVDS_DETECTED bit.

Right, lid-detect is not useful, so I suggested using the new code path
only if the LVDS was *not* actually running at startup time. That should
avoid almost all common cases that work correctly today.

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

[Keith Packard]

#part sign=pgpmime
On Sat, 17 Mar 2012 08:59:56 +0100, Takashi Iwai ti...@suse.de wrote:

 Well, the LVDS reg data isn't in lvds_dvo_timing but in lvds_fp_timing,
 thus you need to look at a different entry in anyway.

Right, a parallel function which returns the lvds_fp_timing structure
instead of pulling the data out of it. Just makes the code look more
like the existing stuff. And, if we need more data from the
lvds_fp_timing in the future, we'll have it available directly.

 I skipped it to simplify the code, but that'd be safer, indeed.

Reducing the chances for regressions is my primary concern here; most
people boot their machines with the lid open, so if we avoid the new
code in that case, we'll be more likely to not break things.

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Check VBIOS value for determining LVDS dual channel mode, too

[Keith Packard]

#part sign=pgpmime
On Fri, 16 Mar 2012 22:41:12 +0100, Takashi Iwai ti...@suse.de wrote:

 +/* read the initial LVDS register value for the given panel mode */
 +static unsigned int get_lvds_reg_val(const struct bdb_header *bdb,
 +  const struct bdb_lvds_lfp_data_ptrs *ptrs,
 +  int index,
 +  struct drm_display_mode *mode)

To follow the style of intel_bios.c, I think it would make sense to have
the function:

static const struct lvds_dvo_timing *
get_lvds_fp_timing(const struct bdb_lvds_lfp_data *lvds_lfp_data,
   const struct bdb_lvds_lfp_data_ptrs *lvds_lfp_data_ptrs,
   int index)

then use the results of this in parse_lfp_panel_data, instead of putting
the whole computation into this new function.

I'd also like to see this code only use the BDB value when the LVDS is
disabled at startup time; otherwise, we'll be changing the behavior for
all LVDS users, and as BIOS tables are notoriously unreliable, I fear
that we'll cause a lot more problems than we solve.

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

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

[Keith Packard]

Jesse sent me a couple of trivial sprite plane fixes.

The following changes since commit 91982b58d35720b75b894c60e1e3133daa455b53:

  drm/gma500: Fix Cedarview boot failures in 3.3-rc (2012-03-05 14:08:31 +)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/keithp/linux drm-intel-fixes

for you to fetch changes up to b250da79a0c972ef7f6d58ebd1083cab066e6c82:

  drm/i915: support 32 bit BGR formats in sprite planes (2012-03-07 10:52:13 
-0800)


Jesse Barnes (2):
  drm/i915: fix color order for BGR formats on SNB
  drm/i915: support 32 bit BGR formats in sprite planes

 drivers/gpu/drm/i915/i915_reg.h  |2 +-
 drivers/gpu/drm/i915/intel_display.c |1 +
 drivers/gpu/drm/i915/intel_sprite.c  |6 +++---
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
keith.pack...@intel.com


pgpcqsugaguPS.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix color order for BGR formats on SNB

[Keith Packard]

#part sign=pgpmime
On Wed, 7 Mar 2012 07:02:02 -0800, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 Dave, not sure if Keith has setup his -fixes tree again, but this one
 needs to go to Linus in your next pull assuming you're doing one.

Yeah, I got home just in time to travel for a whole week. I'll get
things sorted out and merge 1/2 into my -fixes tree. 2/2 seems sketchy
to me.

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix color order for BGR formats on SNB

[Keith Packard]

#part sign=pgpmime
On Mon, 27 Feb 2012 12:40:10 -0800, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 Had the wrong bits and field definitions.

Merged.
   617cf88..ab2f9df  drm-intel-fixes - drm-intel-fixes

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

Re: [Intel-gfx] [PATCH] drm/i915: support 32 bit BGR formats in sprite planes

[Keith Packard]

#part sign=pgpmime
On Wed,  7 Mar 2012 08:49:29 -0800, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 intel_framebuffer_init does some basic sanity checking of the pixel format,
 but is used by the plane code in addition to the primary crtc.  So it
 needs to contain any formats used in either place.

Merged.
   ab2f9df..b250da7  drm-intel-fixes - drm-intel-fixes

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix color order for BGR formats on SNB

[Keith Packard]

#part sign=pgpmime
On Wed, 07 Mar 2012 14:04:28 -0800, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 Yeah, only 1/2 belongs in fixes.  The other one needs another patch on
 top to avoid unpinning an active buffer.

Hence 'sketchy' :-)

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

Re: [Intel-gfx] [RFC] drm/i915: read current config at init time to avoid flicker

[Keith Packard]

#part sign=pgpmime
On Thu, 1 Mar 2012 12:37:47 -0800, Jesse Barnes jbar...@virtuousgeek.org 
wrote:
 The intent here is to build enough of the configuration to allow
 set_config to avoid mode setting if possible.

When I played with this, not setting the fb across this process tended
to do 'bad' things to the hardware -- the scanout would get unmapped
when the GTT was initialized...

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