Re: [Intel-gfx] i915 DVI resolution regression (3.13.7+)
On 9 April 2014 15:08, Jani Nikula jani.nik...@linux.intel.com wrote: On Wed, 09 Apr 2014, Dave Airlie airl...@gmail.com wrote: On Wed, Apr 9, 2014 at 4:07 PM, Daniel J Blueman dan...@quora.org wrote: On 9 April 2014 11:41, Dave Airlie airl...@gmail.com wrote: On Tue, Apr 8, 2014 at 5:32 PM, Daniel J Blueman dan...@quora.org wrote: On 8 April 2014 15:14, Jani Nikula jani.nik...@linux.intel.com wrote: On Tue, 08 Apr 2014, Daniel J Blueman dan...@quora.org wrote: Ville et al, It looks like commit e3ea8fa6beaf55fee64bf816f3b8a80ad733b2c2 (or another commit in 3.13.7) broke modes which require DVI-D dual-link, eg 2560x1440 with my panel. I don't see these modelines in 3.13.7 or later (eg 3.14): [ 5.582] (II) intel(0): Modeline 2560x1440x60.0 312.25 2560 2752 3024 3488 1440 1443 1448 1493 -hsync +vsync (89.5 kHz eP) [ 5.582] (II) intel(0): Modeline 2560x1440x60.0 312.25 2560 2752 3024 3488 1440 1443 1448 1493 -hsync +vsync (89.5 kHz eP) [ 5.582] (II) intel(0): Modeline 1920x1200x59.9 193.25 1920 2056 2256 2592 1200 1203 1209 1245 -hsync +vsync (74.6 kHz e) My monitor is a Dell U2713HM; mobo uses an H87 chipset with i5-4670. By allowing those modes we regressed setups which were not capable of displaying them. So you've got an HDMI-DVI converter? https://bugzilla.kernel.org/show_bug.cgi?id=72961 I am using a dual-link DVI-D to DVI-D cable to this monitor, since I previously couldn't get 2560x1440 via HDMI. Intel hw has dual-link DVI-D? I'm not sure I've ever seen that, is this SDVO device or plain DVI-D? It's the DVI-D connector on: https://www.asus.com/Motherboards/H87IPLUS/ The link which says Integrated Graphics Processor - Supports HDMI with max. resolution 4096 x 2304 @ 24 Hz - Supports DVI with max. resolution 1920 x 1200 @ 60 Hz - Supports RGB with max. resolution 1920 x 1200 @ 60 Hz I'm even wondering electrically how a HDMI-dual-link DVI adapter works. The current assumption is that in the working case, they are really operated in single-link, with a frequency beyond the single-link DVI cable spec. I checked with a single link DVI-D cable, and see the same behaviour as with a dual link cable. This would have had to be driven out of spec to achieve the resolution and refresh rate of my panel. That said, the spec is old and the shielding on the dual-link cable supplied with the panel is way over-specced. If Intel GPUs (such as my Haswell board here) don't support dual-link DVI, then this patch will prevent using panels 1920x1200 unless they advertise low-refresh rate modes. It looks like the panel manufacturers assume dual-link will be available. The lesser of the evils may be to allow reasonable out-of-spec dotclocks, unless I'm missing something? -- Daniel J Blueman ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC-PATCH libdrm] intel/aub: allow to dump only states
Add drm_intel_bufmgr_gem_set_aub_state_only to disable dumping of unannotated
or untyped data. The result should still be a valid AUB dump, in that it can
be fed to the simulator. But it will not trigger execution.
This can be used to dump states in binary form.
Signed-off-by: Chia-I Wu olva...@gmail.com
---
intel/intel_bufmgr.h | 3 +++
intel/intel_bufmgr_gem.c | 47 +++
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 9383c72..46c25ce 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -180,6 +180,9 @@ void drm_intel_gem_bo_start_gtt_access(drm_intel_bo *bo,
int write_enable);
void
drm_intel_bufmgr_gem_set_aub_filename(drm_intel_bufmgr *bufmgr,
const char *filename);
+void
+drm_intel_bufmgr_gem_set_aub_state_only(drm_intel_bufmgr *bufmgr,
+ int enable);
void drm_intel_bufmgr_gem_set_aub_dump(drm_intel_bufmgr *bufmgr, int enable);
void drm_intel_gem_bo_aub_dump_bmp(drm_intel_bo *bo,
int x1, int y1, int width, int height,
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 007a6d8..c91cc3f 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -129,6 +129,7 @@ typedef struct _drm_intel_bufmgr_gem {
bool fenced_relocs;
char *aub_filename;
+ int aub_state_only;
FILE *aub_file;
uint32_t aub_offset;
} drm_intel_bufmgr_gem;
@@ -2018,6 +2019,7 @@ aub_write_large_trace_block(drm_intel_bo *bo, uint32_t
type, uint32_t subtype,
static void
aub_write_bo(drm_intel_bo *bo)
{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo-bufmgr;
drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
uint32_t offset = 0;
unsigned i;
@@ -2029,19 +2031,42 @@ aub_write_bo(drm_intel_bo *bo)
drm_intel_aub_annotation *annotation =
bo_gem-aub_annotations[i];
uint32_t ending_offset = annotation-ending_offset;
+ bool write;
+
+ if (ending_offset = offset)
+ continue;
+
if (ending_offset bo-size)
ending_offset = bo-size;
- if (ending_offset offset) {
+
+ if (bufmgr_gem-aub_state_only) {
+ switch (annotation-type) {
+ case AUB_TRACE_TYPE_BATCH:
+ case AUB_TRACE_TYPE_CONSTANT_BUFFER:
+ case AUB_TRACE_TYPE_GENERAL:
+ case AUB_TRACE_TYPE_SURFACE:
+ write = true;
+ break;
+ default:
+ write = false;
+ break;
+ }
+ } else {
+ write = true;
+ }
+
+ if (write) {
aub_write_large_trace_block(bo, annotation-type,
annotation-subtype,
offset,
ending_offset - offset);
- offset = ending_offset;
}
+
+ offset = ending_offset;
}
/* Write out any remaining unannotated data */
- if (offset bo-size) {
+ if (offset bo-size !bufmgr_gem-aub_state_only) {
aub_write_large_trace_block(bo, AUB_TRACE_TYPE_NOTYPE, 0,
offset, bo-size - offset);
}
@@ -2170,7 +2195,8 @@ aub_exec(drm_intel_bo *bo, int ring_flag, int used)
drm_intel_bufmgr_gem_set_aub_annotations(bo, NULL, 0);
/* Dump ring buffer */
- aub_build_dump_ringbuffer(bufmgr_gem, bo_gem-aub_offset, ring_flag);
+ if (!bufmgr_gem-aub_state_only)
+ aub_build_dump_ringbuffer(bufmgr_gem, bo_gem-aub_offset,
ring_flag);
fflush(bufmgr_gem-aub_file);
@@ -2974,6 +3000,19 @@ drm_intel_bufmgr_gem_set_aub_filename(drm_intel_bufmgr
*bufmgr,
}
/**
+ * Sets the AUB dumping rule.
+ *
+ * When true, only states are dumped.
+ */
+void
+drm_intel_bufmgr_gem_set_aub_state_only(drm_intel_bufmgr *bufmgr,
+ int enable)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+ bufmgr_gem-aub_state_only = enable;
+}
+
+/**
* Sets up AUB dumping.
*
* This is a trace file format that can be used with the simulator.
--
1.8.5.3
___
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: Only break encoder linked when linked to connector
On Mon, Apr 14, 2014 at 07:26:08PM +0200, Egbert Eich wrote: Depending on the SDVO output_flags SDVO may have multiple connectors linking to the same encoder (in intel_connector-encoder-base). Only one of those connectors should be active (ie link to the encoder thru drm_connector-encoder. If intel_connector_break_all_links() is called from intel_sanitize_crtc() we may brake the crtc connection of an encoder thru an inactive connector in which case intel_connector_break_all_links() will not be called again for the active connector due to if (connector-encoder-base.crtc != crtc-base) continue; in intel_sanitize_crtc(). This will however leave the drm_connector-encoder linkage for this active connector in place. Subsequently this will cause multiple warnings in intel_connector_check_state() to trigger and the driver will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL). To avoid this break the encoder links only if the connector is active (ie. has drm_connector-encoder set). Signed-off-by: Egbert Eich e...@suse.de This just leaves me with a nagging doubt that not all links will then be broken. Do we have sufficient sanity checks to detect the obverse? Would it be worth adding a second pass over the connector_list to assert that all links are then broken? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC-PATCH libdrm] intel/aub: allow to dump only states
On Tue, Apr 15, 2014 at 3:37 PM, Chia-I Wu olva...@gmail.com wrote:
Add drm_intel_bufmgr_gem_set_aub_state_only to disable dumping of unannotated
or untyped data. The result should still be a valid AUB dump, in that it can
be fed to the simulator. But it will not trigger execution.
This can be used to dump states in binary form.
The intended use is to create a binary and offline alternative of
Mesa's INTEL_DEBUG=batch. Right now the binary output can only be
decoded with deaub from https://github.com/olvaffe/envytools. But I
can update test_decode to accept AUB files if this change is welcomed.
deaub is an XML-based (rules-ng-ng based) command/state decoder for
GEN6+. I am also curious if there is any interest in it outside deaub
(and ilo).
Signed-off-by: Chia-I Wu olva...@gmail.com
---
intel/intel_bufmgr.h | 3 +++
intel/intel_bufmgr_gem.c | 47 +++
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 9383c72..46c25ce 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -180,6 +180,9 @@ void drm_intel_gem_bo_start_gtt_access(drm_intel_bo *bo,
int write_enable);
void
drm_intel_bufmgr_gem_set_aub_filename(drm_intel_bufmgr *bufmgr,
const char *filename);
+void
+drm_intel_bufmgr_gem_set_aub_state_only(drm_intel_bufmgr *bufmgr,
+ int enable);
void drm_intel_bufmgr_gem_set_aub_dump(drm_intel_bufmgr *bufmgr, int enable);
void drm_intel_gem_bo_aub_dump_bmp(drm_intel_bo *bo,
int x1, int y1, int width, int height,
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 007a6d8..c91cc3f 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -129,6 +129,7 @@ typedef struct _drm_intel_bufmgr_gem {
bool fenced_relocs;
char *aub_filename;
+ int aub_state_only;
FILE *aub_file;
uint32_t aub_offset;
} drm_intel_bufmgr_gem;
@@ -2018,6 +2019,7 @@ aub_write_large_trace_block(drm_intel_bo *bo, uint32_t
type, uint32_t subtype,
static void
aub_write_bo(drm_intel_bo *bo)
{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)
bo-bufmgr;
drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
uint32_t offset = 0;
unsigned i;
@@ -2029,19 +2031,42 @@ aub_write_bo(drm_intel_bo *bo)
drm_intel_aub_annotation *annotation =
bo_gem-aub_annotations[i];
uint32_t ending_offset = annotation-ending_offset;
+ bool write;
+
+ if (ending_offset = offset)
+ continue;
+
if (ending_offset bo-size)
ending_offset = bo-size;
- if (ending_offset offset) {
+
+ if (bufmgr_gem-aub_state_only) {
+ switch (annotation-type) {
+ case AUB_TRACE_TYPE_BATCH:
+ case AUB_TRACE_TYPE_CONSTANT_BUFFER:
+ case AUB_TRACE_TYPE_GENERAL:
+ case AUB_TRACE_TYPE_SURFACE:
+ write = true;
+ break;
+ default:
+ write = false;
+ break;
+ }
+ } else {
+ write = true;
+ }
+
+ if (write) {
aub_write_large_trace_block(bo, annotation-type,
annotation-subtype,
offset,
ending_offset - offset);
- offset = ending_offset;
}
+
+ offset = ending_offset;
}
/* Write out any remaining unannotated data */
- if (offset bo-size) {
+ if (offset bo-size !bufmgr_gem-aub_state_only) {
aub_write_large_trace_block(bo, AUB_TRACE_TYPE_NOTYPE, 0,
offset, bo-size - offset);
}
@@ -2170,7 +2195,8 @@ aub_exec(drm_intel_bo *bo, int ring_flag, int used)
drm_intel_bufmgr_gem_set_aub_annotations(bo, NULL, 0);
/* Dump ring buffer */
- aub_build_dump_ringbuffer(bufmgr_gem, bo_gem-aub_offset, ring_flag);
+ if (!bufmgr_gem-aub_state_only)
+ aub_build_dump_ringbuffer(bufmgr_gem, bo_gem-aub_offset,
ring_flag);
fflush(bufmgr_gem-aub_file);
@@ -2974,6 +3000,19 @@ drm_intel_bufmgr_gem_set_aub_filename(drm_intel_bufmgr
*bufmgr,
}
/**
+ * Sets the AUB dumping rule.
+ *
+ * When true, only states are dumped.
+ */
+void
+drm_intel_bufmgr_gem_set_aub_state_only(drm_intel_bufmgr
[Intel-gfx] [PATCH] drm/plane-helper: Don't fake-implement primary plane disabling
After thinking about this topic a bit more I've reached the conclusion
that implementing this doesn't make sense:
- The locking is all wrong: set_config(NULL) will also unlink encoders
and connectors, but those links are protected with the mode_config
mutex. In the -disable_plane callback we only hold all modeset
locks, but eventually we want to switch to just grabbing the
per-crtc (and maybe per-plane) locks as needed, maybe based on
ww_mutexes. Having a callback which absolutely needs all modeset
locks is bad for this conversion.
Note that the same isn't true for the provided -update_plane since
we've audited the crtc helpers to make sure that not encoder or
connector links are changed.
- There's no way to re-enable the plane with an -update_plane: The
connectors/encoder links are lost and so we can't re-enable the
CRTC. Even without that issue the driver might have reassigned some
shared resources (as opposed to e.g. DPMS off, where drivers are not
allowed to do that to make sure the CRTC can be enabled again).
- The semantics don't make much sense: Userspace asked to scan out
black (or some other color if the driver supports a background
color), not that the screen be disabled.
- Implementing proper primary plane support (i.e. actually disabling
the primary plane without disabling the CRTC) is really simple, at
least if all the hw needs is flipping a bit. The big task is
auditing all the interactions with other ioctls when the CRTC is on
but there's no primary plane (e.g. pageflips). And some of that work
still needs to be done.
Cc: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_plane_helper.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 9263fd5efa58..9540ff9f97fe 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -205,9 +205,9 @@ EXPORT_SYMBOL(drm_primary_helper_update);
*
* Provides a default plane disable handler for primary planes. This is
handler
* is called in response to a userspace SetPlane operation on the plane with a
- * NULL framebuffer parameter. We call the driver's modeset handler with a
NULL
- * framebuffer to disable the CRTC if no other planes are currently enabled.
- * If other planes are still enabled on the same CRTC, we return -EBUSY.
+ * NULL framebuffer parameter. It unconditionally fails the disable call with
+ * -EINVAL the only way to disable the primary plane without driver support is
+ * to disable the entier CRTC. Which does not match the plane -disable hook.
*
* Note that some hardware may be able to disable the primary plane without
* disabling the whole CRTC. Drivers for such hardware should provide their
@@ -216,34 +216,11 @@ EXPORT_SYMBOL(drm_primary_helper_update);
* disabled primary plane).
*
* RETURNS:
- * Zero on success, error code on failure
+ * Unconditionally returns -EINVAL.
*/
int drm_primary_helper_disable(struct drm_plane *plane)
{
- struct drm_plane *p;
- struct drm_mode_set set = {
- .crtc = plane-crtc,
- .fb = NULL,
- };
-
- if (plane-crtc == NULL || plane-fb == NULL)
- /* Already disabled */
- return 0;
-
- list_for_each_entry(p, plane-dev-mode_config.plane_list, head)
- if (p != plane p-fb) {
- DRM_DEBUG_KMS(Cannot disable primary plane while other
planes are still active on CRTC.\n);
- return -EBUSY;
- }
-
- /*
-* N.B. We call set_config() directly here rather than
-* drm_mode_set_config_internal() since drm_mode_setplane() already
-* handles the basic refcounting and we don't need the special
-* cross-CRTC refcounting (no chance of stealing connectors from
-* other CRTC's with this update).
-*/
- return plane-crtc-funcs-set_config(set);
+ return -EINVAL;
}
EXPORT_SYMBOL(drm_primary_helper_disable);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Code cleanup patch to fix checkpatch errors
This cleans up the checkpatch errors for the merged commit - commit d3b542fcfc72d7724585e3fd2c5e75351bc3df47 Author: Shobhit Kumar shobhit.ku...@intel.com Date: Mon Apr 14 11:00:34 2014 +0530 drm/i915: Add parsing support for new MIPI blocks in VBT Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com --- drivers/gpu/drm/i915/intel_bios.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 917f5bb..fba9efd 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -653,13 +653,15 @@ static u8 *goto_next_sequence(u8 *data, int *size) * skip by this element payload size * skip elem id, command flag and data type */ - if ((tmp = tmp - 5) 0) + tmp -= 5; + if (tmp 0) return NULL; data += 3; len = *((u16 *)data); - if ((tmp = tmp - len) 0) + tmp -= len; + if (tmp 0) return NULL; /* skip by len */ @@ -667,13 +669,15 @@ static u8 *goto_next_sequence(u8 *data, int *size) break; case MIPI_SEQ_ELEM_DELAY: /* skip by elem id, and delay is 4 bytes */ - if ((tmp = tmp - 5) 0) + tmp -= 5; + if (tmp 0) return NULL; data += 5; break; case MIPI_SEQ_ELEM_GPIO: - if ((tmp = tmp - 3) 0) + tmp -= 3; + if (tmp 0) return NULL; data += 3; -- 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/2] drm/i915: Only break encoder linked when linked to connector
Chris Wilson writes: On Mon, Apr 14, 2014 at 07:26:08PM +0200, Egbert Eich wrote: Depending on the SDVO output_flags SDVO may have multiple connectors linking to the same encoder (in intel_connector-encoder-base). Only one of those connectors should be active (ie link to the encoder thru drm_connector-encoder. If intel_connector_break_all_links() is called from intel_sanitize_crtc() we may brake the crtc connection of an encoder thru an inactive connector in which case intel_connector_break_all_links() will not be called again for the active connector due to if (connector-encoder-base.crtc != crtc-base) continue; in intel_sanitize_crtc(). This will however leave the drm_connector-encoder linkage for this active connector in place. Subsequently this will cause multiple warnings in intel_connector_check_state() to trigger and the driver will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL). To avoid this break the encoder links only if the connector is active (ie. has drm_connector-encoder set). Signed-off-by: Egbert Eich e...@suse.de This just leaves me with a nagging doubt that not all links will then be broken. Do we have sufficient sanity checks to detect the obverse? Would it be worth adding a second pass over the connector_list to assert that all links are then broken? Possibly. Maybe we should rework intel_sanitize_encoder() a bit: loop over all drm_connectors, see if a drm_connector-encoder points to the encoder in question and make sure that the intel_encoder state (ie connectors_active and base.crtc) is in sync with the results. I'll think about it some more ... Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect
On Mon, Apr 14, 2014 at 07:26:09PM +0200, Egbert Eich wrote: Depending on the SDVO output_flags SDVO may have multiple connectors. These are all initialized in intel_sdvo_output_setup(). The connector that is initialized later will override the encoder_type that has been set up by an earlier connector type initialization. Eventually the one that comes last wins. Eventually when intel_sdvo_detect() is called the active connector is determined. Delay encoder type initialization until the active connector is known and set it to the type that corresponds to this connector. Signed-off-by: Egbert Eich e...@suse.de Please kill the redundant encoder-encoder_type = DRM_MODE_ENCODER_NONE as I think that will help clarify that during init we set the intel_sdvo-type and then during the detection of the actual connected output we assign the encoder_type. Otherwise, lgtm. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: Enabling constant alpha drm property
Hi Ville,
Somehow I did not receive your review comments
(http://lists.freedesktop.org/archives/intel-gfx/2014-April/042956.html
) in my mailbox.
Here is my input w.r.t patches I have sent for review:
Currently my patches are modeling constant alpha blend factor (assuming
pre-multiplied format) as
Dc = Sc * Ca + (1 - Ca) * Dc
User space has to supply (alpha value, CONSTANT_ALPHA) through drm
property to invoke this blending.
Here are my queries on your proposed approach:
1. src factors are 1, Ca, Sa*Ca and dst factors are 0, 1-Ca, 1-Sa,
1-Sa*Ca: How is factor 0 derived and applicable to Dst and not
applicable to src.
2. With this approach user will know what blend equation is supported.
How does user supply the blend parameters? Should we have another
property that will have blend equation selected and corresponding
parameters?
3. Is flag suggested for pre-multiply (yes please, premultiply the data
for me) to be set during AddFB?
Kindly check my patches and provide feedback on the approach of stuffing
blend type and blend parameter into single 64bit drm property.
Thanks,
Sagar
On Thu, 2014-04-03 at 11:13 +0530, Sagar Arun Kamble wrote:
Hi Ville,
Could you please review these patches.
thanks,
Sagar
On Wed, 2014-04-02 at 11:42 +0530, Sagar Arun Kamble wrote:
Reminder for review :)
On Tue, 2014-04-01 at 10:21 +0530, Sagar Arun Kamble wrote:
Gentle Reminder for reviewing this and related patches:
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042350.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042351.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042352.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042587.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042354.html
Also, provide views on how do we handle pre-multiplied alpha pixel
formats or is there need to convey pre-multiplied alpha pixel format
through additional drm property?
Since we are already advertising supported pre-multiplied formats
during intel_plane_init/drm_plane_init by supplying vlv_plane_formats
etc.
On Tue, 2014-03-25 at 20:02 +0530, sagar.a.kam...@intel.com wrote:
From: Sagar Kamble sagar.a.kam...@intel.com
This patch enables constant alpha property for Sprite planes.
Client has to set BIT(DRM_BLEND_CONSTANT_ALPHA) | ((alpha value) 32)
for applying constant alpha on a plane. To disable constant alpha,
client has to set BIT(DRM_BLEND_SRC_COLOR)
v2: Fixing property value comparison in vlv_update_plane with the use
of BIT(). Replacing DRM_BLEND_NONE by DRM_BLEND_SRC_COLOR.
Reverting line spacing change in i915_driver_lastclose. Return value
of intel_plane_set_property is changed to -ENVAL [Damien's Review
Comments]
Testcase: igt/kms_blend
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Tested-by: Sharma, Ankitprasad R ankitprasad.r.sha...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 10 ++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_drv.h| 7 +
drivers/gpu/drm/i915/intel_sprite.c | 61
-
5 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index e4d2b9f..e3a94a3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1898,12 +1898,22 @@ void i915_driver_lastclose(struct drm_device *
dev)
{
drm_i915_private_t *dev_priv = dev-dev_private;
+ struct intel_plane *plane;
/* On gen6+ we refuse to init without kms enabled, but then the
drm core
* goes right around and calls lastclose. Check for this and
don't clean
* up anything. */
if (!dev_priv)
return;
+ if (dev_priv-blend_property) {
+ list_for_each_entry(plane,
dev-mode_config.plane_list, base.head) {
+ plane-blend_param.value =
BIT(DRM_BLEND_SRC_COLOR);
+ drm_object_property_set_value(plane-base.base,
+
dev_priv-blend_property,
+
plane-blend_param.value);
+ }
+ }
+
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
intel_fbdev_restore_mode(dev);
vga_switcheroo_process_delayed_switch();
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 70fbe90..0d1be70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++
Re: [Intel-gfx] i915 DVI resolution regression (3.13.7+)
On Tue, Apr 15, 2014 at 02:27:41PM +0800, Daniel J Blueman wrote: On 9 April 2014 15:08, Jani Nikula jani.nik...@linux.intel.com wrote: On Wed, 09 Apr 2014, Dave Airlie airl...@gmail.com wrote: On Wed, Apr 9, 2014 at 4:07 PM, Daniel J Blueman dan...@quora.org wrote: On 9 April 2014 11:41, Dave Airlie airl...@gmail.com wrote: On Tue, Apr 8, 2014 at 5:32 PM, Daniel J Blueman dan...@quora.org wrote: On 8 April 2014 15:14, Jani Nikula jani.nik...@linux.intel.com wrote: On Tue, 08 Apr 2014, Daniel J Blueman dan...@quora.org wrote: Ville et al, It looks like commit e3ea8fa6beaf55fee64bf816f3b8a80ad733b2c2 (or another commit in 3.13.7) broke modes which require DVI-D dual-link, eg 2560x1440 with my panel. I don't see these modelines in 3.13.7 or later (eg 3.14): [ 5.582] (II) intel(0): Modeline 2560x1440x60.0 312.25 2560 2752 3024 3488 1440 1443 1448 1493 -hsync +vsync (89.5 kHz eP) [ 5.582] (II) intel(0): Modeline 2560x1440x60.0 312.25 2560 2752 3024 3488 1440 1443 1448 1493 -hsync +vsync (89.5 kHz eP) [ 5.582] (II) intel(0): Modeline 1920x1200x59.9 193.25 1920 2056 2256 2592 1200 1203 1209 1245 -hsync +vsync (74.6 kHz e) My monitor is a Dell U2713HM; mobo uses an H87 chipset with i5-4670. By allowing those modes we regressed setups which were not capable of displaying them. So you've got an HDMI-DVI converter? https://bugzilla.kernel.org/show_bug.cgi?id=72961 I am using a dual-link DVI-D to DVI-D cable to this monitor, since I previously couldn't get 2560x1440 via HDMI. Intel hw has dual-link DVI-D? I'm not sure I've ever seen that, is this SDVO device or plain DVI-D? It's the DVI-D connector on: https://www.asus.com/Motherboards/H87IPLUS/ The link which says Integrated Graphics Processor - Supports HDMI with max. resolution 4096 x 2304 @ 24 Hz - Supports DVI with max. resolution 1920 x 1200 @ 60 Hz - Supports RGB with max. resolution 1920 x 1200 @ 60 Hz I'm even wondering electrically how a HDMI-dual-link DVI adapter works. The current assumption is that in the working case, they are really operated in single-link, with a frequency beyond the single-link DVI cable spec. I checked with a single link DVI-D cable, and see the same behaviour as with a dual link cable. This would have had to be driven out of spec to achieve the resolution and refresh rate of my panel. That said, the spec is old and the shielding on the dual-link cable supplied with the panel is way over-specced. If Intel GPUs (such as my Haswell board here) don't support dual-link DVI, then this patch will prevent using panels 1920x1200 unless they advertise low-refresh rate modes. It looks like the panel manufacturers assume dual-link will be available. I don't think I've ever seen a display that would _require_ dual-link. You should at least get some kind of picture with single-link. The lesser of the evils may be to allow reasonable out-of-spec dotclocks, unless I'm missing something? No, we can't do that. If the display won't accept the out of spec signal you get no picture. If the display will accept higher frequencies it needs to tell us that by including the HDMI VSDB in the EDID. There's no other way for us to know. What we do now is allow the user to specify a mode manually that's out of spec, but we won't include such modes in the list automatically. That should guarantee that everyone gets some kind of picture on the screen off the bat, while still allowing the user to force any mode he wishes. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/plane-helper: Don't fake-implement primary plane disabling
On Tue, Apr 15, 2014 at 10:02:43AM +0200, Daniel Vetter wrote:
After thinking about this topic a bit more I've reached the conclusion
that implementing this doesn't make sense:
- The locking is all wrong: set_config(NULL) will also unlink encoders
and connectors, but those links are protected with the mode_config
mutex. In the -disable_plane callback we only hold all modeset
locks, but eventually we want to switch to just grabbing the
per-crtc (and maybe per-plane) locks as needed, maybe based on
ww_mutexes. Having a callback which absolutely needs all modeset
locks is bad for this conversion.
Note that the same isn't true for the provided -update_plane since
we've audited the crtc helpers to make sure that not encoder or
connector links are changed.
- There's no way to re-enable the plane with an -update_plane: The
connectors/encoder links are lost and so we can't re-enable the
CRTC. Even without that issue the driver might have reassigned some
shared resources (as opposed to e.g. DPMS off, where drivers are not
allowed to do that to make sure the CRTC can be enabled again).
- The semantics don't make much sense: Userspace asked to scan out
black (or some other color if the driver supports a background
color), not that the screen be disabled.
- Implementing proper primary plane support (i.e. actually disabling
the primary plane without disabling the CRTC) is really simple, at
least if all the hw needs is flipping a bit. The big task is
auditing all the interactions with other ioctls when the CRTC is on
but there's no primary plane (e.g. pageflips). And some of that work
still needs to be done.
Cc: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Although I'm not sure EINVAL is the best choice here. Maybe ENOSYS?
---
drivers/gpu/drm/drm_plane_helper.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 9263fd5efa58..9540ff9f97fe 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -205,9 +205,9 @@ EXPORT_SYMBOL(drm_primary_helper_update);
*
* Provides a default plane disable handler for primary planes. This is
handler
* is called in response to a userspace SetPlane operation on the plane with
a
- * NULL framebuffer parameter. We call the driver's modeset handler with a
NULL
- * framebuffer to disable the CRTC if no other planes are currently enabled.
- * If other planes are still enabled on the same CRTC, we return -EBUSY.
+ * NULL framebuffer parameter. It unconditionally fails the disable call
with
+ * -EINVAL the only way to disable the primary plane without driver support
is
+ * to disable the entier CRTC. Which does not match the plane -disable hook.
*
* Note that some hardware may be able to disable the primary plane without
* disabling the whole CRTC. Drivers for such hardware should provide their
@@ -216,34 +216,11 @@ EXPORT_SYMBOL(drm_primary_helper_update);
* disabled primary plane).
*
* RETURNS:
- * Zero on success, error code on failure
+ * Unconditionally returns -EINVAL.
*/
int drm_primary_helper_disable(struct drm_plane *plane)
{
- struct drm_plane *p;
- struct drm_mode_set set = {
- .crtc = plane-crtc,
- .fb = NULL,
- };
-
- if (plane-crtc == NULL || plane-fb == NULL)
- /* Already disabled */
- return 0;
-
- list_for_each_entry(p, plane-dev-mode_config.plane_list, head)
- if (p != plane p-fb) {
- DRM_DEBUG_KMS(Cannot disable primary plane while other
planes are still active on CRTC.\n);
- return -EBUSY;
- }
-
- /*
- * N.B. We call set_config() directly here rather than
- * drm_mode_set_config_internal() since drm_mode_setplane() already
- * handles the basic refcounting and we don't need the special
- * cross-CRTC refcounting (no chance of stealing connectors from
- * other CRTC's with this update).
- */
- return plane-crtc-funcs-set_config(set);
+ return -EINVAL;
}
EXPORT_SYMBOL(drm_primary_helper_disable);
--
1.9.2
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: Enabling constant alpha drm property
On Tue, Apr 15, 2014 at 03:14:04PM +0530, Sagar Arun Kamble wrote:
Hi Ville,
Somehow I did not receive your review comments
(http://lists.freedesktop.org/archives/intel-gfx/2014-April/042956.html
) in my mailbox.
Here is my input w.r.t patches I have sent for review:
Currently my patches are modeling constant alpha blend factor (assuming
pre-multiplied format) as
Dc = Sc * Ca + (1 - Ca) * Dc
User space has to supply (alpha value, CONSTANT_ALPHA) through drm
property to invoke this blending.
Here are my queries on your proposed approach:
1. src factors are 1, Ca, Sa*Ca and dst factors are 0, 1-Ca, 1-Sa,
1-Sa*Ca: How is factor 0 derived and applicable to Dst and not
applicable to src.
If blending is disabled the function will be Dc = Sc * 1 + 0 * Dc
Hence we get the ONE and ZERO blend factors.
2. With this approach user will know what blend equation is supported.
How does user supply the blend parameters? Should we have another
property that will have blend equation selected and corresponding
parameters?
By parameters you mean the constant alpha and background color for
instance? I think having those as separate properties is the cleanest
option.
3. Is flag suggested for pre-multiply (yes please, premultiply the data
for me) to be set during AddFB?
If we define the flag that way, then I think we should make it part of
the blend equation rather than apply it to the FB. Or it could even be
a separate plane property.
Kindly check my patches and provide feedback on the approach of stuffing
blend type and blend parameter into single 64bit drm property.
I think we should avoid trying to stuff too much data into a single
property. IMO blend equation can be one property, constant alpha,
background color, pre-multiplication may be other properties.
Thanks,
Sagar
On Thu, 2014-04-03 at 11:13 +0530, Sagar Arun Kamble wrote:
Hi Ville,
Could you please review these patches.
thanks,
Sagar
On Wed, 2014-04-02 at 11:42 +0530, Sagar Arun Kamble wrote:
Reminder for review :)
On Tue, 2014-04-01 at 10:21 +0530, Sagar Arun Kamble wrote:
Gentle Reminder for reviewing this and related patches:
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042350.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042351.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042352.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042587.html
http://lists.freedesktop.org/archives/intel-gfx/2014-March/042354.html
Also, provide views on how do we handle pre-multiplied alpha pixel
formats or is there need to convey pre-multiplied alpha pixel format
through additional drm property?
Since we are already advertising supported pre-multiplied formats
during intel_plane_init/drm_plane_init by supplying vlv_plane_formats
etc.
On Tue, 2014-03-25 at 20:02 +0530, sagar.a.kam...@intel.com wrote:
From: Sagar Kamble sagar.a.kam...@intel.com
This patch enables constant alpha property for Sprite planes.
Client has to set BIT(DRM_BLEND_CONSTANT_ALPHA) | ((alpha value)
32)
for applying constant alpha on a plane. To disable constant alpha,
client has to set BIT(DRM_BLEND_SRC_COLOR)
v2: Fixing property value comparison in vlv_update_plane with the use
of BIT(). Replacing DRM_BLEND_NONE by DRM_BLEND_SRC_COLOR.
Reverting line spacing change in i915_driver_lastclose. Return value
of intel_plane_set_property is changed to -ENVAL [Damien's Review
Comments]
Testcase: igt/kms_blend
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Tested-by: Sharma, Ankitprasad R ankitprasad.r.sha...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 10 ++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_drv.h| 7 +
drivers/gpu/drm/i915/intel_sprite.c | 61
-
5 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index e4d2b9f..e3a94a3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1898,12 +1898,22 @@ void i915_driver_lastclose(struct drm_device
* dev)
{
drm_i915_private_t *dev_priv = dev-dev_private;
+ struct intel_plane *plane;
/* On gen6+ we refuse to init without kms enabled, but then the
drm core
* goes right around and calls lastclose. Check for this and
don't clean
* up anything. */
if (!dev_priv)
return;
+ if (dev_priv-blend_property) {
+
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: Enabling constant alpha drm property
Hi Ville, Thanks for the reply. I have few more queries. On Tue, 2014-04-15 at 13:35 +0300, Ville Syrjälä wrote: On Tue, Apr 15, 2014 at 03:14:04PM +0530, Sagar Arun Kamble wrote: Hi Ville, Somehow I did not receive your review comments (http://lists.freedesktop.org/archives/intel-gfx/2014-April/042956.html ) in my mailbox. Here is my input w.r.t patches I have sent for review: Currently my patches are modeling constant alpha blend factor (assuming pre-multiplied format) as Dc = Sc * Ca + (1 - Ca) * Dc User space has to supply (alpha value, CONSTANT_ALPHA) through drm property to invoke this blending. Here are my queries on your proposed approach: 1. src factors are 1, Ca, Sa*Ca and dst factors are 0, 1-Ca, 1-Sa, 1-Sa*Ca: How is factor 0 derived and applicable to Dst and not applicable to src. If blending is disabled the function will be Dc = Sc * 1 + 0 * Dc Hence we get the ONE and ZERO blend factors. 2. With this approach user will know what blend equation is supported. How does user supply the blend parameters? Should we have another property that will have blend equation selected and corresponding parameters? By parameters you mean the constant alpha and background color for instance? I think having those as separate properties is the cleanest option. Yes. I meant constant alpha value. For what type of blending background color will be used? So, the overall flow will be: 1. User queries supported blending equations. 2. Sets required blend equation. 3. Sets required blend parameters like constant alpha value etc. Step 2 and 3 need to be done atomically. 3. Is flag suggested for pre-multiply (yes please, premultiply the data for me) to be set during AddFB? If we define the flag that way, then I think we should make it part of the blend equation rather than apply it to the FB. Or it could even be a separate plane property. This flag is actually part of following factors: SRC_ALPHA, ONE_MINUS_SRC_ALPHA,SRC_CONST_ALPHA, ONE_MINUS_SRC_CONST_ALPHA. What action does driver take if user selects these blend factors? Change the pixel format to non-pre-multiplied format? This is again coming back to our previous discussion where changing pixel format on fly should not be done. Kindly check my patches and provide feedback on the approach of stuffing blend type and blend parameter into single 64bit drm property. I think we should avoid trying to stuff too much data into a single property. IMO blend equation can be one property, constant alpha, background color, pre-multiplication may be other properties. Thanks, Sagar On Thu, 2014-04-03 at 11:13 +0530, Sagar Arun Kamble wrote: Hi Ville, Could you please review these patches. thanks, Sagar On Wed, 2014-04-02 at 11:42 +0530, Sagar Arun Kamble wrote: Reminder for review :) On Tue, 2014-04-01 at 10:21 +0530, Sagar Arun Kamble wrote: Gentle Reminder for reviewing this and related patches: http://lists.freedesktop.org/archives/intel-gfx/2014-March/042350.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042351.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042352.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042587.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042354.html Also, provide views on how do we handle pre-multiplied alpha pixel formats or is there need to convey pre-multiplied alpha pixel format through additional drm property? Since we are already advertising supported pre-multiplied formats during intel_plane_init/drm_plane_init by supplying vlv_plane_formats etc. On Tue, 2014-03-25 at 20:02 +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com This patch enables constant alpha property for Sprite planes. Client has to set BIT(DRM_BLEND_CONSTANT_ALPHA) | ((alpha value) 32) for applying constant alpha on a plane. To disable constant alpha, client has to set BIT(DRM_BLEND_SRC_COLOR) v2: Fixing property value comparison in vlv_update_plane with the use of BIT(). Replacing DRM_BLEND_NONE by DRM_BLEND_SRC_COLOR. Reverting line spacing change in i915_driver_lastclose. Return value of intel_plane_set_property is changed to -ENVAL [Damien's Review Comments] Testcase: igt/kms_blend Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Jani Nikula jani.nik...@linux.intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Tested-by: Sharma, Ankitprasad R ankitprasad.r.sha...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 10 ++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 2 ++ drivers/gpu/drm/i915/intel_drv.h
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Jesse Barnes
Sent: Friday, April 11, 2014 11:46 PM
To: Ville Syrjälä
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband
reset on resume
On Fri, 11 Apr 2014 21:10:21 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Apr 11, 2014 at 10:35:40AM -0700, Jesse Barnes wrote:
On Fri, 11 Apr 2014 20:26:24 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on resume
to
properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the
time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
I'm open to better suggestions on how to do the power well toggle,
with
the existing code it looks like I'd have to walk through a bunch of
power domains looking for a match, then call a generic function which
will warn. I'd prefer to just expose the specific domains directly
for
low level platform code like this.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_pm.c |4 ++--
drivers/gpu/drm/i915/intel_uncore.c | 19 +++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index fa00185..3afd0bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5454,8 +5454,8 @@ static bool
i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
return true;
}
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
-struct i915_power_well *power_well, bool
enable)
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool
enable)
{
enum punit_power_well power_well_id = power_well-data;
u32 mask;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 2a72bab..f1abd2d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct
drm_device *dev, bool restore)
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool
enable);
+
void intel_uncore_early_sanitize(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct
drm_device *dev)
DRM_INFO(Found %zuMB of eLLC\n, dev_priv-
ellc_size);
}
+ /*
+ * From
VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+ * Need to assert and de-assert PHY SB reset by gating the
common
+ * lane power, then un-gating it.
+ * Simply ungating isn't enough to reset the PHY enough to get
+ * ports and lanes running.
+ */
+ if (IS_VALLEYVIEW(dev)) {
+ struct i915_power_well cmn_well = {
+ .data = PUNIT_POWER_WELL_DPIO_CMN_BC
+ };
+
+ vlv_set_power_well(dev_priv, cmn_well, false);
+ vlv_set_power_well(dev_priv, cmn_well, true);
+ }
Stick this into intel_reset_dpio() instead?
And what about fastboot and whatnot? Should we check if the display is
already up and running somehow before we go and kill it with this?
reset_dpio is too late.
How come? We shouldn't touch the PHY before it. So either reset_dpio is
in the wrong place for some reason, or something else gets called too
soon.
Oh actually I haven't tested with just the common reset, it may be ok
to put that into the DPIO init function. My earlier patch was toggling
all the wells, including display, which would obviously clobber things.
Following is my understanding after talking to PHY windows teams..
The exact sequence to follow during power gating (as part of the suspend
sequence):
- Power gate display controller poll for the operation to complete
- Power gate DPIO RX / TX lanes poll for the operation to complete
- Power gate DPIO common lanes poll for the operation to complete
The power ungating sequence
- Power
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: Enabling constant alpha drm property
On Tue, Apr 15, 2014 at 04:53:22PM +0530, Sagar Arun Kamble wrote: Hi Ville, Thanks for the reply. I have few more queries. On Tue, 2014-04-15 at 13:35 +0300, Ville Syrjälä wrote: On Tue, Apr 15, 2014 at 03:14:04PM +0530, Sagar Arun Kamble wrote: Hi Ville, Somehow I did not receive your review comments (http://lists.freedesktop.org/archives/intel-gfx/2014-April/042956.html ) in my mailbox. Here is my input w.r.t patches I have sent for review: Currently my patches are modeling constant alpha blend factor (assuming pre-multiplied format) as Dc = Sc * Ca + (1 - Ca) * Dc User space has to supply (alpha value, CONSTANT_ALPHA) through drm property to invoke this blending. Here are my queries on your proposed approach: 1. src factors are 1, Ca, Sa*Ca and dst factors are 0, 1-Ca, 1-Sa, 1-Sa*Ca: How is factor 0 derived and applicable to Dst and not applicable to src. If blending is disabled the function will be Dc = Sc * 1 + 0 * Dc Hence we get the ONE and ZERO blend factors. 2. With this approach user will know what blend equation is supported. How does user supply the blend parameters? Should we have another property that will have blend equation selected and corresponding parameters? By parameters you mean the constant alpha and background color for instance? I think having those as separate properties is the cleanest option. Yes. I meant constant alpha value. For what type of blending background color will be used? The background color just defines what color is at the bottom of the stack. Typically it's black, but on certain hardware you can configure it. The only really good use for this feature I can think of is power saving. If most of the screen is supposed to be painted with a single solid color, we could configure that background color accordingly and configure the planes to cover only the parts of the screen where there's some other content. This way there would be no memory fetches for most of screen. So, the overall flow will be: 1. User queries supported blending equations. 2. Sets required blend equation. 3. Sets required blend parameters like constant alpha value etc. Step 2 and 3 need to be done atomically. Nuclear flip ftw. Before we get that, the best option is probably to turn off the plane, configure all the properties, and re-enable the plane. Obviosuly you still risk glitches during the plane enable/disable, but that's already the case anyway. 3. Is flag suggested for pre-multiply (yes please, premultiply the data for me) to be set during AddFB? If we define the flag that way, then I think we should make it part of the blend equation rather than apply it to the FB. Or it could even be a separate plane property. This flag is actually part of following factors: SRC_ALPHA, ONE_MINUS_SRC_ALPHA,SRC_CONST_ALPHA, ONE_MINUS_SRC_CONST_ALPHA. No, based on the docs I have our hardware can effectively perform the Sc*Sa twice. Once as the premultiplication step, and a second time as part of the pipe blending. The pipe blending is what the blend equation will control, and in addition the premultiplication can be performed by the plane before the data even enters the pipe blender. So we need both the SRC_ALPHA blend factors and some kind of a premultiplication control. What action does driver take if user selects these blend factors? Change the pixel format to non-pre-multiplied format? This is again coming back to our previous discussion where changing pixel format on fly should not be done. Kindly check my patches and provide feedback on the approach of stuffing blend type and blend parameter into single 64bit drm property. I think we should avoid trying to stuff too much data into a single property. IMO blend equation can be one property, constant alpha, background color, pre-multiplication may be other properties. Thanks, Sagar On Thu, 2014-04-03 at 11:13 +0530, Sagar Arun Kamble wrote: Hi Ville, Could you please review these patches. thanks, Sagar On Wed, 2014-04-02 at 11:42 +0530, Sagar Arun Kamble wrote: Reminder for review :) On Tue, 2014-04-01 at 10:21 +0530, Sagar Arun Kamble wrote: Gentle Reminder for reviewing this and related patches: http://lists.freedesktop.org/archives/intel-gfx/2014-March/042350.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042351.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042352.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042587.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042354.html Also, provide views on how do we handle pre-multiplied alpha pixel formats or is there need to convey pre-multiplied alpha pixel format through additional drm
Re: [Intel-gfx] [PATCH] drm/plane-helper: Don't fake-implement primary plane disabling
On Tue, Apr 15, 2014 at 12:22 PM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: Although I'm not sure EINVAL is the best choice here. Maybe ENOSYS? We return -EINVAL everywhere else where we don't support a giving config, e.g. lack of dpll, unsupported plane scaling, pixel format mismatch of the primary plane (since we don't yet have per-gen lists of supported formats for the primary plane) and so on. So I've figured -EINVAL is good enough until we have something better with the test mode for atomic modesets. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume
On Tue, Apr 15, 2014 at 11:39:41AM +, Purushothaman, Vijay A wrote:
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of
Jesse Barnes
Sent: Friday, April 11, 2014 11:46 PM
To: Ville Syrjälä
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband
reset on resume
On Fri, 11 Apr 2014 21:10:21 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Apr 11, 2014 at 10:35:40AM -0700, Jesse Barnes wrote:
On Fri, 11 Apr 2014 20:26:24 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on
resume to
properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the
time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
I'm open to better suggestions on how to do the power well toggle,
with
the existing code it looks like I'd have to walk through a bunch of
power domains looking for a match, then call a generic function
which
will warn. I'd prefer to just expose the specific domains directly
for
low level platform code like this.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_pm.c |4 ++--
drivers/gpu/drm/i915/intel_uncore.c | 19 +++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index fa00185..3afd0bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5454,8 +5454,8 @@ static bool
i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
return true;
}
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
- struct i915_power_well *power_well, bool
enable)
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool
enable)
{
enum punit_power_well power_well_id = power_well-data;
u32 mask;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 2a72bab..f1abd2d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct
drm_device *dev, bool restore)
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool
enable);
+
void intel_uncore_early_sanitize(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct
drm_device *dev)
DRM_INFO(Found %zuMB of eLLC\n, dev_priv-
ellc_size);
}
+ /*
+* From
VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the
common
+* lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+ if (IS_VALLEYVIEW(dev)) {
+ struct i915_power_well cmn_well = {
+ .data = PUNIT_POWER_WELL_DPIO_CMN_BC
+ };
+
+ vlv_set_power_well(dev_priv, cmn_well, false);
+ vlv_set_power_well(dev_priv, cmn_well, true);
+ }
Stick this into intel_reset_dpio() instead?
And what about fastboot and whatnot? Should we check if the display is
already up and running somehow before we go and kill it with this?
reset_dpio is too late.
How come? We shouldn't touch the PHY before it. So either reset_dpio is
in the wrong place for some reason, or something else gets called too
soon.
Oh actually I haven't tested with just the common reset, it may be ok
to put that into the DPIO init function. My earlier patch was toggling
all the wells, including display, which would obviously clobber things.
Following is my understanding after talking to PHY windows teams..
The exact sequence to follow during power gating (as part of the suspend
sequence):
- Power gate display controller poll for the operation to complete
- Power gate
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume
On Tue, 2014-04-15 at 11:39 +, Purushothaman, Vijay A wrote:
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of
Jesse Barnes
Sent: Friday, April 11, 2014 11:46 PM
To: Ville Syrjälä
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband
reset on resume
On Fri, 11 Apr 2014 21:10:21 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Apr 11, 2014 at 10:35:40AM -0700, Jesse Barnes wrote:
On Fri, 11 Apr 2014 20:26:24 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on
resume to
properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the
time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
I'm open to better suggestions on how to do the power well toggle,
with
the existing code it looks like I'd have to walk through a bunch of
power domains looking for a match, then call a generic function
which
will warn. I'd prefer to just expose the specific domains directly
for
low level platform code like this.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_pm.c |4 ++--
drivers/gpu/drm/i915/intel_uncore.c | 19 +++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index fa00185..3afd0bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5454,8 +5454,8 @@ static bool
i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
return true;
}
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
- struct i915_power_well *power_well, bool
enable)
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool
enable)
{
enum punit_power_well power_well_id = power_well-data;
u32 mask;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 2a72bab..f1abd2d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct
drm_device *dev, bool restore)
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool
enable);
+
void intel_uncore_early_sanitize(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct
drm_device *dev)
DRM_INFO(Found %zuMB of eLLC\n, dev_priv-
ellc_size);
}
+ /*
+* From
VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the
common
+* lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+ if (IS_VALLEYVIEW(dev)) {
+ struct i915_power_well cmn_well = {
+ .data = PUNIT_POWER_WELL_DPIO_CMN_BC
+ };
+
+ vlv_set_power_well(dev_priv, cmn_well, false);
+ vlv_set_power_well(dev_priv, cmn_well, true);
+ }
Stick this into intel_reset_dpio() instead?
And what about fastboot and whatnot? Should we check if the display is
already up and running somehow before we go and kill it with this?
reset_dpio is too late.
How come? We shouldn't touch the PHY before it. So either reset_dpio is
in the wrong place for some reason, or something else gets called too
soon.
Oh actually I haven't tested with just the common reset, it may be ok
to put that into the DPIO init function. My earlier patch was toggling
all the wells, including display, which would obviously clobber things.
Following is my understanding after talking to PHY windows teams..
The exact sequence to follow during power gating (as part of the suspend
sequence):
- Power gate display controller poll for the operation to complete
- Power gate DPIO RX /
[Intel-gfx] [PATCH v3 24/25] drm/i915: propagate the error code from runtime PM callbacks
Atm, none of the RPM callbacks can fail, but the next patch adding
RPM support for VLV changes this, so prepare for it.
In case one of these callbacks return error RPM will get permanently
disabled until the error is explicitly cleared. In the future we could
add support for re-enabling it, for example after resetting the HW, but
for now - hopefully - we can live with the simpler solution.
v2:
- propagate the error from the resume callbacks too (Paulo)
v3:
- fix rebase fail typo around IS_GEN6() check in intel_runtime_suspend()
Signed-off-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 57 ++---
1 file changed, 42 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 845e1e1..aeb7dec 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -888,21 +888,27 @@ static int i915_pm_poweroff(struct device *dev)
return i915_drm_freeze(drm_dev);
}
-static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
+static int hsw_runtime_suspend(struct drm_i915_private *dev_priv)
{
hsw_enable_pc8(dev_priv);
+
+ return 0;
}
-static void snb_runtime_resume(struct drm_i915_private *dev_priv)
+static int snb_runtime_resume(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv-dev;
intel_init_pch_refclk(dev);
+
+ return 0;
}
-static void hsw_runtime_resume(struct drm_i915_private *dev_priv)
+static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
{
hsw_disable_pc8(dev_priv);
+
+ return 0;
}
int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
@@ -947,6 +953,7 @@ static int intel_runtime_suspend(struct device *device)
struct pci_dev *pdev = to_pci_dev(device);
struct drm_device *dev = pci_get_drvdata(pdev);
struct drm_i915_private *dev_priv = dev-dev_private;
+ int ret;
if (WARN_ON_ONCE(!(dev_priv-rps.enabled intel_enable_rc6(dev
return -ENODEV;
@@ -959,12 +966,21 @@ static int intel_runtime_suspend(struct device *device)
intel_runtime_pm_disable_interrupts(dev);
cancel_work_sync(dev_priv-rps.work);
- if (IS_GEN6(dev))
- ;
- else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- hsw_runtime_suspend(dev_priv);
- else
+ if (IS_GEN6(dev)) {
+ ret = 0;
+ } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+ ret = hsw_runtime_suspend(dev_priv);
+ } else {
+ ret = -ENODEV;
WARN_ON(1);
+ }
+
+ if (ret) {
+ DRM_ERROR(Runtime suspend failed, disabling it (%d)\n, ret);
+ intel_runtime_pm_restore_interrupts(dev);
+
+ return ret;
+ }
i915_gem_release_all_mmaps(dev_priv);
@@ -989,6 +1005,7 @@ static int intel_runtime_resume(struct device *device)
struct pci_dev *pdev = to_pci_dev(device);
struct drm_device *dev = pci_get_drvdata(pdev);
struct drm_i915_private *dev_priv = dev-dev_private;
+ int ret;
WARN_ON(!HAS_RUNTIME_PM(dev));
@@ -997,21 +1014,31 @@ static int intel_runtime_resume(struct device *device)
intel_opregion_notify_adapter(dev, PCI_D0);
dev_priv-pm.suspended = false;
- if (IS_GEN6(dev))
- snb_runtime_resume(dev_priv);
- else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- hsw_runtime_resume(dev_priv);
- else
+ if (IS_GEN6(dev)) {
+ ret = snb_runtime_resume(dev_priv);
+ } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+ ret = hsw_runtime_resume(dev_priv);
+ } else {
WARN_ON(1);
+ ret = -ENODEV;
+ }
+ /*
+* No point of rolling back things in case of an error, as the best
+* we can do is to hope that things will still work (and disable RPM).
+*/
i915_gem_init_swizzling(dev);
gen6_update_ring_freq(dev);
intel_reset_gt_powersave(dev);
intel_runtime_pm_restore_interrupts(dev);
- DRM_DEBUG_KMS(Device resumed\n);
- return 0;
+ if (ret)
+ DRM_ERROR(Runtime resume failed, disabling it (%d)\n, ret);
+ else
+ DRM_DEBUG_KMS(Device resumed\n);
+
+ return ret;
}
static const struct dev_pm_ops i915_pm_ops = {
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/plane-helper: Don't fake-implement primary plane disabling
On Tue, Apr 15, 2014 at 10:02:43AM +0200, Daniel Vetter wrote:
After thinking about this topic a bit more I've reached the conclusion
that implementing this doesn't make sense:
- The locking is all wrong: set_config(NULL) will also unlink encoders
and connectors, but those links are protected with the mode_config
mutex. In the -disable_plane callback we only hold all modeset
locks, but eventually we want to switch to just grabbing the
per-crtc (and maybe per-plane) locks as needed, maybe based on
ww_mutexes. Having a callback which absolutely needs all modeset
locks is bad for this conversion.
Note that the same isn't true for the provided -update_plane since
we've audited the crtc helpers to make sure that not encoder or
connector links are changed.
- There's no way to re-enable the plane with an -update_plane: The
connectors/encoder links are lost and so we can't re-enable the
CRTC. Even without that issue the driver might have reassigned some
shared resources (as opposed to e.g. DPMS off, where drivers are not
allowed to do that to make sure the CRTC can be enabled again).
- The semantics don't make much sense: Userspace asked to scan out
black (or some other color if the driver supports a background
color), not that the screen be disabled.
- Implementing proper primary plane support (i.e. actually disabling
the primary plane without disabling the CRTC) is really simple, at
least if all the hw needs is flipping a bit. The big task is
auditing all the interactions with other ioctls when the CRTC is on
but there's no primary plane (e.g. pageflips). And some of that work
still needs to be done.
Cc: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Yeah, I agree this is probably a better way to go.
Reviewed-by: Matt Roper matthew.d.ro...@intel.com
Matt
---
drivers/gpu/drm/drm_plane_helper.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 9263fd5efa58..9540ff9f97fe 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -205,9 +205,9 @@ EXPORT_SYMBOL(drm_primary_helper_update);
*
* Provides a default plane disable handler for primary planes. This is
handler
* is called in response to a userspace SetPlane operation on the plane with
a
- * NULL framebuffer parameter. We call the driver's modeset handler with a
NULL
- * framebuffer to disable the CRTC if no other planes are currently enabled.
- * If other planes are still enabled on the same CRTC, we return -EBUSY.
+ * NULL framebuffer parameter. It unconditionally fails the disable call
with
+ * -EINVAL the only way to disable the primary plane without driver support
is
+ * to disable the entier CRTC. Which does not match the plane -disable hook.
*
* Note that some hardware may be able to disable the primary plane without
* disabling the whole CRTC. Drivers for such hardware should provide their
@@ -216,34 +216,11 @@ EXPORT_SYMBOL(drm_primary_helper_update);
* disabled primary plane).
*
* RETURNS:
- * Zero on success, error code on failure
+ * Unconditionally returns -EINVAL.
*/
int drm_primary_helper_disable(struct drm_plane *plane)
{
- struct drm_plane *p;
- struct drm_mode_set set = {
- .crtc = plane-crtc,
- .fb = NULL,
- };
-
- if (plane-crtc == NULL || plane-fb == NULL)
- /* Already disabled */
- return 0;
-
- list_for_each_entry(p, plane-dev-mode_config.plane_list, head)
- if (p != plane p-fb) {
- DRM_DEBUG_KMS(Cannot disable primary plane while other
planes are still active on CRTC.\n);
- return -EBUSY;
- }
-
- /*
- * N.B. We call set_config() directly here rather than
- * drm_mode_set_config_internal() since drm_mode_setplane() already
- * handles the basic refcounting and we don't need the special
- * cross-CRTC refcounting (no chance of stealing connectors from
- * other CRTC's with this update).
- */
- return plane-crtc-funcs-set_config(set);
+ return -EINVAL;
}
EXPORT_SYMBOL(drm_primary_helper_disable);
--
1.9.2
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] tests/gem_error_capture: Initial testcase for error state capture/dump
Hi,
oscar.ma...@intel.com writes:
From: Oscar Mateo oscar.ma...@intel.com
Guarantees that error capture works at a very basic level.
v2: Also check that the ring object contains a reloc with MI_BB_START
for the presumed batch object's address.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
tests/.gitignore | 1 +
tests/Makefile.sources| 1 +
tests/gem_error_capture.c | 269
++
3 files changed, 271 insertions(+)
create mode 100644 tests/gem_error_capture.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab0..945574c 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -27,6 +27,7 @@ gem_ctx_create
gem_ctx_exec
gem_double_irq_loop
gem_dummy_reloc_loop
+gem_error_capture
gem_evict_alignment
gem_evict_everything
gem_exec_bad_domains
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index bf02a48..612beb6 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -24,6 +24,7 @@ TESTS_progs_M = \
gem_ctx_bad_exec \
gem_ctx_exec \
gem_dummy_reloc_loop \
+ gem_error_capture \
gem_evict_alignment \
gem_evict_everything \
gem_exec_bad_domains \
diff --git a/tests/gem_error_capture.c b/tests/gem_error_capture.c
new file mode 100644
index 000..88845c9
--- /dev/null
+++ b/tests/gem_error_capture.c
@@ -0,0 +1,269 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Oscar Mateo oscar.ma...@intel.com
+ *
+ */
+
+/*
+ * Testcase: Check whether basic error state capture/dump mechanism is
correctly
+ * working for all rings.
+ */
+
+#include unistd.h
+#include stdlib.h
+#include stdint.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include drm.h
+#include ioctl_wrappers.h
+#include drmtest.h
+#include intel_io.h
+#include igt_debugfs.h
+
+#define MAGIC_NUMBER 0x10001
+uint32_t batch[4] = {MI_NOOP, MI_BATCH_BUFFER_END, MAGIC_NUMBER,
MAGIC_NUMBER};
+
+static void stop_rings(void)
+{
+ int fd;
+ static const char buf[] = 0xf;
+
+ fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
+ igt_assert(fd = 0);
+
+ igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
+
+ close(fd);
+}
+
+static bool rings_stopped(void)
+{
+ int fd;
+ static char buf[128];
+ unsigned long long val;
+
+ fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
+ igt_assert(fd = 0);
+
+ igt_assert(read(fd, buf, sizeof(buf)) 0);
+ close(fd);
+
+ sscanf(buf, %llx, val);
+
+ return (bool)val;
+}
Please use igt_set_stop_rings() and igt_get_stop_rings().
Also consider stopping only the ring you are testing for.
+static void clear_error_state(void)
+{
+ int fd;
+ static const char buf[] = ;
+
+ fd = igt_debugfs_open(i915_error_state, O_WRONLY);
+ igt_assert(fd = 0);
+
+ igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
+ close(fd);
+}
+
+static void check_error_state(const char *expected_ring_name,
+ uint64_t expected_offset)
+{
+ FILE *file;
+ int debug_fd;
+ char *line = NULL;
+ size_t line_size = 0;
+ char *dashes = NULL;
+ char *ring_name = NULL;
+ int bb_matched = 0;
+ uint32_t gtt_offset;
+ char expected_line[32];
+ int req_matched = 0;
+ int requests;
+ uint32_t seqno, tail;
+ long jiffies;
+ int ringbuf_matched = 0;
+ unsigned int offset, command, expected_addr = 0;
+ int i, items;
+ bool bb_ok = false, req_ok = false, ringbuf_ok = false;
+
+ debug_fd = igt_debugfs_open(i915_error_state, O_RDONLY);
+ igt_assert(debug_fd = 0);
+ file
[Intel-gfx] The whole round of i-g-t testing cost too long running time
Hi all, I have discussed with Daniel about the running time for each cases before and we set the standard as 10M, if one can’t finish after running 10M we will see it as Timeout and report bug on FDO(such as : Bug 77474https://bugs.freedesktop.org/show_bug.cgi?id=77474 - [PNV/IVB/HSW]igt/gem_tiled_swapping is slow and Bug 77475https://bugs.freedesktop.org/show_bug.cgi?id=77475 - [PNV/IVB/HSW]igt//kms_pipe_crc_basic/read-crc-pipe-A is slow) Now the true status is that i-g-t have more than 650+ subcases, running a whole round of testing will cost such a long time on QA side(beside that Timeout cases), QA also need to spend more time to analysis the result changing on each platforms. You can find an example with this page: http://tinderbox.sh.intel.com/PRTS_UI/prtsresult.php?task_id=2778 for how long one testing round cost. With the table of subtask:10831 on the page which for i-g-t test cases on BDW. Testing start at 19:16 PM and finished at 03:25 AM the next day, cost about 8 hours to run 638 test cases. Each cases finished less than 10M as we expect, but the full time it too large, especially the BDW is the powerful machine on our side, ILK or PNV may take more than 10 hours. We not only run i-g-t but also need to test the piglit/performance/media which already need time. Do we have any solutions to reduce the running time for whole i-g-t? it’s a pressing problem for QA after seeing the i-g-t case count enhance from 50 -600+. Best Regards~~ Open Source Technology Center (OTC) Terence Yang（杨光） Tel: 86-021-61167360 iNet: 8821-7360 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 49/71] drm/i915/chv: Add CHV display support
On Wed, 2014-04-09 at 13:28 +0300, ville.syrj...@linux.intel.com wrote:
From: Rafael Barbalho rafael.barba...@intel.com
Add support for the third pipe in cherrview
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
[vsyrjala: slightly massaged the patch]
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
With the formatting fix from Jani:
Reviewed-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 7 +++
drivers/gpu/drm/i915/i915_reg.h | 11 ---
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2415fa2..c5e9fa8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,12 @@ static struct drm_driver driver;
.dpll_md_offsets = { DPLL_A_MD_OFFSET, DPLL_B_MD_OFFSET }, \
.palette_offsets = { PALETTE_A_OFFSET, PALETTE_B_OFFSET }
+#define GEN_CHV_PIPEOFFSETS \
+ .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, CHV_PIPE_C_OFFSET }, \
+ .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET,
CHV_TRANSCODER_C_OFFSET, }, \
+ .dpll_offsets = { DPLL_A_OFFSET, DPLL_B_OFFSET, CHV_DPLL_C_OFFSET }, \
+ .dpll_md_offsets = { DPLL_A_MD_OFFSET, DPLL_B_MD_OFFSET,
CHV_DPLL_C_MD_OFFSET }, \
+ .palette_offsets = { PALETTE_A_OFFSET, PALETTE_B_OFFSET,
CHV_PALETTE_C_OFFSET }
static const struct intel_device_info intel_i830_info = {
.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
@@ -286,6 +292,7 @@ static const struct intel_device_info
intel_cherryview_info = {
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.is_valleyview = 1,
.display_mmio_offset = VLV_DISPLAY_BASE,
+ GEN_CHV_PIPEOFFSETS,
};
/*
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7587752..3831d84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1430,6 +1430,7 @@ enum punit_power_well {
*/
#define DPLL_A_OFFSET 0x6014
#define DPLL_B_OFFSET 0x6018
+#define CHV_DPLL_C_OFFSET 0x6030
#define DPLL(pipe) (dev_priv-info.dpll_offsets[pipe] + \
dev_priv-info.display_mmio_offset)
@@ -1521,6 +1522,7 @@ enum punit_power_well {
#define DPLL_A_MD_OFFSET 0x601c /* 965+ only */
#define DPLL_B_MD_OFFSET 0x6020 /* 965+ only */
+#define CHV_DPLL_C_MD_OFFSET 0x603c
#define DPLL_MD(pipe) (dev_priv-info.dpll_md_offsets[pipe] + \
dev_priv-info.display_mmio_offset)
@@ -1717,6 +1719,7 @@ enum punit_power_well {
*/
#define PALETTE_A_OFFSET 0xa000
#define PALETTE_B_OFFSET 0xa800
+#define CHV_PALETTE_C_OFFSET 0xc000
#define PALETTE(pipe) (dev_priv-info.palette_offsets[pipe] + \
dev_priv-info.display_mmio_offset)
@@ -2206,6 +2209,7 @@ enum punit_power_well {
#define TRANSCODER_A_OFFSET 0x6
#define TRANSCODER_B_OFFSET 0x61000
#define TRANSCODER_C_OFFSET 0x62000
+#define CHV_TRANSCODER_C_OFFSET 0x63000
#define TRANSCODER_EDP_OFFSET 0x6f000
#define _TRANSCODER2(pipe, reg) (dev_priv-info.trans_offsets[(pipe)] - \
@@ -3533,9 +3537,10 @@ enum punit_power_well {
#define PIPESTAT_INT_ENABLE_MASK 0x7fff
#define PIPESTAT_INT_STATUS_MASK 0x
-#define PIPE_A_OFFSET0x7
-#define PIPE_B_OFFSET0x71000
-#define PIPE_C_OFFSET0x72000
+#define PIPE_A_OFFSET0x7
+#define PIPE_B_OFFSET0x71000
+#define PIPE_C_OFFSET0x72000
+#define CHV_PIPE_C_OFFSET0x74000
/*
* There's actually no pipe EDP. Some pipe registers have
* simply shifted from the pipe to the transcoder, while
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)
On Thu, Mar 27, 2014 at 05:59:48PM +, oscar.ma...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
For the most part, logical rinf context objects are similar to hardware
contexts in that the backing object is meant to be opaque. There are
some exceptions where we need to poke certain offsets of the object for
initialization, updating the tail pointer or updating the PDPs.
For our basic execlist implementation we'll only need our PPGTT PDs,
and ringbuffer addresses in order to set up the context. With previous
patches, we have both, so start prepping the context to be load.
Before running a context for the first time you must populate some
fields in the context object. These fields begin 1 PAGE + LRCA, ie. the
first page (in 0 based counting) of the context image. These same
fields will be read and written to as contexts are saved and restored
once the system is up and running.
Many of these fields are completely reused from previous global
registers: ringbuffer head/tail/control, context control matches some
previous MI_SET_CONTEXT flags, and page directories. There are other
fields which we don't touch which we may want in the future.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and (11)
for other engines.
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
v3: Several rebases and general changes to the code.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/i915_lrc.c | 145
++--
1 file changed, 138 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_lrc.c b/drivers/gpu/drm/i915/i915_lrc.c
index 40dfa95..f0176ff 100644
--- a/drivers/gpu/drm/i915/i915_lrc.c
+++ b/drivers/gpu/drm/i915/i915_lrc.c
@@ -43,6 +43,38 @@
#define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
+#define RING_ELSP(ring) ((ring)-mmio_base+0x230)
+#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
+
+#define CTX_LRI_HEADER_0 0x01
+#define CTX_CONTEXT_CONTROL 0x02
+#define CTX_RING_HEAD0x04
+#define CTX_RING_TAIL0x06
+#define CTX_RING_BUFFER_START0x08
+#define CTX_RING_BUFFER_CONTROL 0x0a
+#define CTX_BB_HEAD_U0x0c
+#define CTX_BB_HEAD_L0x0e
+#define CTX_BB_STATE 0x10
+#define CTX_SECOND_BB_HEAD_U 0x12
+#define CTX_SECOND_BB_HEAD_L 0x14
+#define CTX_SECOND_BB_STATE 0x16
+#define CTX_BB_PER_CTX_PTR 0x18
+#define CTX_RCS_INDIRECT_CTX 0x1a
+#define CTX_RCS_INDIRECT_CTX_OFFSET 0x1c
+#define CTX_LRI_HEADER_1 0x21
+#define CTX_CTX_TIMESTAMP0x22
+#define CTX_PDP3_UDW 0x24
+#define CTX_PDP3_LDW 0x26
+#define CTX_PDP2_UDW 0x28
+#define CTX_PDP2_LDW 0x2a
+#define CTX_PDP1_UDW 0x2c
+#define CTX_PDP1_LDW 0x2e
+#define CTX_PDP0_UDW 0x30
+#define CTX_PDP0_LDW 0x32
+#define CTX_LRI_HEADER_2 0x41
+#define CTX_R_PWR_CLK_STATE 0x42
+#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
+
struct i915_hw_context *
gen8_gem_create_context(struct drm_device *dev,
struct intel_engine *ring,
@@ -51,6 +83,9 @@ gen8_gem_create_context(struct drm_device *dev,
{
struct i915_hw_context *ctx = NULL;
struct drm_i915_gem_object *ring_obj = NULL;
+ struct i915_hw_ppgtt *ppgtt = NULL;
+ struct page *page;
+ uint32_t *reg_state;
int ret;
ctx = i915_gem_create_context(dev, file_priv, create_vm);
@@ -79,18 +114,114 @@ gen8_gem_create_context(struct drm_device *dev,
/* Failure at this point is almost impossible */
ret = i915_gem_object_set_to_gtt_domain(ring_obj, true);
- if (ret) {
- i915_gem_object_ggtt_unpin(ring_obj);
- drm_gem_object_unreference(ring_obj-base);
- i915_gem_object_ggtt_unpin(ctx-obj);
- i915_gem_context_unreference(ctx);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto destroy_ring_obj;
ctx-ringbuf = ring-default_ringbuf;
ctx-ringbuf-obj = ring_obj;
+ ppgtt = ctx_to_ppgtt(ctx);
+
+ ret = i915_gem_object_set_to_cpu_domain(ctx-obj, true);
+ if (ret)
+ goto destroy_ring_obj;
+
+ ret = i915_gem_object_get_pages(ctx-obj);
+ if (ret)
+ goto destroy_ring_obj;
+
+ i915_gem_object_pin_pages(ctx-obj);
+
+ /* The second page of the context object contains some fields which must
+ * be set up prior to the first execution.
+ */
+ page = i915_gem_object_get_page(ctx-obj, 1);
+ reg_state = kmap_atomic(page);
+
+ if (ring-id == RCS)
+
Re: [Intel-gfx] [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)
On Tue, Apr 15, 2014 at 11:00:33AM -0500, Jeff McGee wrote:
On Thu, Mar 27, 2014 at 05:59:48PM +, oscar.ma...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
For the most part, logical rinf context objects are similar to hardware
contexts in that the backing object is meant to be opaque. There are
some exceptions where we need to poke certain offsets of the object for
initialization, updating the tail pointer or updating the PDPs.
For our basic execlist implementation we'll only need our PPGTT PDs,
and ringbuffer addresses in order to set up the context. With previous
patches, we have both, so start prepping the context to be load.
Before running a context for the first time you must populate some
fields in the context object. These fields begin 1 PAGE + LRCA, ie. the
first page (in 0 based counting) of the context image. These same
fields will be read and written to as contexts are saved and restored
once the system is up and running.
Many of these fields are completely reused from previous global
registers: ringbuffer head/tail/control, context control matches some
previous MI_SET_CONTEXT flags, and page directories. There are other
fields which we don't touch which we may want in the future.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and (11)
for other engines.
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
v3: Several rebases and general changes to the code.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/i915_lrc.c | 145
++--
1 file changed, 138 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_lrc.c
b/drivers/gpu/drm/i915/i915_lrc.c
index 40dfa95..f0176ff 100644
--- a/drivers/gpu/drm/i915/i915_lrc.c
+++ b/drivers/gpu/drm/i915/i915_lrc.c
@@ -43,6 +43,38 @@
#define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
+#define RING_ELSP(ring)((ring)-mmio_base+0x230)
+#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
+
+#define CTX_LRI_HEADER_0 0x01
+#define CTX_CONTEXT_CONTROL0x02
+#define CTX_RING_HEAD 0x04
+#define CTX_RING_TAIL 0x06
+#define CTX_RING_BUFFER_START 0x08
+#define CTX_RING_BUFFER_CONTROL0x0a
+#define CTX_BB_HEAD_U 0x0c
+#define CTX_BB_HEAD_L 0x0e
+#define CTX_BB_STATE 0x10
+#define CTX_SECOND_BB_HEAD_U 0x12
+#define CTX_SECOND_BB_HEAD_L 0x14
+#define CTX_SECOND_BB_STATE0x16
+#define CTX_BB_PER_CTX_PTR 0x18
+#define CTX_RCS_INDIRECT_CTX 0x1a
+#define CTX_RCS_INDIRECT_CTX_OFFSET0x1c
+#define CTX_LRI_HEADER_1 0x21
+#define CTX_CTX_TIMESTAMP 0x22
+#define CTX_PDP3_UDW 0x24
+#define CTX_PDP3_LDW 0x26
+#define CTX_PDP2_UDW 0x28
+#define CTX_PDP2_LDW 0x2a
+#define CTX_PDP1_UDW 0x2c
+#define CTX_PDP1_LDW 0x2e
+#define CTX_PDP0_UDW 0x30
+#define CTX_PDP0_LDW 0x32
+#define CTX_LRI_HEADER_2 0x41
+#define CTX_R_PWR_CLK_STATE0x42
+#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
+
struct i915_hw_context *
gen8_gem_create_context(struct drm_device *dev,
struct intel_engine *ring,
@@ -51,6 +83,9 @@ gen8_gem_create_context(struct drm_device *dev,
{
struct i915_hw_context *ctx = NULL;
struct drm_i915_gem_object *ring_obj = NULL;
+ struct i915_hw_ppgtt *ppgtt = NULL;
+ struct page *page;
+ uint32_t *reg_state;
int ret;
ctx = i915_gem_create_context(dev, file_priv, create_vm);
@@ -79,18 +114,114 @@ gen8_gem_create_context(struct drm_device *dev,
/* Failure at this point is almost impossible */
ret = i915_gem_object_set_to_gtt_domain(ring_obj, true);
- if (ret) {
- i915_gem_object_ggtt_unpin(ring_obj);
- drm_gem_object_unreference(ring_obj-base);
- i915_gem_object_ggtt_unpin(ctx-obj);
- i915_gem_context_unreference(ctx);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto destroy_ring_obj;
ctx-ringbuf = ring-default_ringbuf;
ctx-ringbuf-obj = ring_obj;
+ ppgtt = ctx_to_ppgtt(ctx);
+
+ ret = i915_gem_object_set_to_cpu_domain(ctx-obj, true);
+ if (ret)
+ goto destroy_ring_obj;
+
+ ret = i915_gem_object_get_pages(ctx-obj);
+ if (ret)
+ goto destroy_ring_obj;
+
+ i915_gem_object_pin_pages(ctx-obj);
+
+ /* The second page of the context object contains some fields which must
+
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
On Tue, Apr 15, 2014 at 09:19:27PM +0530, S, Deepak wrote: On 4/10/2014 7:34 PM, Ville Syrjälä wrote: On Thu, Apr 10, 2014 at 04:41:10PM +0300, Jani Nikula wrote: On Thu, 10 Apr 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Thu, Apr 10, 2014 at 09:31:39AM +0530, S, Deepak wrote: On 4/10/2014 1:30 AM, Daniel Vetter wrote: On Thu, Apr 10, 2014 at 12:42:42AM +0530, S, Deepak wrote: On 4/9/2014 10:23 PM, Daniel Vetter wrote: On Wed, Apr 09, 2014 at 06:05:27PM +0300, Ville Syrjälä wrote: On Wed, Apr 09, 2014 at 02:30:52PM +, S, Deepak wrote: Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Well you can consider it pushed now that it's in the open. The patches just need a bit of extra polish I think. Well, unless you're planning a full blown rewrite of the code ;) I guess you need to take into consideration whatever bdw rc6/rps patches are still in flight, but since you've been doing some review there I think you have a better idea than I do how things are progressing. I always work on top of nightly, so I guess that's a good choice :) Yes, -nightly is always the recommended branch to base upstream patches on. I'll sort out the conflict mess (or well, try to) if it doesn't apply to plain dinq or some other branch. drm-intel-next tends to be too outdated ;-) -Daniel Hi Daniel/Ville. Some of the patches are lined up for squashing right? So you want me to work on this patches to align to upstream code and resubmit it to same email thread? Hm, I expect this chv thread to become a bit mess really quickly tbh ;-) And since we don't have chv merged yet there's not really a baseline to do this on top. I guess the simplest approach would be for you to grab ville's chv tree, squash in the patches as discussed and then just starting on polishing your chv patches. Then as I pull in patches from this series you can drop them from yours. A bit messy, but I don't see any other approach really. Note that a pile of people are signed up to review this, so maybe hold off a bit until the review for your patches have been done. -Daniel Thanks Daniel. Ville can you please share your chv tree details? I rebased the lot and pushed here: git://gitorious.org/vsyrjala/linux.git chv_rebase /me being lazy, did you squash/reorder patches, i.e. do the patch # assignments [1] for review still apply? The numbers would get shifted around a bit due to two these two patches already getting merged: drm/i915/chv: IS_BROADWELL() should not be true for Cherryview drm/i915/chv: Add IS_CHERRYVIEW() macro And this patch got dropped as it no longer applies: drm/i915/chv: Add plane C support Apart from that no reordering/squashing. Jani. [1] http://mid.gmane.org/20140410110857.gw18...@intel.com -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center Hi Ville, Have you already squashed some of the RC6/turbo patches? Or you want me to do it as part of RC6/RPS rework patches submission. No, I didn't do it. If you can take care of it that'd be great. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Don't vblank wait on ilk-ivb after pipe enable
Like on hsw/bdw the pipe isn't actually running yet at this point. This holds for both pch ports and the cpu edp port according to my testing on ilk, snb and ivb. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77297 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1aae7361b7a5..e0310e3018ee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1827,16 +1827,6 @@ static void intel_enable_pipe(struct intel_crtc *crtc) I915_WRITE(reg, val | PIPECONF_ENABLE); POSTING_READ(reg); - - /* -* There's no guarantee the pipe will really start running now. It -* depends on the Gen, the output type and the relative order between -* pipe and plane enabling. Avoid waiting on HSW+ since it's not -* necessary. -* TODO: audit the previous gens. -*/ - if (INTEL_INFO(dev)-gen = 7 !IS_HASWELL(dev)) - intel_wait_for_vblank(dev_priv-dev, pipe); } /** @@ -4461,7 +4451,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); + intel_wait_for_vblank(dev_priv-dev, pipe); intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + intel_enable_primary_hw_plane(dev_priv, plane, pipe); intel_enable_planes(crtc); intel_crtc_update_cursor(crtc, true); @@ -4546,7 +4538,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); + intel_wait_for_vblank(dev_priv-dev, pipe); intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + intel_enable_primary_hw_plane(dev_priv, plane, pipe); intel_enable_planes(crtc); /* The fixup needs to happen before cursor is enabled */ -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Move plane enabling to the end of ilk_crtc_enable
Like on hsw/bdw the pipe only starts running once the port/pch transcoder combo is all enabled. Before that the vblank wait in the primary plane enable function simply times out. This is also really nice prep work for atomic modesets since now all the plane enabling is at the very end and all tightly grouped together, like on hsw+. Which means we can enable it all atomically with a nuclear pageflip. vlv is still different and the watermark code is also still somewhere else, so it's not yet quite perfect. But we're getting there. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77297 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e0310e3018ee..e6555c0dedca 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3714,9 +3714,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_enable_primary_hw_plane(dev_priv, plane, pipe); - intel_enable_planes(crtc); - intel_crtc_update_cursor(crtc, true); if (intel_crtc-config.has_pch_encoder) ironlake_pch_enable(crtc); @@ -3742,6 +3739,10 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) */ intel_wait_for_vblank(dev, intel_crtc-pipe); + intel_enable_primary_hw_plane(dev_priv, plane, pipe); + intel_enable_planes(crtc); + intel_crtc_update_cursor(crtc, true); + drm_vblank_on(dev, pipe); } -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] The whole round of i-g-t testing cost too long running time
Chated with Ben last week about this It may be feasiable to have a fast.tests for intel-gpu-tools also Thanks --Shuang From: Yang, Guang A Sent: Tuesday, April 15, 2014 11:46 PM To: Vetter, Daniel; Barnes, Jesse; Widawsky, Benjamin; Wood, Thomas; Jin, Gordon; OTC GFX QA Extended; intel-gfx@lists.freedesktop.org Subject: The whole round of i-g-t testing cost too long running time Hi all, I have discussed with Daniel about the running time for each cases before and we set the standard as 10M, if one can’t finish after running 10M we will see it as Timeout and report bug on FDO(such as : Bug 77474https://bugs.freedesktop.org/show_bug.cgi?id=77474 - [PNV/IVB/HSW]igt/gem_tiled_swapping is slow and Bug 77475https://bugs.freedesktop.org/show_bug.cgi?id=77475 - [PNV/IVB/HSW]igt//kms_pipe_crc_basic/read-crc-pipe-A is slow) Now the true status is that i-g-t have more than 650+ subcases, running a whole round of testing will cost such a long time on QA side(beside that Timeout cases), QA also need to spend more time to analysis the result changing on each platforms. You can find an example with this page: http://tinderbox.sh.intel.com/PRTS_UI/prtsresult.php?task_id=2778 for how long one testing round cost. With the table of subtask:10831 on the page which for i-g-t test cases on BDW. Testing start at 19:16 PM and finished at 03:25 AM the next day, cost about 8 hours to run 638 test cases. Each cases finished less than 10M as we expect, but the full time it too large, especially the BDW is the powerful machine on our side, ILK or PNV may take more than 10 hours. We not only run i-g-t but also need to test the piglit/performance/media which already need time. Do we have any solutions to reduce the running time for whole i-g-t? it’s a pressing problem for QA after seeing the i-g-t case count enhance from 50 -600+. Best Regards~~ Open Source Technology Center (OTC) Terence Yang（杨光） Tel: 86-021-61167360 iNet: 8821-7360 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
On 4/15/2014 9:46 PM, Ville Syrjälä wrote: On Tue, Apr 15, 2014 at 09:19:27PM +0530, S, Deepak wrote: On 4/10/2014 7:34 PM, Ville Syrjälä wrote: On Thu, Apr 10, 2014 at 04:41:10PM +0300, Jani Nikula wrote: On Thu, 10 Apr 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Thu, Apr 10, 2014 at 09:31:39AM +0530, S, Deepak wrote: On 4/10/2014 1:30 AM, Daniel Vetter wrote: On Thu, Apr 10, 2014 at 12:42:42AM +0530, S, Deepak wrote: On 4/9/2014 10:23 PM, Daniel Vetter wrote: On Wed, Apr 09, 2014 at 06:05:27PM +0300, Ville Syrjälä wrote: On Wed, Apr 09, 2014 at 02:30:52PM +, S, Deepak wrote: Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Well you can consider it pushed now that it's in the open. The patches just need a bit of extra polish I think. Well, unless you're planning a full blown rewrite of the code ;) I guess you need to take into consideration whatever bdw rc6/rps patches are still in flight, but since you've been doing some review there I think you have a better idea than I do how things are progressing. I always work on top of nightly, so I guess that's a good choice :) Yes, -nightly is always the recommended branch to base upstream patches on. I'll sort out the conflict mess (or well, try to) if it doesn't apply to plain dinq or some other branch. drm-intel-next tends to be too outdated ;-) -Daniel Hi Daniel/Ville. Some of the patches are lined up for squashing right? So you want me to work on this patches to align to upstream code and resubmit it to same email thread? Hm, I expect this chv thread to become a bit mess really quickly tbh ;-) And since we don't have chv merged yet there's not really a baseline to do this on top. I guess the simplest approach would be for you to grab ville's chv tree, squash in the patches as discussed and then just starting on polishing your chv patches. Then as I pull in patches from this series you can drop them from yours. A bit messy, but I don't see any other approach really. Note that a pile of people are signed up to review this, so maybe hold off a bit until the review for your patches have been done. -Daniel Thanks Daniel. Ville can you please share your chv tree details? I rebased the lot and pushed here: git://gitorious.org/vsyrjala/linux.git chv_rebase /me being lazy, did you squash/reorder patches, i.e. do the patch # assignments [1] for review still apply? The numbers would get shifted around a bit due to two these two patches already getting merged: drm/i915/chv: IS_BROADWELL() should not be true for Cherryview drm/i915/chv: Add IS_CHERRYVIEW() macro And this patch got dropped as it no longer applies: drm/i915/chv: Add plane C support Apart from that no reordering/squashing. Jani. [1] http://mid.gmane.org/20140410110857.gw18...@intel.com -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center Hi Ville, Have you already squashed some of the RC6/turbo patches? Or you want me to do it as part of RC6/RPS rework patches submission. No, I didn't do it. If you can take care of it that'd be great. Ok, I will take care of squashing the patch. I will submit all rc6/rps patches after cleanup. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] The whole round of i-g-t testing cost too long running time
On 15/04/2014 17:46, Yang, Guang A wrote: Hi all, I have discussed with Daniel about the running time for each cases before and we set the standard as 10M, if one can’t finish after running 10M we will see it as Timeout and report bug on FDO(such as : Bug 77474 https://bugs.freedesktop.org/show_bug.cgi?id=77474 - [PNV/IVB/HSW]igt/gem_tiled_swapping is slow and Bug 77475 https://bugs.freedesktop.org/show_bug.cgi?id=77475 - [PNV/IVB/HSW]igt//kms_pipe_crc_basic/read-crc-pipe-A is slow) Now the true status is that i-g-t have more than 650+ subcases, running a whole round of testing will cost such a long time on QA side(*beside that Timeout cases*), QA also need to spend more time to analysis the result changing on each platforms. You can find an example with this page:http://tinderbox.sh.intel.com/PRTS_UI/prtsresult.php?task_id=2778 for how long one testing round cost. With the table of subtask:10831 on the page which for i-g-t test cases on BDW. Testing start at 19:16 PM and finished at 03:25 AM the next day, cost about *8 hours* to run 638 test cases. Each cases finished less than 10M as we expect, but the full time it too large, especially the BDW is the powerful machine on our side, ILK or PNV may take more than *10 hours*. We not only run i-g-t but also need to test the piglit/performance/media which already need time. Do we have any solutions to reduce the running time for whole i-g-t? it’s a pressing problem for QA after seeing the i-g-t case count enhance from 50 -600+. Ok there are a few cases where we can indeed make tests faster, but it will be work for us. And that won't really speed up much since we're adding piles more testcases at a pretty quick rate. And many of these new testcases are CRC based, so inheritely take some time to run. So I think longer-term we simply need to throw more machines at the problem and run testcases in parallel on identical machines. Wrt analyzing issues I think the right approach for moving forward is: a) switch to piglit to run tests, not just enumerate them. This will allow QA and developers to share testcase analysis. b) add automated analysis for time-consuming and error prone cases like dmesg warnings and backtraces. ThomasI have just discussed a few ideas in this are in our 1:1 today. Reducing the set of igt tests we run is imo pointless: The goal of igt is to hit corner-cases, arbitrarily selecting which kinds of corner-cases we test just means that we have a nice illusion about our test coverage. Adding more people to the discussion. Cheers, Daniel Intel Semiconductor AG Registered No. 020.30.913.786-7 Registered Office: Badenerstrasse 549, 8048 Zurich, Switzerland ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: Move buffer pinning and ring selection to intel_crtc_page_flip()
From: Ville Syrjälä ville.syrj...@linux.intel.com
All of the .queue_flip() callbacks duplicate the same code to pin the
buffers and calculate the gtt_offset. Move that code to
intel_crtc_page_flip(). In order to do that we must also move the ring
selection logic there.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 115 +++
2 files changed, 35 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7d6acb4..5efc435 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -462,6 +462,7 @@ struct drm_i915_display_funcs {
int (*queue_flip)(struct drm_device *dev, struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_i915_gem_object *obj,
+ struct intel_ring_buffer *ring,
uint32_t flags);
int (*update_primary_plane)(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c0fe5dd..e844a6b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8637,24 +8637,16 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_i915_gem_object *obj,
+struct intel_ring_buffer *ring,
uint32_t flags)
{
- struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
u32 flip_mask;
- struct intel_ring_buffer *ring = dev_priv-ring[RCS];
int ret;
- ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
- if (ret)
- goto err;
-
- intel_crtc-unpin_work-gtt_offset =
- i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
-
ret = intel_ring_begin(ring, 6);
if (ret)
- goto err_unpin;
+ return ret;
/* Can't queue multiple flips, so wait for the previous
* one to finish before executing the next.
@@ -8674,35 +8666,22 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
intel_mark_page_flip_active(intel_crtc);
__intel_ring_advance(ring);
return 0;
-
-err_unpin:
- intel_unpin_fb_obj(obj);
-err:
- return ret;
}
static int intel_gen3_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_i915_gem_object *obj,
+struct intel_ring_buffer *ring,
uint32_t flags)
{
- struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
u32 flip_mask;
- struct intel_ring_buffer *ring = dev_priv-ring[RCS];
int ret;
- ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
- if (ret)
- goto err;
-
- intel_crtc-unpin_work-gtt_offset =
- i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
-
ret = intel_ring_begin(ring, 6);
if (ret)
- goto err_unpin;
+ return ret;
if (intel_crtc-plane)
flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
@@ -8719,35 +8698,23 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
intel_mark_page_flip_active(intel_crtc);
__intel_ring_advance(ring);
return 0;
-
-err_unpin:
- intel_unpin_fb_obj(obj);
-err:
- return ret;
}
static int intel_gen4_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_i915_gem_object *obj,
+struct intel_ring_buffer *ring,
uint32_t flags)
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
uint32_t pf, pipesrc;
- struct intel_ring_buffer *ring = dev_priv-ring[RCS];
int ret;
- ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
- if (ret)
- goto err;
-
- intel_crtc-unpin_work-gtt_offset =
- i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
-
ret = intel_ring_begin(ring, 4);
if (ret)
- goto err_unpin;
+ return ret;
/* i965+ uses the linear or tiled offsets from the
* Display Registers (which do
[Intel-gfx] [PATCH 3/5] drm/i915: Drop the excessive vblank waits from modeset codepaths
From: Ville Syrjälä ville.syrj...@linux.intel.com Now that we've plugged the mmio vs. ring flip race, we shouldn't need these vblank waits in the modeset codepaths anymore. So get rid of them. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 12 1 file changed, 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 33d21bf..9b181fc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1901,7 +1901,6 @@ static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv, I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE); intel_flush_primary_plane(dev_priv, plane); - intel_wait_for_vblank(dev_priv-dev, pipe); } /** @@ -1931,7 +1930,6 @@ static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv, I915_WRITE(reg, val ~DISPLAY_PLANE_ENABLE); intel_flush_primary_plane(dev_priv, plane); - intel_wait_for_vblank(dev_priv-dev, pipe); } static bool need_vtd_wa(struct drm_device *dev) @@ -3706,16 +3704,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) if (HAS_PCH_CPT(dev)) cpt_verify_modeset(dev, intel_crtc-pipe); - /* -* There seems to be a race in PCH platform hw (at least on some -* outputs) where an enabled pipe still completes any pageflip right -* away (as if the pipe is off) instead of waiting for vblank. As soon -* as the first vblank happend, everything works as expected. Hence just -* wait for one vblank before returning to avoid strange things -* happening. -*/ - intel_wait_for_vblank(dev, intel_crtc-pipe); - drm_vblank_on(dev, pipe); } -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt] tests/kms_mmio_vs_cs_flip: Add a test case to exercise mmio vs. CS flip races
From: Ville Syrjälä ville.syrj...@linux.intel.com
kms_mmio_vs_cs_flip has two subtests:
- setplane_vs_cs_flip tests the interaction between
fullscreen sprites and CS flips
- setcrtc_vs_cs_flip tests the interaction between
primary plane panning and CS flips
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
tests/Makefile.sources | 1 +
tests/kms_mmio_vs_cs_flip.c | 519
2 files changed, 520 insertions(+)
create mode 100644 tests/kms_mmio_vs_cs_flip.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace..6ff0a35 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -60,6 +60,7 @@ TESTS_progs_M = \
kms_fbc_crc \
kms_flip \
kms_flip_tiling \
+ kms_mmio_vs_cs_flip \
kms_pipe_crc_basic \
kms_plane \
kms_render \
diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
new file mode 100644
index 000..9d9b02a
--- /dev/null
+++ b/tests/kms_mmio_vs_cs_flip.c
@@ -0,0 +1,519 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include errno.h
+#include stdbool.h
+#include stdio.h
+#include string.h
+#include time.h
+
+#include drmtest.h
+#include igt_debugfs.h
+#include igt_kms.h
+#include intel_chipset.h
+#include ioctl_wrappers.h
+
+typedef struct {
+ int drm_fd;
+ igt_display_t display;
+ igt_pipe_crc_t *pipe_crc;
+ drm_intel_bufmgr *bufmgr;
+ drm_intel_bo *busy_bo;
+ uint32_t devid;
+ bool flip_done;
+} data_t;
+
+static void exec_nop(data_t *data, uint32_t handle, unsigned int ring)
+{
+ struct intel_batchbuffer *batch;
+ drm_intel_bo *bo;
+
+ batch = intel_batchbuffer_alloc(data-bufmgr, data-devid);
+ igt_assert(batch);
+
+ bo = gem_handle_to_libdrm_bo(data-bufmgr, data-drm_fd, , handle);
+ igt_assert(bo);
+
+ /* add relocs to make sure the kernel will think we write to dst */
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_BATCH_BUFFER_END);
+ OUT_BATCH(MI_NOOP);
+ OUT_RELOC(bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+
+ intel_batchbuffer_flush_on_ring(batch, ring);
+ intel_batchbuffer_free(batch);
+}
+
+static void exec_blt(data_t *data)
+{
+ struct intel_batchbuffer *batch;
+ int w, h, pitch, i;
+
+ batch = intel_batchbuffer_alloc(data-bufmgr, data-devid);
+ igt_assert(batch);
+
+ w = 8192;
+ h = data-busy_bo-size / (8192 * 4);
+ pitch = w * 4;
+
+ for (i = 0; i 20; i++) {
+ BLIT_COPY_BATCH_START(data-devid, 0);
+ OUT_BATCH((3 24) | /* 32 bits */
+ (0xcc 16) | /* copy ROP */
+ pitch);
+ OUT_BATCH(0 16 | 0);
+ OUT_BATCH(h 16 | w);
+ OUT_RELOC(data-busy_bo, I915_GEM_DOMAIN_RENDER,
I915_GEM_DOMAIN_RENDER, 0);
+ BLIT_RELOC_UDW(data-devid);
+ OUT_BATCH(0 16 | 0);
+ OUT_BATCH(pitch);
+ OUT_RELOC(data-busy_bo, I915_GEM_DOMAIN_RENDER, 0, 0);
+ BLIT_RELOC_UDW(data-devid);
+ ADVANCE_BATCH();
+ }
+
+ intel_batchbuffer_flush(batch);
+ intel_batchbuffer_free(batch);
+}
+
+static void page_flip_handler(int fd, unsigned int frame, unsigned int sec,
+ unsigned int usec, void *_data)
+{
+ data_t *data = _data;
+
+ data-flip_done = true;
+}
+
+static void wait_for_flip(data_t *data, uint32_t flip_handle)
+{
+ struct timeval timeout = {
+ .tv_sec = 3,
+ .tv_usec = 0,
+ };
+ drmEventContext evctx = {
+ .version = DRM_EVENT_CONTEXT_VERSION,
+ .page_flip_handler = page_flip_handler,
+ };
+
[Intel-gfx] [PATCH v2 1/5] drm/i915: Fix mmio vs. CS flip race on ILK+
From: Ville Syrjälä ville.syrj...@linux.intel.com
Starting from ILK, mmio flips also cause a flip done interrupt to be
signalled. This means if we first do a set_base and follow it
immediately with the CS flip, we might mistake the flip done interrupt
caused by the set_base as the flip done interrupt caused by the CS
flip.
The hardware has a flip counter which increments every time a mmio or
CS flip is issued. It basically counts the number of DSPSURF register
writes. This means we can sample the counter before we put the CS
flip into the ring, and then when we get a flip done interrupt we can
check whether the CS flip has actually performed the surface address
update, or if the interrupt was caused by a previous but yet
unfinished mmio flip.
Even with the flip counter we still have a race condition of the CS flip
base address update happens after the mmio flip done interrupt was
raised but not yet processed by the driver. When the interrupt is
eventually processed, the flip counter will already indicate that the
CS flip has been executed, but it would not actually complete until the
next start of vblank. We can use the DSPSURFLIVE register to check
whether the hardware is actually scanning out of the buffer we expect,
or if we managed hit this race window.
This covers all the cases where the CS flip actually changes the base
address. If the base address remains unchanged, we might still complete
the CS flip before it has actually completed. But since the address
didn't change anyway, the premature flip completion can't result in
userspace overwriting data that's still being scanned out.
CTG already has the flip counter and DSPSURFLIVE registers, and
although the flip done interrupt is still limited to CS flips alone,
the code now also checks the flip counter on CTG as well.
v2: s/dspsurf/gtt_offset/ (Chris)
Testcase: igt/kms_mmio_vs_cs_flip/setcrtc_vs_cs_flip
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73027
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 73
drivers/gpu/drm/i915/intel_drv.h | 2 +
3 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f84555..c28169c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3638,6 +3638,7 @@ enum punit_power_well {
#define _PIPEA_FRMCOUNT_GM45 0x70040
#define _PIPEA_FLIPCOUNT_GM45 0x70044
#define PIPE_FRMCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FRMCOUNT_GM45)
+#define PIPE_FLIPCOUNT_GM45(pipe) _PIPE2(pipe, _PIPEA_FLIPCOUNT_GM45)
/* Cursor A B regs */
#define _CURACNTR (dev_priv-info.display_mmio_offset + 0x70080)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5..32c6c16 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8577,6 +8577,48 @@ void intel_finish_page_flip_plane(struct drm_device
*dev, int plane)
do_intel_finish_page_flip(dev, crtc);
}
+/* Is 'a' after or equal to 'b'? */
+static bool flip_count_after_eq(u32 a, u32 b)
+{
+ return !((a - b) 0x8000);
+}
+
+static bool page_flip_finished(struct intel_crtc *crtc)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ /*
+* The relevant registers doen't exist on pre-ctg.
+* As the flip done interrupt doesn't trigger for mmio
+* flips on gmch platforms, a flip count check isn't
+* really needed there. But since ctg has the registers,
+* include it in the check anyway.
+*/
+ if (INTEL_INFO(dev)-gen 5 !IS_G4X(dev))
+ return true;
+
+ /*
+* A DSPSURFLIVE check isn't enough in case the mmio and CS flips
+* used the same base address. In that case the mmio flip might
+* have completed, but the CS hasn't even executed the flip yet.
+*
+* A flip count check isn't enough as the CS might have updated
+* the base address just after start of vblank, but before we
+* managed to process the interrupt. This means we'd complete the
+* CS flip too soon.
+*
+* Combining both checks should get us a good enough result. It may
+* still happen that the CS flip has been executed, but has not
+* yet actually completed. But in case the base address is the same
+* anyway, we don't really care.
+*/
+ return (I915_READ(DSPSURFLIVE(crtc-plane)) ~0xfff) ==
+ crtc-unpin_work-gtt_offset
+ flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_GM45(crtc-pipe)),
+ crtc-unpin_work-flip_count);
+}
+
void intel_prepare_page_flip(struct drm_device *dev, int plane)
{
struct drm_i915_private *dev_priv =
[Intel-gfx] [PATCH 2/5] drm/i915: Wait for vblank in hsw_enable_ips()
From: Ville Syrjälä ville.syrj...@linux.intel.com
Now that the vblank wait is gone from intel_enable_primary_plane(),
hsw_enable_ips() needs to do the vblank wait itself.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 12 ++--
drivers/gpu/drm/i915/intel_sprite.c | 5 +
2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 32c6c16..33d21bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3543,17 +3543,17 @@ static void intel_disable_planes(struct drm_crtc *crtc)
void hsw_enable_ips(struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = crtc-base.dev-dev_private;
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
if (!crtc-config.ips_enabled)
return;
- /* We can only enable IPS after we enable a plane and wait for a vblank.
-* We guarantee that the plane is enabled by calling intel_enable_ips
-* only after intel_enable_plane. And intel_enable_plane already waits
-* for a vblank, so all we need to do here is to enable the IPS bit. */
+ /* We can only enable IPS after we enable a plane and wait for a vblank
*/
+ intel_wait_for_vblank(dev, crtc-pipe);
+
assert_plane_enabled(dev_priv, crtc-plane);
- if (IS_BROADWELL(crtc-base.dev)) {
+ if (IS_BROADWELL(dev)) {
mutex_lock(dev_priv-rps.hw_lock);
WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
0xc000));
mutex_unlock(dev_priv-rps.hw_lock);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..4df7245 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -540,10 +540,7 @@ intel_enable_primary(struct drm_crtc *crtc)
* when going from primary only to sprite only and vice
* versa.
*/
- if (intel_crtc-config.ips_enabled) {
- intel_wait_for_vblank(dev, intel_crtc-pipe);
- hsw_enable_ips(intel_crtc);
- }
+ hsw_enable_ips(intel_crtc);
mutex_lock(dev-struct_mutex);
intel_update_fbc(dev);
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/5] drm/i915: mmio vs. CS flip race fix
From: Ville Syrjälä ville.syrj...@linux.intel.com This is the second version of the mmio vs. CS flip race fix. Since v1 I fixed one if Chris's complaints. I still left the flip_count_after_eq() untouched so that it's reasonably consistent with the vbl_count_after_eq() I have in my watermark series. The IPS vs. vblank wait removal patches got reordered. And I realized we have another race with the primary enable/disable logic in the sprite code, so I added a fix for that as well. And finally I tried to kill off some code duplication in the .queue_flip() functions. I also wrote a few testcases that exercise these bugs. Ville Syrjälä (5): drm/i915: Fix mmio vs. CS flip race on ILK+ drm/i915: Wait for vblank in hsw_enable_ips() drm/i915: Drop the excessive vblank waits from modeset codepaths drm/i915: Wait for pending page flips before enabling/disabling the primary plane drm/i915: Move buffer pinning and ring selection to intel_crtc_page_flip() drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 184 +-- drivers/gpu/drm/i915/intel_drv.h | 3 + drivers/gpu/drm/i915/intel_sprite.c | 9 +- 5 files changed, 102 insertions(+), 96 deletions(-) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915: Wait for pending page flips before enabling/disabling the primary plane
From: Ville Syrjälä ville.syrj...@linux.intel.com
We have to write to the primary plane base address registrer when we
enable/disable the primary plane in response to sprite coverage. Those
writes will cause the flip counter to increment which could interfere
with the detection of CS flip completion. We could end up completing
CS flips before the CS has even executed the commands from the ring.
To avoid such issues, wait for CS flips to finish before we toggle the
primary plane on/off.
Testcase: igt/kms_mmio_vs_cs_flip/setplane_vs_cs_flip
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 4
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9b181fc..c0fe5dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3096,7 +3096,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
return false;
}
-static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
+void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index edef34e..f578e42 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -775,6 +775,7 @@ int valleyview_get_vco(struct drm_i915_private *dev_priv);
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
struct intel_crtc_config *pipe_config);
int intel_format_to_fourcc(int format);
+void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
/* intel_dp.c */
void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 4df7245..36a8f5e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -529,6 +529,8 @@ intel_enable_primary(struct drm_crtc *crtc)
if (intel_crtc-primary_enabled)
return;
+ intel_crtc_wait_for_pending_flips(crtc);
+
intel_crtc-primary_enabled = true;
I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
@@ -558,6 +560,8 @@ intel_disable_primary(struct drm_crtc *crtc)
if (!intel_crtc-primary_enabled)
return;
+ intel_crtc_wait_for_pending_flips(crtc);
+
intel_crtc-primary_enabled = false;
mutex_lock(dev-struct_mutex);
--
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] drm/i915/SDVO: For sysfs link put directory and target in correct order
On Fri, Apr 11, 2014 at 08:26:25PM +0300, Imre Deak wrote: On Fri, 2014-04-11 at 19:07 +0200, Egbert Eich wrote: When linking the i2c sysfs file into the connector's directory pass directory and link target in the right order. This code was introduced with: commit 931c1c26983b4f84e33b78579fc8d57e4a14c6b4 Author: Imre Deak imre.d...@intel.com Date: Tue Feb 11 17:12:51 2014 +0200 drm/i915: sdvo: add i2c sysfs symlink to the connector's directory This is the same what we do for DP connectors, so make things more consistent. Signed-off-by: Egbert Eich e...@suse.de Oops, good catch. The fix looks ok, fwiw: Reviewed-by: Imre Deak imre.d...@intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Only break encoder linked when linked to connector
On Mon, Apr 14, 2014 at 07:26:08PM +0200, Egbert Eich wrote:
Depending on the SDVO output_flags SDVO may have multiple connectors
linking to the same encoder (in intel_connector-encoder-base).
Only one of those connectors should be active (ie link to the encoder
thru drm_connector-encoder.
If intel_connector_break_all_links() is called from intel_sanitize_crtc()
we may brake the crtc connection of an encoder thru an inactive connector
in which case intel_connector_break_all_links() will not be called again
for the active connector due to
if (connector-encoder-base.crtc != crtc-base)
continue;
in intel_sanitize_crtc().
This will however leave the drm_connector-encoder linkage for this
active connector in place. Subsequently this will cause multiple
warnings in intel_connector_check_state() to trigger and the driver
will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL).
To avoid this break the encoder links only if the connector is active
(ie. has drm_connector-encoder set).
Signed-off-by: Egbert Eich e...@suse.de
---
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 1390ab5..041f847 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11390,6 +11390,8 @@ static void
intel_connector_break_all_links(struct intel_connector *connector)
{
connector-base.dpms = DRM_MODE_DPMS_OFF;
+ if (!connector-base.encoder)
+ return;
Hm, I think to address Chris' concern we should split this into two
pieces: connector_break_all links which only breaks connector-encoder
stuff, used in both places as is. And a new encoder_break_all links which
we'll use in sanitize_crtc in a new encoder loop.
Really nice catch though!
-Daniel
connector-base.encoder = NULL;
connector-encoder-connectors_active = false;
connector-encoder-base.crtc = NULL;
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect
On Mon, Apr 14, 2014 at 07:26:09PM +0200, Egbert Eich wrote:
Depending on the SDVO output_flags SDVO may have multiple connectors.
These are all initialized in intel_sdvo_output_setup(). The connector
that is initialized later will override the encoder_type that has been
set up by an earlier connector type initialization. Eventually the
one that comes last wins.
Eventually when intel_sdvo_detect() is called the active connector is
determined.
Delay encoder type initialization until the active connector is known
and set it to the type that corresponds to this connector.
Signed-off-by: Egbert Eich e...@suse.de
Hm, has this any effect on the code itself? I think if we want to fix this
just for optics we should add a new DRM_MODE_ENCODER_MULTI. At least I
have an sdvo here which has a dac, hdmi and tv-out ...
This also reminds that I should finally polish of the multi-sdvo fixes I
have around here - currently the last encoder detected wins on a
multi-encoder chip, which means if you have an lvds+hdmi combo and plug in
a screeen you'll never be able to use the lvds again until the hdmi is
unplugged.
Much worse if there's a tv-out detect issue ;-)
Cheers, Daniel
---
drivers/gpu/drm/i915/intel_sdvo.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index d27155a..5043f16 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -154,6 +154,9 @@ struct intel_sdvo_connector {
/* Mark the type of connector */
uint16_t output_flag;
+ /* store encoder type for convenience */
+ int encoder_type;
+
enum hdmi_force_audio force_audio;
/* This contains all current supported TV format */
@@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool
force)
if (response == 0)
return connector_status_disconnected;
+ intel_sdvo-base.base.encoder_type = intel_sdvo_connector-encoder_type;
intel_sdvo-attached_output = response;
intel_sdvo-has_hdmi_monitor = false;
@@ -2489,7 +2493,9 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int
device)
} else {
intel_connector-polled = DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT;
}
- encoder-encoder_type = DRM_MODE_ENCODER_TMDS;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_TMDS;
+ encoder-encoder_type = DRM_MODE_ENCODER_NONE;
connector-connector_type = DRM_MODE_CONNECTOR_DVID;
if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
@@ -2524,7 +2530,9 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int
type)
intel_connector = intel_sdvo_connector-base;
connector = intel_connector-base;
- encoder-encoder_type = DRM_MODE_ENCODER_TVDAC;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_TVDAC;
+ encoder-encoder_type = DRM_MODE_ENCODER_NONE;
connector-connector_type = DRM_MODE_CONNECTOR_SVIDEO;
intel_sdvo-controlled_output |= type;
@@ -2568,7 +2576,9 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo,
int device)
intel_connector = intel_sdvo_connector-base;
connector = intel_connector-base;
intel_connector-polled = DRM_CONNECTOR_POLL_CONNECT;
- encoder-encoder_type = DRM_MODE_ENCODER_DAC;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_DAC;
+ encoder-encoder_type = DRM_MODE_ENCODER_NONE;
connector-connector_type = DRM_MODE_CONNECTOR_VGA;
if (device == 0) {
@@ -2603,7 +2613,9 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int
device)
intel_connector = intel_sdvo_connector-base;
connector = intel_connector-base;
- encoder-encoder_type = DRM_MODE_ENCODER_LVDS;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_LVDS;
+ encoder-encoder_type = DRM_MODE_ENCODER_NONE;
connector-connector_type = DRM_MODE_CONNECTOR_LVDS;
if (device == 0) {
@@ -2984,7 +2996,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t
sdvo_reg, bool is_sdvob)
/* encoder type will be decided later */
intel_encoder = intel_sdvo-base;
intel_encoder-type = INTEL_OUTPUT_SDVO;
- drm_encoder_init(dev, intel_encoder-base, intel_sdvo_enc_funcs, 0);
+ drm_encoder_init(dev, intel_encoder-base, intel_sdvo_enc_funcs,
+ DRM_MODE_ENCODER_NONE);
/* Read the regs to test if we can talk to the device */
for (i = 0; i 0x40; i++) {
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
[Intel-gfx] [PATCH v2] drm/i915: Set up SDVO encoder type only at detect
Depending on the SDVO output_flags SDVO may have multiple connectors.
These are all initialized in intel_sdvo_output_setup(). The connector
that is initialized later will override the encoder_type that has been
set up by an earlier connector type initialization. Eventually the
one that comes last wins.
Eventually when intel_sdvo_detect() is called the active connector is
determined.
Delay encoder type initialization until the active connector is known
and set it to the type that corresponds to this connector.
v2: Remove explicit encoder type initialization to
DRM_MODE_ENCODER_NONE in the SDVO connector setup functions
as suggested by Chris Wilson ch...@chris-wilson.co.uk.
Signed-off-by: Egbert Eich e...@suse.de
---
drivers/gpu/drm/i915/intel_sdvo.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index d27155a..f324ca1 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -154,6 +154,9 @@ struct intel_sdvo_connector {
/* Mark the type of connector */
uint16_t output_flag;
+ /* store encoder type for convenience */
+ int encoder_type;
+
enum hdmi_force_audio force_audio;
/* This contains all current supported TV format */
@@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool
force)
if (response == 0)
return connector_status_disconnected;
+ intel_sdvo-base.base.encoder_type = intel_sdvo_connector-encoder_type;
intel_sdvo-attached_output = response;
intel_sdvo-has_hdmi_monitor = false;
@@ -2489,7 +2493,8 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int
device)
} else {
intel_connector-polled = DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT;
}
- encoder-encoder_type = DRM_MODE_ENCODER_TMDS;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_TMDS;
connector-connector_type = DRM_MODE_CONNECTOR_DVID;
if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
@@ -2524,7 +2529,8 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int
type)
intel_connector = intel_sdvo_connector-base;
connector = intel_connector-base;
- encoder-encoder_type = DRM_MODE_ENCODER_TVDAC;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_TVDAC;
connector-connector_type = DRM_MODE_CONNECTOR_SVIDEO;
intel_sdvo-controlled_output |= type;
@@ -2568,7 +2574,8 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int
device)
intel_connector = intel_sdvo_connector-base;
connector = intel_connector-base;
intel_connector-polled = DRM_CONNECTOR_POLL_CONNECT;
- encoder-encoder_type = DRM_MODE_ENCODER_DAC;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_DAC;
connector-connector_type = DRM_MODE_CONNECTOR_VGA;
if (device == 0) {
@@ -2603,7 +2610,8 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int
device)
intel_connector = intel_sdvo_connector-base;
connector = intel_connector-base;
- encoder-encoder_type = DRM_MODE_ENCODER_LVDS;
+ /* delay encoder_type setting until detection */
+ intel_sdvo_connector-encoder_type = DRM_MODE_ENCODER_LVDS;
connector-connector_type = DRM_MODE_CONNECTOR_LVDS;
if (device == 0) {
@@ -2981,10 +2989,11 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t
sdvo_reg, bool is_sdvob)
if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev))
goto err_i2c_bus;
- /* encoder type will be decided later */
+ /* encoder type will be decided in intel_sdvo_detect() */
intel_encoder = intel_sdvo-base;
intel_encoder-type = INTEL_OUTPUT_SDVO;
- drm_encoder_init(dev, intel_encoder-base, intel_sdvo_enc_funcs, 0);
+ drm_encoder_init(dev, intel_encoder-base, intel_sdvo_enc_funcs,
+DRM_MODE_ENCODER_NONE);
/* Read the regs to test if we can talk to the device */
for (i = 0; i 0x40; i++) {
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Set up SDVO encoder type only at detect
On Tue, Apr 15, 2014 at 09:14:13PM +0200, Egbert Eich wrote: Depending on the SDVO output_flags SDVO may have multiple connectors. These are all initialized in intel_sdvo_output_setup(). The connector that is initialized later will override the encoder_type that has been set up by an earlier connector type initialization. Eventually the one that comes last wins. Eventually when intel_sdvo_detect() is called the active connector is determined. Delay encoder type initialization until the active connector is known and set it to the type that corresponds to this connector. v2: Remove explicit encoder type initialization to DRM_MODE_ENCODER_NONE in the SDVO connector setup functions as suggested by Chris Wilson ch...@chris-wilson.co.uk. Signed-off-by: Egbert Eich e...@suse.de Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] Reduce intel_display.c
On Fri, Apr 11, 2014 at 10:21:59AM +0300, Jani Nikula wrote: On Fri, 11 Apr 2014, Ben Widawsky b...@bwidawsk.net wrote: On Wed, Apr 09, 2014 at 06:44:29PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Hi We always talk about how intel_display.c is a giant file and how we would like to reduce it, so this is my attempt. Currently the file has 12090 lines, and after my patch series it has 8850 lines. I don't know if right now is the appropriate time to merge patches like this. I don't remember seeing too many patches on the list touching cursor/fdi/eld/pll functions, but I know there is never an appropriate time for huge changes. Also, this change will obviously make the lives of people who backport our patches more complicated. So if we don't want this series at all, feel free to NACK it. I am only responding because it seems nobody else really said much. I never touch this code, and I shouldn't be the authority. I really quickly glanced at the patches. 1. +LOC: It sucks that you ended up adding 220 lines. I assume half of it is the copyright header, but still, considering there are no actual refactors, cleanups, or functional changes - adding lines makes me unhappy. 2. necessary? I personally haven't heard from anyone that we need to shrink intel_display.c (again, I am the furthest from being an expert). I doubt anyone isn't using some form of tags, or grep to navigate anyway. My problem has never been the file size itself, but just the structure of the display code interacting with the core KMS was hard to follow. 3. conflicts: Like you said, it's likely nobody touches this code, but we should keep in mind we do have several people working on older branches, and this kind of thing makes any sort of backport hard. On the other hand: 1. If more than one person finds the results more readable/consumable, I think it's worth it, and probably mostly justifies doing it. You've also shrunk the file by quite a bit, so it's somewhat useful churn. 2. intel_pll.c sounds like a good idea I'm in favour of reducing the size of intel_display.c. I did not look at the patches though, because I've sent a few patches to this effect in the past (limits/pll and quirks at least), but they were stalled because they were in the collision course with the Grand Plans Daniel has. So Daniel, I expect you to chime in on this one too. ;) Chiming in now ;-) I've seen a few extract stuff patches float by and occasionally also merged some, but thus far I' haven't been terribly convinced. I don't mind the conflicts these patches cause, but if we want to reorg the driver the goal shouldn't be to just make files smaller (cscope can cope) but actually improve the organization of all this. Often these patches just grab a bag of functions with related names, throw them all into a new file and then add forward and header declaration until the damn thing compiles again. What we want instead are real code modules where interactions within one file are high and the number of exported functions fairly low. Two examples: - Extracting the pageflip helpers and related code would mean that we also should extract a new pageflip_init functions, so that all the platform functions don't need to be exported. We've done similar things when creating intel_unocore.c. - I've just stumbled around in the haswell code and noticed that pretty much all the lpt and hsw fdi code could be moved into intel_crt.c with a new hsw specific crt encoder. In the pre_enable and post_disable hooks we could then do the ddi setup, fdi link training. And all the ltp handling code could mostly be moved away too. With this we could also remove a lot (if not all) of the has_pch_encoder checks and I think also many type == INTEL_OUTPUT_ANALOG checks in the haswell code. I haven't actually checked whether it'll all nicely work out, but my gut feeling says it'll be fewer forward declarations than just shoveling all the fdi related functions (including the lpt stuff) into a new intel_fdi.c. So from my pov to make extractions into new files really useful we need to put a bit of time into first polishing the interfaces a bit and maybe reorganizing the code structure to have real modules. In my experience trying to write abi docs for those interfaces helps a lot. To reduce the conflict/backport pains, might be good to do this spread out over a few releases instead of everything at once. *shrug*. tbh it'll always hurt, and due to our rolling -next model there's never really a time where it's convenient. I'm willing to merge such reorg patches pretty much always. Taking all the above into account I propose the following approach: 1. Each patch series only does one extraction. 2. The series also does any necessary interface polish where the extraction requires tons of header declarations. 3. To make it really worth it
Re: [Intel-gfx] Power saving using Display port HPD
On Mon, Apr 14, 2014 at 11:17:53AM +0300, Imre Deak wrote: On Mon, 2014-04-14 at 09:47 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 9:43 AM, Arun Chandran achand...@mvista.com wrote: 1) Revert the commit 77961eb984c7e5394bd29cc7be2ab0bf0cc7e7b1. With this commit DP hotplug events are not coming after doing xset dpms force off commit bfcbf45b5b458ebdc38118ca67279a1cd90e085d Author: Arun Chandran achand...@mvista.com Date: Fri Apr 11 16:16:32 2014 +0530 Revert drm/i915: power domains: add vlv power wells This reverts commit 77961eb984c7e5394bd29cc7be2ab0bf0cc7e7b1. So this breaks DP hotplug detection? Imre? Yes, unfortunately. I made this clear in my patchset [1] and it was also discussed on IRC. If there isn't any (e)DP,HDMI pipe active we power down the DPIO HW block responsible normally for DP and HDMI hotplug detection. There is one possible solution: the pin that is used for HPD detection can be used either normally in the above way, where it's controlled by the DPIO block, or as a GPIO where it's controlled by the GPIO HW block which is on even if we power down the DPIO. So during power down periods we could reconfigure that pin to work as a GPIO and treat any interrupts arriving it as an HPD event. I haven't had yet time to investigate this. Another way is to turn on polling while powered down. This would also make VGA hotplug work, for which we don't have a GPIO alternative as above. [1] http://lists.freedesktop.org/archives/intel-gfx/2014-February/040232.html Iirc we've agreed that when all screens are off it's ok to no longer support hotplug. Or is this only the case when _only_ the DP port is off but e.g. another port (edp or mipi) is on? I'm asking since currently on hsw/bdw hotplug also doesn't work when you switch everything off ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] tests/gem_error_capture: Initial testcase for error state capture/dump
On Mon, Apr 14, 2014 at 01:03:58PM +, Mateo Lozano, Oscar wrote: I would add a little more smarts to both the kernel and error-decode. In the kernel, we can print the guilty request, which you can then use to confirm that it is yours. That seems to me to be a stronger validation of gem_error_capture, and a useful bit of information from hangstats that we do not expose currently. That sounds good. I have to add a number of other things to i915_gpu_error as part of the Execlists code, so I´ll add a --- guilty request as well and resubmit this test together with the series. If we want this much smarts then we need a properly hanging batch, e.g. like the looping batch used in gem_reset_stats. The problem with that is that this will kill the gpu if reset doesn't work (i.e. gen2/3) so we need to skip this test there. Or maybe split things into 2 subtests and use the properly hanging batch only when we do the extended guilty testing under discussion here. But in any case just checking that the batch is somewhere in the ring (properly masking of lower bits 0-11 ofc) and checking whether the batch is correctl dumped (with the magic value) would catch a lot of the pastpresent execbuf bugs - we've had issues with dumping fancy values of 0 a lot. For the guilty stuff we have an extensive set of tests in gem_reset_stat using the reset stat ioctl already. And for the occasional the hang detection logic is busted bug I think nothing short of a human brain locking at the batch really helps. At least if we want to be somewhat platform agnostic ... So imo the current level of checking loosk Good Enough. But I'm certainly not going to stop you ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] REGRESSION 3.14 i915 warning mouse cursor vanishing
On Mon, Apr 14, 2014 at 11:56:03AM -0700, Steven Noonan wrote: On Mon, Apr 14, 2014 at 11:35:05AM -0700, Keith Packard wrote: 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. Any idea what the trace is all about then? Seems it has something to do with runtime power management (maybe my aggressive kernel command-line options are triggering it). Please test without them. Currently runtime pm should be disabled still on vlv (since it's incomplete in 3.14). If you've force-enabled that then you get to keep all pieces ;-) In general don't set any i915 options if you're not a developer or someone else who _really_ knows what's going on. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Code cleanup patch to fix checkpatch errors
On Tue, Apr 15, 2014 at 01:54:07PM +0530, Shobhit Kumar wrote: This cleans up the checkpatch errors for the merged commit - commit d3b542fcfc72d7724585e3fd2c5e75351bc3df47 Author: Shobhit Kumar shobhit.ku...@intel.com Date: Mon Apr 14 11:00:34 2014 +0530 drm/i915: Add parsing support for new MIPI blocks in VBT Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)
On Tue, Apr 15, 2014 at 11:10:34AM -0500, Jeff McGee wrote: Oh, nevermind. I understand now. Quickly explaining your understanding for everyone else's benefit would be nice ... In general be explicit and verbose on mailing lists to make sure you don't miss some important bit of context people from a different geo lack. And if you discussed something off-list (either off or chat) please summarize the conclusions and participants in an on-list message. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Power saving using Display port HPD
On Tue, 2014-04-15 at 21:32 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 11:17:53AM +0300, Imre Deak wrote: On Mon, 2014-04-14 at 09:47 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 9:43 AM, Arun Chandran achand...@mvista.com wrote: 1) Revert the commit 77961eb984c7e5394bd29cc7be2ab0bf0cc7e7b1. With this commit DP hotplug events are not coming after doing xset dpms force off commit bfcbf45b5b458ebdc38118ca67279a1cd90e085d Author: Arun Chandran achand...@mvista.com Date: Fri Apr 11 16:16:32 2014 +0530 Revert drm/i915: power domains: add vlv power wells This reverts commit 77961eb984c7e5394bd29cc7be2ab0bf0cc7e7b1. So this breaks DP hotplug detection? Imre? Yes, unfortunately. I made this clear in my patchset [1] and it was also discussed on IRC. If there isn't any (e)DP,HDMI pipe active we power down the DPIO HW block responsible normally for DP and HDMI hotplug detection. There is one possible solution: the pin that is used for HPD detection can be used either normally in the above way, where it's controlled by the DPIO block, or as a GPIO where it's controlled by the GPIO HW block which is on even if we power down the DPIO. So during power down periods we could reconfigure that pin to work as a GPIO and treat any interrupts arriving it as an HPD event. I haven't had yet time to investigate this. Another way is to turn on polling while powered down. This would also make VGA hotplug work, for which we don't have a GPIO alternative as above. [1] http://lists.freedesktop.org/archives/intel-gfx/2014-February/040232.html Iirc we've agreed that when all screens are off it's ok to no longer support hotplug. Or is this only the case when _only_ the DP port is off but e.g. another port (edp or mipi) is on? If eDP is on HPD should work fine since the DPIO block is on. With only MIPI on, we would atm turn off the DPIO, so I assume we would again lose HPD :( But I haven't tested this last scenario. Btw, I think Antti is planning to look into the GPIO workaround thing, so if that works out we'd get back HPD for DP and HDMI at least (but not for VGA). I'm asking since currently on hsw/bdw hotplug also doesn't work when you switch everything off ... Hm, on BDW/HSW we mask all interrupts at runtime suspend-D3 state, so that's the reason there .. I don't know if it's possible to get a wake-up signal on an HPD event in D3, I haven't checked this myself, maybe Paulo knows. But I doubt. CC'ing him. It's a bit different than VLV, since there we lose HPD already in D0, when the display side is off. On BDW,HSW in that case we still have HPD, although probably shortly afterwards (10 sec) runtime supend-D3 follows anyway. --Imre ___ 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
On Tue, 2014-04-15 at 21:43 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 11:56:03AM -0700, Steven Noonan wrote: On Mon, Apr 14, 2014 at 11:35:05AM -0700, Keith Packard wrote: 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. Any idea what the trace is all about then? Seems it has something to do with runtime power management (maybe my aggressive kernel command-line options are triggering it). Please test without them. Currently runtime pm should be disabled still on vlv (since it's incomplete in 3.14). If you've force-enabled that then you get to keep all pieces ;-) In general don't set any i915 options if you're not a developer or someone else who _really_ knows what's going on. Note that the lspci output and the [ 1795.275026] [drm:hsw_unclaimed_reg_clear] *ERROR* Unknown unclaimed register before writing to 70084 line suggests HSW and the specs for ThinkPad Yoga suggests the same. But I don't know how the vlv_* functions can possible end up in those traces then, perhaps just a coincidence, random data on stack? For HSW the rc6 kernel option shouldn't make a difference. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Move plane enabling to the end of ilk_crtc_enable
On Tue, Apr 15, 2014 at 06:41:23PM +0200, Daniel Vetter wrote: Like on hsw/bdw the pipe only starts running once the port/pch transcoder combo is all enabled. Before that the vblank wait in the primary plane enable function simply times out. This is also really nice prep work for atomic modesets since now all the plane enabling is at the very end and all tightly grouped together, like on hsw+. Which means we can enable it all atomically with a nuclear pageflip. vlv is still different and the watermark code is also still somewhere else, so it's not yet quite perfect. But we're getting there. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77297 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e0310e3018ee..e6555c0dedca 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3714,9 +3714,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); - intel_enable_primary_hw_plane(dev_priv, plane, pipe); - intel_enable_planes(crtc); - intel_crtc_update_cursor(crtc, true); if (intel_crtc-config.has_pch_encoder) ironlake_pch_enable(crtc); @@ -3742,6 +3739,10 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) */ intel_wait_for_vblank(dev, intel_crtc-pipe); + intel_enable_primary_hw_plane(dev_priv, plane, pipe); + intel_enable_planes(crtc); + intel_crtc_update_cursor(crtc, true); This more or less duplicates what's in my watermark series already. Except you have a few more bugs here. The intel_wait_for_vblank() should be after the plane enabling since it's a hack to avoid the flip done interrupts getting mixed up. Also intel_update_fbc() must happen after enabling the planes. I also made the enable and disable symmetric whereas you didn't. Dunno maybe you just want to grab the patch from my series? + drm_vblank_on(dev, pipe); } -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)
On Tue, Apr 15, 2014 at 11:10:34AM -0500, Jeff McGee wrote:
On Tue, Apr 15, 2014 at 11:00:33AM -0500, Jeff McGee wrote:
On Thu, Mar 27, 2014 at 05:59:48PM +, oscar.ma...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
For the most part, logical rinf context objects are similar to hardware
contexts in that the backing object is meant to be opaque. There are
some exceptions where we need to poke certain offsets of the object for
initialization, updating the tail pointer or updating the PDPs.
For our basic execlist implementation we'll only need our PPGTT PDs,
and ringbuffer addresses in order to set up the context. With previous
patches, we have both, so start prepping the context to be load.
Before running a context for the first time you must populate some
fields in the context object. These fields begin 1 PAGE + LRCA, ie. the
first page (in 0 based counting) of the context image. These same
fields will be read and written to as contexts are saved and restored
once the system is up and running.
Many of these fields are completely reused from previous global
registers: ringbuffer head/tail/control, context control matches some
previous MI_SET_CONTEXT flags, and page directories. There are other
fields which we don't touch which we may want in the future.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and (11)
for other engines.
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
v3: Several rebases and general changes to the code.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/i915_lrc.c | 145
++--
1 file changed, 138 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_lrc.c
b/drivers/gpu/drm/i915/i915_lrc.c
index 40dfa95..f0176ff 100644
--- a/drivers/gpu/drm/i915/i915_lrc.c
+++ b/drivers/gpu/drm/i915/i915_lrc.c
@@ -43,6 +43,38 @@
#define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
+#define RING_ELSP(ring) ((ring)-mmio_base+0x230)
+#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
+
+#define CTX_LRI_HEADER_0 0x01
+#define CTX_CONTEXT_CONTROL 0x02
+#define CTX_RING_HEAD0x04
+#define CTX_RING_TAIL0x06
+#define CTX_RING_BUFFER_START0x08
+#define CTX_RING_BUFFER_CONTROL 0x0a
+#define CTX_BB_HEAD_U0x0c
+#define CTX_BB_HEAD_L0x0e
+#define CTX_BB_STATE 0x10
+#define CTX_SECOND_BB_HEAD_U 0x12
+#define CTX_SECOND_BB_HEAD_L 0x14
+#define CTX_SECOND_BB_STATE 0x16
+#define CTX_BB_PER_CTX_PTR 0x18
+#define CTX_RCS_INDIRECT_CTX 0x1a
+#define CTX_RCS_INDIRECT_CTX_OFFSET 0x1c
+#define CTX_LRI_HEADER_1 0x21
+#define CTX_CTX_TIMESTAMP0x22
+#define CTX_PDP3_UDW 0x24
+#define CTX_PDP3_LDW 0x26
+#define CTX_PDP2_UDW 0x28
+#define CTX_PDP2_LDW 0x2a
+#define CTX_PDP1_UDW 0x2c
+#define CTX_PDP1_LDW 0x2e
+#define CTX_PDP0_UDW 0x30
+#define CTX_PDP0_LDW 0x32
+#define CTX_LRI_HEADER_2 0x41
+#define CTX_R_PWR_CLK_STATE 0x42
+#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
+
struct i915_hw_context *
gen8_gem_create_context(struct drm_device *dev,
struct intel_engine *ring,
@@ -51,6 +83,9 @@ gen8_gem_create_context(struct drm_device *dev,
{
struct i915_hw_context *ctx = NULL;
struct drm_i915_gem_object *ring_obj = NULL;
+ struct i915_hw_ppgtt *ppgtt = NULL;
+ struct page *page;
+ uint32_t *reg_state;
int ret;
ctx = i915_gem_create_context(dev, file_priv, create_vm);
@@ -79,18 +114,114 @@ gen8_gem_create_context(struct drm_device *dev,
/* Failure at this point is almost impossible */
ret = i915_gem_object_set_to_gtt_domain(ring_obj, true);
- if (ret) {
- i915_gem_object_ggtt_unpin(ring_obj);
- drm_gem_object_unreference(ring_obj-base);
- i915_gem_object_ggtt_unpin(ctx-obj);
- i915_gem_context_unreference(ctx);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto destroy_ring_obj;
ctx-ringbuf = ring-default_ringbuf;
ctx-ringbuf-obj = ring_obj;
+ ppgtt = ctx_to_ppgtt(ctx);
+
+ ret = i915_gem_object_set_to_cpu_domain(ctx-obj, true);
+ if (ret)
+ goto destroy_ring_obj;
+
+ ret = i915_gem_object_get_pages(ctx-obj);
+ if (ret)
+ goto destroy_ring_obj;
+
+
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't vblank wait on ilk-ivb after pipe enable
On Tue, Apr 15, 2014 at 06:41:22PM +0200, Daniel Vetter wrote: Like on hsw/bdw the pipe isn't actually running yet at this point. This holds for both pch ports and the cpu edp port according to my testing on ilk, snb and ivb. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77297 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Yeah I hate these weird waits we have sprinkled all over the place w/o clear justification. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1aae7361b7a5..e0310e3018ee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1827,16 +1827,6 @@ static void intel_enable_pipe(struct intel_crtc *crtc) I915_WRITE(reg, val | PIPECONF_ENABLE); POSTING_READ(reg); - - /* - * There's no guarantee the pipe will really start running now. It - * depends on the Gen, the output type and the relative order between - * pipe and plane enabling. Avoid waiting on HSW+ since it's not - * necessary. - * TODO: audit the previous gens. - */ - if (INTEL_INFO(dev)-gen = 7 !IS_HASWELL(dev)) - intel_wait_for_vblank(dev_priv-dev, pipe); } /** @@ -4461,7 +4451,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); + intel_wait_for_vblank(dev_priv-dev, pipe); intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + intel_enable_primary_hw_plane(dev_priv, plane, pipe); intel_enable_planes(crtc); intel_crtc_update_cursor(crtc, true); @@ -4546,7 +4538,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); + intel_wait_for_vblank(dev_priv-dev, pipe); intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); + intel_enable_primary_hw_plane(dev_priv, plane, pipe); intel_enable_planes(crtc); /* The fixup needs to happen before cursor is enabled */ -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Move plane enabling to the end of ilk_crtc_enable
On Tue, Apr 15, 2014 at 10:05 PM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: This more or less duplicates what's in my watermark series already. Except you have a few more bugs here. The intel_wait_for_vblank() should be after the plane enabling since it's a hack to avoid the flip done interrupts getting mixed up. Also intel_update_fbc() must happen after enabling the planes. I also made the enable and disable symmetric whereas you didn't. Dunno maybe you just want to grab the patch from my series? Which one would that be? I just frobbed things sufficiently until the warning from enable_primary_hw_plane disappeared ;-) But yeah fbc should be enabled afterwards for sure. I'm really looking forward to the brave new world where we bring up the pipe in a fashionable black and then do a nuclear flip. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] The whole round of i-g-t testing cost too long running time
From: Vetter, Daniel Sent: Wednesday, April 16, 2014 1:18 AM To: Yang, Guang A; Barnes, Jesse; Widawsky, Benjamin; Wood, Thomas; Jin, Gordon; OTC GFX QA Extended; intel-gfx@lists.freedesktop.org; Parenteau, Paul A; Nikkanen, Kimmo Subject: Re: The whole round of i-g-t testing cost too long running time On 15/04/2014 17:46, Yang, Guang A wrote: Hi all, I have discussed with Daniel about the running time for each cases before and we set the standard as 10M, if one can't finish after running 10M we will see it as Timeout and report bug on FDO(such as : Bug 77474https://bugs.freedesktop.org/show_bug.cgi?id=77474 - [PNV/IVB/HSW]igt/gem_tiled_swapping is slow and Bug 77475https://bugs.freedesktop.org/show_bug.cgi?id=77475 - [PNV/IVB/HSW]igt//kms_pipe_crc_basic/read-crc-pipe-A is slow) Now the true status is that i-g-t have more than 650+ subcases, running a whole round of testing will cost such a long time on QA side(beside that Timeout cases), QA also need to spend more time to analysis the result changing on each platforms. You can find an example with this page: http://tinderbox.sh.intel.com/PRTS_UI/prtsresult.php?task_id=2778 for how long one testing round cost. With the table of subtask:10831 on the page which for i-g-t test cases on BDW. Testing start at 19:16 PM and finished at 03:25 AM the next day, cost about 8 hours to run 638 test cases. Each cases finished less than 10M as we expect, but the full time it too large, especially the BDW is the powerful machine on our side, ILK or PNV may take more than 10 hours. We not only run i-g-t but also need to test the piglit/performance/media which already need time. Do we have any solutions to reduce the running time for whole i-g-t? it's a pressing problem for QA after seeing the i-g-t case count enhance from 50 -600+. Ok there are a few cases where we can indeed make tests faster, but it will be work for us. And that won't really speed up much since we're adding piles more testcases at a pretty quick rate. And many of these new testcases are CRC based, so inheritely take some time to run. [He, Shuang] OK, so it takes at least n/60 in usual case to have result detected plus additional execution time, depending on how many rounds of testing. We will be absolutely happy to see more tests coming that is useful So I think longer-term we simply need to throw more machines at the problem and run testcases in parallel on identical machines. [He, Shuang] This would be the perfect way to go if all tests are really feasible to take long time to run. If we get more identical test machines, then problem solved Wrt analyzing issues I think the right approach for moving forward is: a) switch to piglit to run tests, not just enumerate them. This will allow QA and developers to share testcase analysis. [He, Shuang] Yes, though this could not actually accelerate the test. We could directly wrap over piglit to run testing (have other control process to monitor and collecting test results) b) add automated analysis for time-consuming and error prone cases like dmesg warnings and backtraces. ThomasI have just discussed a few ideas in this are in our 1:1 today. Reducing the set of igt tests we run is imo pointless: The goal of igt is to hit corner-cases, arbitrarily selecting which kinds of corner-cases we test just means that we have a nice illusion about our test coverage. [He, Shuang] I don't think select a subset of test cases to run is pointless. It's a trade-off between speed and correctness. For our nightly testing it's not so useful to run only a small set of testing. But for fast sanity testing, it should be easier, which is supposed to catch regression in major/critical functionality (So other developers and QA could continue their work). Adding more people to the discussion. Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)
On Tue, Apr 15, 2014 at 03:43:23PM -0500, Jeff McGee wrote:
On Tue, Apr 15, 2014 at 11:10:34AM -0500, Jeff McGee wrote:
On Tue, Apr 15, 2014 at 11:00:33AM -0500, Jeff McGee wrote:
On Thu, Mar 27, 2014 at 05:59:48PM +, oscar.ma...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
For the most part, logical rinf context objects are similar to hardware
contexts in that the backing object is meant to be opaque. There are
some exceptions where we need to poke certain offsets of the object for
initialization, updating the tail pointer or updating the PDPs.
For our basic execlist implementation we'll only need our PPGTT PDs,
and ringbuffer addresses in order to set up the context. With previous
patches, we have both, so start prepping the context to be load.
Before running a context for the first time you must populate some
fields in the context object. These fields begin 1 PAGE + LRCA, ie. the
first page (in 0 based counting) of the context image. These same
fields will be read and written to as contexts are saved and restored
once the system is up and running.
Many of these fields are completely reused from previous global
registers: ringbuffer head/tail/control, context control matches some
previous MI_SET_CONTEXT flags, and page directories. There are other
fields which we don't touch which we may want in the future.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and (11)
for other engines.
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
v3: Several rebases and general changes to the code.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/i915_lrc.c | 145
++--
1 file changed, 138 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_lrc.c
b/drivers/gpu/drm/i915/i915_lrc.c
index 40dfa95..f0176ff 100644
--- a/drivers/gpu/drm/i915/i915_lrc.c
+++ b/drivers/gpu/drm/i915/i915_lrc.c
@@ -43,6 +43,38 @@
#define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
+#define RING_ELSP(ring)
((ring)-mmio_base+0x230)
+#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
+
+#define CTX_LRI_HEADER_0 0x01
+#define CTX_CONTEXT_CONTROL0x02
+#define CTX_RING_HEAD 0x04
+#define CTX_RING_TAIL 0x06
+#define CTX_RING_BUFFER_START 0x08
+#define CTX_RING_BUFFER_CONTROL0x0a
+#define CTX_BB_HEAD_U 0x0c
+#define CTX_BB_HEAD_L 0x0e
+#define CTX_BB_STATE 0x10
+#define CTX_SECOND_BB_HEAD_U 0x12
+#define CTX_SECOND_BB_HEAD_L 0x14
+#define CTX_SECOND_BB_STATE0x16
+#define CTX_BB_PER_CTX_PTR 0x18
+#define CTX_RCS_INDIRECT_CTX 0x1a
+#define CTX_RCS_INDIRECT_CTX_OFFSET0x1c
+#define CTX_LRI_HEADER_1 0x21
+#define CTX_CTX_TIMESTAMP 0x22
+#define CTX_PDP3_UDW 0x24
+#define CTX_PDP3_LDW 0x26
+#define CTX_PDP2_UDW 0x28
+#define CTX_PDP2_LDW 0x2a
+#define CTX_PDP1_UDW 0x2c
+#define CTX_PDP1_LDW 0x2e
+#define CTX_PDP0_UDW 0x30
+#define CTX_PDP0_LDW 0x32
+#define CTX_LRI_HEADER_2 0x41
+#define CTX_R_PWR_CLK_STATE0x42
+#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
+
struct i915_hw_context *
gen8_gem_create_context(struct drm_device *dev,
struct intel_engine *ring,
@@ -51,6 +83,9 @@ gen8_gem_create_context(struct drm_device *dev,
{
struct i915_hw_context *ctx = NULL;
struct drm_i915_gem_object *ring_obj = NULL;
+ struct i915_hw_ppgtt *ppgtt = NULL;
+ struct page *page;
+ uint32_t *reg_state;
int ret;
ctx = i915_gem_create_context(dev, file_priv, create_vm);
@@ -79,18 +114,114 @@ gen8_gem_create_context(struct drm_device *dev,
/* Failure at this point is almost impossible */
ret = i915_gem_object_set_to_gtt_domain(ring_obj, true);
- if (ret) {
- i915_gem_object_ggtt_unpin(ring_obj);
- drm_gem_object_unreference(ring_obj-base);
- i915_gem_object_ggtt_unpin(ctx-obj);
- i915_gem_context_unreference(ctx);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto destroy_ring_obj;
ctx-ringbuf = ring-default_ringbuf;
ctx-ringbuf-obj = ring_obj;
+
Re: [Intel-gfx] Power saving using Display port HPD
On Tue, Apr 15, 2014 at 11:01:02PM +0300, Imre Deak wrote: On Tue, 2014-04-15 at 21:32 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 11:17:53AM +0300, Imre Deak wrote: On Mon, 2014-04-14 at 09:47 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 9:43 AM, Arun Chandran achand...@mvista.com wrote: 1) Revert the commit 77961eb984c7e5394bd29cc7be2ab0bf0cc7e7b1. With this commit DP hotplug events are not coming after doing xset dpms force off commit bfcbf45b5b458ebdc38118ca67279a1cd90e085d Author: Arun Chandran achand...@mvista.com Date: Fri Apr 11 16:16:32 2014 +0530 Revert drm/i915: power domains: add vlv power wells This reverts commit 77961eb984c7e5394bd29cc7be2ab0bf0cc7e7b1. So this breaks DP hotplug detection? Imre? Yes, unfortunately. I made this clear in my patchset [1] and it was also discussed on IRC. If there isn't any (e)DP,HDMI pipe active we power down the DPIO HW block responsible normally for DP and HDMI hotplug detection. There is one possible solution: the pin that is used for HPD detection can be used either normally in the above way, where it's controlled by the DPIO block, or as a GPIO where it's controlled by the GPIO HW block which is on even if we power down the DPIO. So during power down periods we could reconfigure that pin to work as a GPIO and treat any interrupts arriving it as an HPD event. I haven't had yet time to investigate this. Another way is to turn on polling while powered down. This would also make VGA hotplug work, for which we don't have a GPIO alternative as above. [1] http://lists.freedesktop.org/archives/intel-gfx/2014-February/040232.html Iirc we've agreed that when all screens are off it's ok to no longer support hotplug. Or is this only the case when _only_ the DP port is off but e.g. another port (edp or mipi) is on? If eDP is on HPD should work fine since the DPIO block is on. With only MIPI on, we would atm turn off the DPIO, so I assume we would again lose HPD :( But I haven't tested this last scenario. Btw, I think Antti is planning to look into the GPIO workaround thing, so if that works out we'd get back HPD for DP and HDMI at least (but not for VGA). Yeah, this is a bit worse. Otoh most byt platforms actually shipping will only have a hdmi port externally, so I think we could just enable the required power well always as long as we're not in D3. Imo when the integrated panel is on, hotplug really should work. Or maybe we should just switch between hotplug and polling on byt for vga. I'm asking since currently on hsw/bdw hotplug also doesn't work when you switch everything off ... Hm, on BDW/HSW we mask all interrupts at runtime suspend-D3 state, so that's the reason there .. I don't know if it's possible to get a wake-up signal on an HPD event in D3, I haven't checked this myself, maybe Paulo knows. But I doubt. CC'ing him. It's a bit different than VLV, since there we lose HPD already in D0, when the display side is off. On BDW,HSW in that case we still have HPD, although probably shortly afterwards (10 sec) runtime supend-D3 follows anyway. Hm, we need to tune the default runtime value. If all screens are of we should be able to do runtime pm in a few seconds, so anything more than 1 second is massive overkill imo. For the overall hotplug in D3 issue I think we can wait until someone screams with an actual use-case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/24] drm/i915: Disable/enable planes as the first/last thing during modeset on ILK+
On Mon, Apr 07, 2014 at 04:51:21PM -0300, Paulo Zanoni wrote: 2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com We already do this for HSW, but doing it makes sense for everything else as well. Extend it for ILK/SNB/IVB since that's where the new watermark code is used. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com I've frobbed the conflict and added the bz link for the vblank wait warnings which this should fix. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)
On Tue, Apr 15, 2014 at 11:08:02PM +0200, Daniel Vetter wrote:
On Tue, Apr 15, 2014 at 03:43:23PM -0500, Jeff McGee wrote:
On Tue, Apr 15, 2014 at 11:10:34AM -0500, Jeff McGee wrote:
On Tue, Apr 15, 2014 at 11:00:33AM -0500, Jeff McGee wrote:
On Thu, Mar 27, 2014 at 05:59:48PM +, oscar.ma...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
For the most part, logical rinf context objects are similar to
hardware
contexts in that the backing object is meant to be opaque. There are
some exceptions where we need to poke certain offsets of the object
for
initialization, updating the tail pointer or updating the PDPs.
For our basic execlist implementation we'll only need our PPGTT PDs,
and ringbuffer addresses in order to set up the context. With previous
patches, we have both, so start prepping the context to be load.
Before running a context for the first time you must populate some
fields in the context object. These fields begin 1 PAGE + LRCA, ie.
the
first page (in 0 based counting) of the context image. These same
fields will be read and written to as contexts are saved and restored
once the system is up and running.
Many of these fields are completely reused from previous global
registers: ringbuffer head/tail/control, context control matches some
previous MI_SET_CONTEXT flags, and page directories. There are other
fields which we don't touch which we may want in the future.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and (11)
for other engines.
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
v3: Several rebases and general changes to the code.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/i915_lrc.c | 145
++--
1 file changed, 138 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_lrc.c
b/drivers/gpu/drm/i915/i915_lrc.c
index 40dfa95..f0176ff 100644
--- a/drivers/gpu/drm/i915/i915_lrc.c
+++ b/drivers/gpu/drm/i915/i915_lrc.c
@@ -43,6 +43,38 @@
#define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
+#define RING_ELSP(ring)
((ring)-mmio_base+0x230)
+#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
+
+#define CTX_LRI_HEADER_0 0x01
+#define CTX_CONTEXT_CONTROL 0x02
+#define CTX_RING_HEAD0x04
+#define CTX_RING_TAIL0x06
+#define CTX_RING_BUFFER_START0x08
+#define CTX_RING_BUFFER_CONTROL 0x0a
+#define CTX_BB_HEAD_U0x0c
+#define CTX_BB_HEAD_L0x0e
+#define CTX_BB_STATE 0x10
+#define CTX_SECOND_BB_HEAD_U 0x12
+#define CTX_SECOND_BB_HEAD_L 0x14
+#define CTX_SECOND_BB_STATE 0x16
+#define CTX_BB_PER_CTX_PTR 0x18
+#define CTX_RCS_INDIRECT_CTX 0x1a
+#define CTX_RCS_INDIRECT_CTX_OFFSET 0x1c
+#define CTX_LRI_HEADER_1 0x21
+#define CTX_CTX_TIMESTAMP0x22
+#define CTX_PDP3_UDW 0x24
+#define CTX_PDP3_LDW 0x26
+#define CTX_PDP2_UDW 0x28
+#define CTX_PDP2_LDW 0x2a
+#define CTX_PDP1_UDW 0x2c
+#define CTX_PDP1_LDW 0x2e
+#define CTX_PDP0_UDW 0x30
+#define CTX_PDP0_LDW 0x32
+#define CTX_LRI_HEADER_2 0x41
+#define CTX_R_PWR_CLK_STATE 0x42
+#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
+
struct i915_hw_context *
gen8_gem_create_context(struct drm_device *dev,
struct intel_engine *ring,
@@ -51,6 +83,9 @@ gen8_gem_create_context(struct drm_device *dev,
{
struct i915_hw_context *ctx = NULL;
struct drm_i915_gem_object *ring_obj = NULL;
+ struct i915_hw_ppgtt *ppgtt = NULL;
+ struct page *page;
+ uint32_t *reg_state;
int ret;
ctx = i915_gem_create_context(dev, file_priv, create_vm);
@@ -79,18 +114,114 @@ gen8_gem_create_context(struct drm_device *dev,
/* Failure at this point is almost impossible */
ret = i915_gem_object_set_to_gtt_domain(ring_obj, true);
- if (ret) {
- i915_gem_object_ggtt_unpin(ring_obj);
- drm_gem_object_unreference(ring_obj-base);
- i915_gem_object_ggtt_unpin(ctx-obj);
- i915_gem_context_unreference(ctx);
- return ERR_PTR(ret);
- }
+ if (ret)
+
Re: [Intel-gfx] REGRESSION 3.14 i915 warning mouse cursor vanishing
On Tue, Apr 15, 2014 at 12:59 PM, Imre Deak imre.d...@intel.com wrote: On Tue, 2014-04-15 at 21:43 +0200, Daniel Vetter wrote: On Mon, Apr 14, 2014 at 11:56:03AM -0700, Steven Noonan wrote: On Mon, Apr 14, 2014 at 11:35:05AM -0700, Keith Packard wrote: 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. Any idea what the trace is all about then? Seems it has something to do with runtime power management (maybe my aggressive kernel command-line options are triggering it). Please test without them. Currently runtime pm should be disabled still on vlv (since it's incomplete in 3.14). If you've force-enabled that then you get to keep all pieces ;-) In general don't set any i915 options if you're not a developer or someone else who _really_ knows what's going on. Note that the lspci output and the [ 1795.275026] [drm:hsw_unclaimed_reg_clear] *ERROR* Unknown unclaimed register before writing to 70084 line suggests HSW and the specs for ThinkPad Yoga suggests the same. But I don't know how the vlv_* functions can possible end up in those traces then, perhaps just a coincidence, random data on stack? For HSW the rc6 kernel option shouldn't make a difference. Correct, it's Haswell. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 6/6] drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3
The BDW GT3 has two independent BSD rings, which can be used to process the
video commands. To be simpler, it is transparent to user-space driver/middle.
Instead the kernel driver will decide which ring is to dispatch the BSD video
command.
As every BSD ring is powerful, it is enough to dispatch the BSD video command
based on the drm fd. In such case it can play back video stream while encoding
another video stream. The coarse ping-pong mechanism is used to determine
which BSD ring is used to dispatch the BSD video command.
V1-V2: Follow Daniel's comment and use the simple ping-pong mechanism.
This is only to add the support of dual BSD rings on BDW GT3 machine.
The further optimization will be considered in another patch set.
V2-V3: Follow Daniel's comment to use the struct_mutext instead of
atomic_t during determining which ring can be used to dispatch Video command.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c|3 +++
drivers/gpu/drm/i915/i915_drv.h|3 +++
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 40 +++-
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0b38f88..f7558f5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1572,6 +1572,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
spin_lock_init(dev_priv-backlight_lock);
spin_lock_init(dev_priv-uncore.lock);
spin_lock_init(dev_priv-mm.object_stat_lock);
+ dev_priv-ring_index = 0;
mutex_init(dev_priv-dpio_lock);
mutex_init(dev_priv-modeset_restore_lock);
@@ -1929,6 +1930,8 @@ void i915_driver_postclose(struct drm_device *dev, struct
drm_file *file)
{
struct drm_i915_file_private *file_priv = file-driver_priv;
+ if (file_priv file_priv-bsd_ring)
+ file_priv-bsd_ring = NULL;
kfree(file_priv);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 74aef6a..032f992 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1472,6 +1472,8 @@ struct drm_i915_private {
struct i915_dri1_state dri1;
/* Old ums support infrastructure, same warning applies. */
struct i915_ums_state ums;
+ /* the indicator for dispatch video commands on two BSD rings */
+ int ring_index;
};
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
@@ -1679,6 +1681,7 @@ struct drm_i915_file_private {
struct i915_hw_context *private_default_ctx;
atomic_t rps_wait_boost;
+ struct intel_ring_buffer *bsd_ring;
};
/*
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 341ec68..1dc6f03 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -999,6 +999,37 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
return 0;
}
+/**
+ * Find one BSD ring to dispatch the corresponding BSD command.
+ * The Ring ID is returned.
+ */
+static int gen8_dispatch_bsd_ring(struct drm_device *dev,
+ struct drm_file *file)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_i915_file_private *file_priv = file-driver_priv;
+
+ /* Check whether the file_priv is using one ring */
+ if (file_priv-bsd_ring)
+ return file_priv-bsd_ring-id;
+ else {
+ /* If no, use the ping-pong mechanism to select one ring */
+ int ring_id;
+
+ mutex_lock(dev-struct_mutex);
+ if (dev_priv-ring_index == 0) {
+ ring_id = VCS;
+ dev_priv-ring_index = 1;
+ } else {
+ ring_id = VCS2;
+ dev_priv-ring_index = 0;
+ }
+ file_priv-bsd_ring = dev_priv-ring[ring_id];
+ mutex_unlock(dev-struct_mutex);
+ return ring_id;
+ }
+}
+
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1043,7 +1074,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if ((args-flags I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
ring = dev_priv-ring[RCS];
- else
+ else if ((args-flags I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
+ if (HAS_BSD2(dev)) {
+ int ring_id;
+ ring_id = gen8_dispatch_bsd_ring(dev, file);
+ ring = dev_priv-ring[ring_id];
+ } else
+ ring = dev_priv-ring[VCS];
+ } else
ring = dev_priv-ring[(args-flags I915_EXEC_RING_MASK) - 1];
if (!intel_ring_initialized(ring)) {
--
1.7.10.1
[Intel-gfx] [PATCH V3 2/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine
Based on the hardware spec, the BDW GT3 machine has two independent
BSD ring that can be used to dispatch the video commands.
So just initialize it.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c |4 +--
drivers/gpu/drm/i915/i915_drv.h |2 ++
drivers/gpu/drm/i915/i915_gem.c |9 +-
drivers/gpu/drm/i915/i915_gpu_error.c |1 +
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 54 +++
drivers/gpu/drm/i915/intel_ringbuffer.h |4 ++-
7 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 17fbbe5..2a7842b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -282,7 +282,7 @@ static const struct intel_device_info
intel_broadwell_m_info = {
static const struct intel_device_info intel_broadwell_gt3d_info = {
.gen = 8, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
- .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.has_llc = 1,
.has_ddi = 1,
.has_fbc = 1,
@@ -292,7 +292,7 @@ static const struct intel_device_info
intel_broadwell_gt3d_info = {
static const struct intel_device_info intel_broadwell_gt3m_info = {
.gen = 8, .is_mobile = 1, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
- .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.has_llc = 1,
.has_ddi = 1,
.has_fbc = 1,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92c3095..74aef6a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1833,7 +1833,9 @@ struct drm_i915_cmd_table {
#define BSD_RING (1VCS)
#define BLT_RING (1BCS)
#define VEBOX_RING (1VECS)
+#define BSD2_RING (1VCS2)
#define HAS_BSD(dev)(INTEL_INFO(dev)-ring_mask BSD_RING)
+#define HAS_BSD2(dev) (INTEL_INFO(dev)-ring_mask BSD2_RING)
#define HAS_BLT(dev)(INTEL_INFO(dev)-ring_mask BLT_RING)
#define HAS_VEBOX(dev)(INTEL_INFO(dev)-ring_mask VEBOX_RING)
#define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 85c9cf0..b4dcf2a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4374,13 +4374,20 @@ static int i915_gem_init_rings(struct drm_device *dev)
goto cleanup_blt_ring;
}
+ if (HAS_BSD2(dev)) {
+ ret = intel_init_bsd2_ring_buffer(dev);
+ if (ret)
+ goto cleanup_vebox_ring;
+ }
ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
if (ret)
- goto cleanup_vebox_ring;
+ goto cleanup_ring;
return 0;
+cleanup_ring:
+ intel_cleanup_ring_buffer(dev_priv-ring[VCS2]);
cleanup_vebox_ring:
intel_cleanup_ring_buffer(dev_priv-ring[VECS]);
cleanup_blt_ring:
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 4865ade..3cab7f9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -42,6 +42,7 @@ static const char *ring_str(int ring)
case VCS: return bsd;
case BCS: return blt;
case VECS: return vebox;
+ case VCS2: return second bsd;
default: return ;
}
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f84555..0b88508 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -760,6 +760,7 @@ enum punit_power_well {
#define RENDER_RING_BASE 0x02000
#define BSD_RING_BASE 0x04000
#define GEN6_BSD_RING_BASE 0x12000
+#define GEN8_BSD2_RING_BASE0x1c000
#define VEBOX_RING_BASE0x1a000
#define BLT_RING_BASE 0x22000
#define RING_TAIL(base)((base)+0x30)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index eb3dd26..8b9b89080 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1924,10 +1924,12 @@ int intel_init_render_ring_buffer(struct drm_device
*dev)
ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
ring-semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
+ ring-semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
ring-signal_mbox[RCS] = GEN6_NOSYNC;
ring-signal_mbox[VCS] = GEN6_VRSYNC;
[Intel-gfx] [PATCH V3 1/6] drm/i915: Split the BDW device definition to prepare for dual BSD rings on BDW GT3
Based on the hardware spec, the BDW GT3 has the different configuration
with the BDW GT1/GT2. So split the BDW device info definition.
This is to do the preparation for adding the Dual BSD rings on BDW GT3 machine.
V1-V2: Follow Daniel's comment to pay attention to the stolen check for BDW
in kernel/early-quirks.c
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 26 --
include/drm/i915_pciids.h | 22 +-
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5d8250f..17fbbe5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -279,6 +279,26 @@ static const struct intel_device_info
intel_broadwell_m_info = {
GEN_DEFAULT_PIPEOFFSETS,
};
+static const struct intel_device_info intel_broadwell_gt3d_info = {
+ .gen = 8, .num_pipes = 3,
+ .need_gfx_hws = 1, .has_hotplug = 1,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .has_llc = 1,
+ .has_ddi = 1,
+ .has_fbc = 1,
+ GEN_DEFAULT_PIPEOFFSETS,
+};
+
+static const struct intel_device_info intel_broadwell_gt3m_info = {
+ .gen = 8, .is_mobile = 1, .num_pipes = 3,
+ .need_gfx_hws = 1, .has_hotplug = 1,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .has_llc = 1,
+ .has_ddi = 1,
+ .has_fbc = 1,
+ GEN_DEFAULT_PIPEOFFSETS,
+};
+
/*
* Make sure any device matches here are from most specific to most
* general. For example, since the Quanta match is based on the subsystem
@@ -311,8 +331,10 @@ static const struct intel_device_info
intel_broadwell_m_info = {
INTEL_HSW_M_IDS(intel_haswell_m_info), \
INTEL_VLV_M_IDS(intel_valleyview_m_info), \
INTEL_VLV_D_IDS(intel_valleyview_d_info), \
- INTEL_BDW_M_IDS(intel_broadwell_m_info), \
- INTEL_BDW_D_IDS(intel_broadwell_d_info)
+ INTEL_BDW_GT12M_IDS(intel_broadwell_m_info), \
+ INTEL_BDW_GT12D_IDS(intel_broadwell_d_info), \
+ INTEL_BDW_GT3M_IDS(intel_broadwell_gt3m_info), \
+ INTEL_BDW_GT3D_IDS(intel_broadwell_gt3d_info)
static const struct pci_device_id pciidlist[] = { /* aka */
INTEL_PCI_IDS,
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 940ece4..24f3cad 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -223,14 +223,26 @@
_INTEL_BDW_D(gt, 0x160A, info), /* Server */ \
_INTEL_BDW_D(gt, 0x160D, info) /* Workstation */
-#define INTEL_BDW_M_IDS(info) \
+#define INTEL_BDW_GT12M_IDS(info) \
_INTEL_BDW_M_IDS(1, info), \
- _INTEL_BDW_M_IDS(2, info), \
- _INTEL_BDW_M_IDS(3, info)
+ _INTEL_BDW_M_IDS(2, info)
-#define INTEL_BDW_D_IDS(info) \
+#define INTEL_BDW_GT12D_IDS(info) \
_INTEL_BDW_D_IDS(1, info), \
- _INTEL_BDW_D_IDS(2, info), \
+ _INTEL_BDW_D_IDS(2, info)
+
+#define INTEL_BDW_GT3M_IDS(info) \
+ _INTEL_BDW_M_IDS(3, info)
+
+#define INTEL_BDW_GT3D_IDS(info) \
_INTEL_BDW_D_IDS(3, info)
+#define INTEL_BDW_M_IDS(info) \
+ INTEL_BDW_GT12M_IDS(info), \
+ INTEL_BDW_GT3M_IDS(info)
+
+#define INTEL_BDW_D_IDS(info) \
+ INTEL_BDW_GT12D_IDS(info), \
+ INTEL_BDW_GT3D_IDS(info)
+
#endif /* _I915_PCIIDS_H */
--
1.7.10.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 5/6] drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space
One extra ring is added in the kernel driver but it is transparent to the
user-space application/middleware. In such case the number of the rings
in kernel driver is bigger than that exported to the user-space. So
it needs to filter out the wrong Ring ID passed by user-space.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
drivers/gpu/drm/i915/intel_ringbuffer.h|1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3491402..341ec68 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1035,7 +1035,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (args-flags I915_EXEC_IS_PINNED)
flags |= I915_DISPATCH_PINNED;
- if ((args-flags I915_EXEC_RING_MASK) I915_NUM_RINGS) {
+ if ((args-flags I915_EXEC_RING_MASK) LAST_USER_RING) {
DRM_DEBUG(execbuf with unknown ring: %d\n,
(int)(args-flags I915_EXEC_RING_MASK));
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8ca4285..59f4cdd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -64,6 +64,7 @@ struct intel_ring_buffer {
VCS2,
} id;
#define I915_NUM_RINGS 5
+#define LAST_USER_RING (VECS + 1)
u32 mmio_base;
void__iomem *virtual_start;
struct drm_device *dev;
--
1.7.10.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 0/6] drm/i915: Add the support of dual BSD rings on BDW GT3
This is the patch set that tries to add the support of dual BSD rings on BDW GT3. Based on hardware spec, the BDW GT3 has two independent BSD rings, which can be used to process the video commands. To be simpler, it is transparent to user-space driver/middleware. In such case the kernel driver will decide which ring is to dispatch the BSD video command. As every BSD ring is powerful, it is enough to dispatch the BSD video command based on the drm fd. In such case the different BSD ring is used for video playing back and encoding. V1-V2: Follow Daniel's comment to do the following update: a. consider the stolen check for BDW in kernel/early-quirks.c in patch 01 b. update the comment in Patch 04 c. use the simple ping-pong mechanism to add the support of dual BSD rings. The further optimization will be considered in another patch set. V2-V3: Follow Daniel's comment to use the struct_mutext instead of atomic_t during determining which ring can be used to dispatch Video command. Zhao Yakui (6): drm/i915: Split the BDW device definition to prepare for dual BSD rings on BDW GT3 drm/i915:Initialize the second BSD ring on BDW GT3 machine drm/i915:Handle the irq interrupt for the second BSD ring drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to remove the switch check warning drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3 drivers/gpu/drm/i915/i915_dma.c|3 ++ drivers/gpu/drm/i915/i915_drv.c| 26 +++- drivers/gpu/drm/i915/i915_drv.h|5 +++ drivers/gpu/drm/i915/i915_gem.c|9 - drivers/gpu/drm/i915/i915_gem_execbuffer.c | 42 +++- drivers/gpu/drm/i915/i915_gpu_error.c |1 + drivers/gpu/drm/i915/i915_irq.c|5 ++- drivers/gpu/drm/i915/i915_reg.h|1 + drivers/gpu/drm/i915/intel_ringbuffer.c| 59 drivers/gpu/drm/i915/intel_ringbuffer.h|5 ++- include/drm/i915_pciids.h | 22 --- 11 files changed, 166 insertions(+), 12 deletions(-) -- 1.7.10.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 4/6] drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to remove the switch check warning
The Gen7 doesn't have the second BSD ring. But it will complain the switch check warning message during compilation. So just add it to remove the switch check warning. V1-V2: Follow Daniel's comment to update the comment Signed-off-by: Zhao Yakui yakui.z...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8b9b89080..2c89525 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -988,6 +988,11 @@ void intel_ring_setup_status_page(struct intel_ring_buffer *ring) case BCS: mmio = BLT_HWS_PGA_GEN7; break; + /* +* VCS2 actually doesn't exist on Gen7. Only shut up +* gcc switch check warning +*/ + case VCS2: case VCS: mmio = BSD_HWS_PGA_GEN7; break; -- 1.7.10.1 ___ 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/bdw: Implement a basic PM interrupt handler
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81
+---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device
*dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct
*work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
version of this patch, but it's still a valid question.
The answer is ambiguous actually. The WARN is always valid
[Intel-gfx] [PATCH V3 1/6] drm/i915: Split the BDW device definition to prepare for dual BSD rings on BDW GT3
Based on the hardware spec, the BDW GT3 has the different configuration
with the BDW GT1/GT2. So split the BDW device info definition.
This is to do the preparation for adding the Dual BSD rings on BDW GT3 machine.
V1-V2: Follow Daniel's comment to pay attention to the stolen check for BDW
in kernel/early-quirks.c
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 26 --
include/drm/i915_pciids.h | 22 +-
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5d8250f..17fbbe5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -279,6 +279,26 @@ static const struct intel_device_info
intel_broadwell_m_info = {
GEN_DEFAULT_PIPEOFFSETS,
};
+static const struct intel_device_info intel_broadwell_gt3d_info = {
+ .gen = 8, .num_pipes = 3,
+ .need_gfx_hws = 1, .has_hotplug = 1,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .has_llc = 1,
+ .has_ddi = 1,
+ .has_fbc = 1,
+ GEN_DEFAULT_PIPEOFFSETS,
+};
+
+static const struct intel_device_info intel_broadwell_gt3m_info = {
+ .gen = 8, .is_mobile = 1, .num_pipes = 3,
+ .need_gfx_hws = 1, .has_hotplug = 1,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .has_llc = 1,
+ .has_ddi = 1,
+ .has_fbc = 1,
+ GEN_DEFAULT_PIPEOFFSETS,
+};
+
/*
* Make sure any device matches here are from most specific to most
* general. For example, since the Quanta match is based on the subsystem
@@ -311,8 +331,10 @@ static const struct intel_device_info
intel_broadwell_m_info = {
INTEL_HSW_M_IDS(intel_haswell_m_info), \
INTEL_VLV_M_IDS(intel_valleyview_m_info), \
INTEL_VLV_D_IDS(intel_valleyview_d_info), \
- INTEL_BDW_M_IDS(intel_broadwell_m_info), \
- INTEL_BDW_D_IDS(intel_broadwell_d_info)
+ INTEL_BDW_GT12M_IDS(intel_broadwell_m_info), \
+ INTEL_BDW_GT12D_IDS(intel_broadwell_d_info), \
+ INTEL_BDW_GT3M_IDS(intel_broadwell_gt3m_info), \
+ INTEL_BDW_GT3D_IDS(intel_broadwell_gt3d_info)
static const struct pci_device_id pciidlist[] = { /* aka */
INTEL_PCI_IDS,
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 940ece4..24f3cad 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -223,14 +223,26 @@
_INTEL_BDW_D(gt, 0x160A, info), /* Server */ \
_INTEL_BDW_D(gt, 0x160D, info) /* Workstation */
-#define INTEL_BDW_M_IDS(info) \
+#define INTEL_BDW_GT12M_IDS(info) \
_INTEL_BDW_M_IDS(1, info), \
- _INTEL_BDW_M_IDS(2, info), \
- _INTEL_BDW_M_IDS(3, info)
+ _INTEL_BDW_M_IDS(2, info)
-#define INTEL_BDW_D_IDS(info) \
+#define INTEL_BDW_GT12D_IDS(info) \
_INTEL_BDW_D_IDS(1, info), \
- _INTEL_BDW_D_IDS(2, info), \
+ _INTEL_BDW_D_IDS(2, info)
+
+#define INTEL_BDW_GT3M_IDS(info) \
+ _INTEL_BDW_M_IDS(3, info)
+
+#define INTEL_BDW_GT3D_IDS(info) \
_INTEL_BDW_D_IDS(3, info)
+#define INTEL_BDW_M_IDS(info) \
+ INTEL_BDW_GT12M_IDS(info), \
+ INTEL_BDW_GT3M_IDS(info)
+
+#define INTEL_BDW_D_IDS(info) \
+ INTEL_BDW_GT12D_IDS(info), \
+ INTEL_BDW_GT3D_IDS(info)
+
#endif /* _I915_PCIIDS_H */
--
1.7.10.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 4/6] drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to remove the switch check warning
The Gen7 doesn't have the second BSD ring. But it will complain the switch check warning message during compilation. So just add it to remove the switch check warning. V1-V2: Follow Daniel's comment to update the comment Signed-off-by: Zhao Yakui yakui.z...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8b9b89080..2c89525 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -988,6 +988,11 @@ void intel_ring_setup_status_page(struct intel_ring_buffer *ring) case BCS: mmio = BLT_HWS_PGA_GEN7; break; + /* +* VCS2 actually doesn't exist on Gen7. Only shut up +* gcc switch check warning +*/ + case VCS2: case VCS: mmio = BSD_HWS_PGA_GEN7; break; -- 1.7.10.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 6/6] drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3
The BDW GT3 has two independent BSD rings, which can be used to process the
video commands. To be simpler, it is transparent to user-space driver/middle.
Instead the kernel driver will decide which ring is to dispatch the BSD video
command.
As every BSD ring is powerful, it is enough to dispatch the BSD video command
based on the drm fd. In such case it can play back video stream while encoding
another video stream. The coarse ping-pong mechanism is used to determine
which BSD ring is used to dispatch the BSD video command.
V1-V2: Follow Daniel's comment and use the simple ping-pong mechanism.
This is only to add the support of dual BSD rings on BDW GT3 machine.
The further optimization will be considered in another patch set.
V2-V3: Follow Daniel's comment to use the struct_mutext instead of
atomic_t during determining which ring can be used to dispatch Video command.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c|3 +++
drivers/gpu/drm/i915/i915_drv.h|3 +++
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 40 +++-
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0b38f88..f7558f5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1572,6 +1572,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
spin_lock_init(dev_priv-backlight_lock);
spin_lock_init(dev_priv-uncore.lock);
spin_lock_init(dev_priv-mm.object_stat_lock);
+ dev_priv-ring_index = 0;
mutex_init(dev_priv-dpio_lock);
mutex_init(dev_priv-modeset_restore_lock);
@@ -1929,6 +1930,8 @@ void i915_driver_postclose(struct drm_device *dev, struct
drm_file *file)
{
struct drm_i915_file_private *file_priv = file-driver_priv;
+ if (file_priv file_priv-bsd_ring)
+ file_priv-bsd_ring = NULL;
kfree(file_priv);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 74aef6a..032f992 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1472,6 +1472,8 @@ struct drm_i915_private {
struct i915_dri1_state dri1;
/* Old ums support infrastructure, same warning applies. */
struct i915_ums_state ums;
+ /* the indicator for dispatch video commands on two BSD rings */
+ int ring_index;
};
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
@@ -1679,6 +1681,7 @@ struct drm_i915_file_private {
struct i915_hw_context *private_default_ctx;
atomic_t rps_wait_boost;
+ struct intel_ring_buffer *bsd_ring;
};
/*
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 341ec68..1dc6f03 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -999,6 +999,37 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
return 0;
}
+/**
+ * Find one BSD ring to dispatch the corresponding BSD command.
+ * The Ring ID is returned.
+ */
+static int gen8_dispatch_bsd_ring(struct drm_device *dev,
+ struct drm_file *file)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_i915_file_private *file_priv = file-driver_priv;
+
+ /* Check whether the file_priv is using one ring */
+ if (file_priv-bsd_ring)
+ return file_priv-bsd_ring-id;
+ else {
+ /* If no, use the ping-pong mechanism to select one ring */
+ int ring_id;
+
+ mutex_lock(dev-struct_mutex);
+ if (dev_priv-ring_index == 0) {
+ ring_id = VCS;
+ dev_priv-ring_index = 1;
+ } else {
+ ring_id = VCS2;
+ dev_priv-ring_index = 0;
+ }
+ file_priv-bsd_ring = dev_priv-ring[ring_id];
+ mutex_unlock(dev-struct_mutex);
+ return ring_id;
+ }
+}
+
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1043,7 +1074,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if ((args-flags I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
ring = dev_priv-ring[RCS];
- else
+ else if ((args-flags I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
+ if (HAS_BSD2(dev)) {
+ int ring_id;
+ ring_id = gen8_dispatch_bsd_ring(dev, file);
+ ring = dev_priv-ring[ring_id];
+ } else
+ ring = dev_priv-ring[VCS];
+ } else
ring = dev_priv-ring[(args-flags I915_EXEC_RING_MASK) - 1];
if (!intel_ring_initialized(ring)) {
--
1.7.10.1
[Intel-gfx] [PATCH V3 5/6] drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space
One extra ring is added in the kernel driver but it is transparent to the
user-space application/middleware. In such case the number of the rings
in kernel driver is bigger than that exported to the user-space. So
it needs to filter out the wrong Ring ID passed by user-space.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
drivers/gpu/drm/i915/intel_ringbuffer.h|1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3491402..341ec68 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1035,7 +1035,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (args-flags I915_EXEC_IS_PINNED)
flags |= I915_DISPATCH_PINNED;
- if ((args-flags I915_EXEC_RING_MASK) I915_NUM_RINGS) {
+ if ((args-flags I915_EXEC_RING_MASK) LAST_USER_RING) {
DRM_DEBUG(execbuf with unknown ring: %d\n,
(int)(args-flags I915_EXEC_RING_MASK));
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8ca4285..59f4cdd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -64,6 +64,7 @@ struct intel_ring_buffer {
VCS2,
} id;
#define I915_NUM_RINGS 5
+#define LAST_USER_RING (VECS + 1)
u32 mmio_base;
void__iomem *virtual_start;
struct drm_device *dev;
--
1.7.10.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 0/6] drm/i915: Add the support of dual BSD rings on BDW GT3
This is the patch set that tries to add the support of dual BSD rings on BDW GT3. Based on hardware spec, the BDW GT3 has two independent BSD rings, which can be used to process the video commands. To be simpler, it is transparent to user-space driver/middleware. In such case the kernel driver will decide which ring is to dispatch the BSD video command. As every BSD ring is powerful, it is enough to dispatch the BSD video command based on the drm fd. In such case the different BSD ring is used for video playing back and encoding. V1-V2: Follow Daniel's comment to do the following update: a. consider the stolen check for BDW in kernel/early-quirks.c in patch 01 b. update the comment in Patch 04 c. use the simple ping-pong mechanism to add the support of dual BSD rings. The further optimization will be considered in another patch set. V2-V3: Follow Daniel's comment to use the struct_mutext instead of atomic_t during determining which ring can be used to dispatch Video command. Zhao Yakui (6): drm/i915: Split the BDW device definition to prepare for dual BSD rings on BDW GT3 drm/i915:Initialize the second BSD ring on BDW GT3 machine drm/i915:Handle the irq interrupt for the second BSD ring drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to remove the switch check warning drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3 drivers/gpu/drm/i915/i915_dma.c|3 ++ drivers/gpu/drm/i915/i915_drv.c| 26 +++- drivers/gpu/drm/i915/i915_drv.h|5 +++ drivers/gpu/drm/i915/i915_gem.c|9 - drivers/gpu/drm/i915/i915_gem_execbuffer.c | 42 +++- drivers/gpu/drm/i915/i915_gpu_error.c |1 + drivers/gpu/drm/i915/i915_irq.c|5 ++- drivers/gpu/drm/i915/i915_reg.h|1 + drivers/gpu/drm/i915/intel_ringbuffer.c| 59 drivers/gpu/drm/i915/intel_ringbuffer.h|5 ++- include/drm/i915_pciids.h | 22 --- 11 files changed, 166 insertions(+), 12 deletions(-) -- 1.7.10.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 3/6] drm/i915:Handle the irq interrupt for the second BSD ring
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..63bd5de 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1347,13 +1347,16 @@ static irqreturn_t gen8_gt_irq_handler(struct
drm_device *dev,
DRM_ERROR(The master control interrupt lied (GT0)!\n);
}
- if (master_ctl GEN8_GT_VCS1_IRQ) {
+ if (master_ctl (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) {
tmp = I915_READ(GEN8_GT_IIR(1));
if (tmp) {
ret = IRQ_HANDLED;
vcs = tmp GEN8_VCS1_IRQ_SHIFT;
if (vcs GT_RENDER_USER_INTERRUPT)
notify_ring(dev, dev_priv-ring[VCS]);
+ vcs = tmp GEN8_VCS2_IRQ_SHIFT;
+ if (vcs GT_RENDER_USER_INTERRUPT)
+ notify_ring(dev, dev_priv-ring[VCS2]);
I915_WRITE(GEN8_GT_IIR(1), tmp);
} else
DRM_ERROR(The master control interrupt lied (GT1)!\n);
--
1.7.10.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 2/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine
Based on the hardware spec, the BDW GT3 machine has two independent
BSD ring that can be used to dispatch the video commands.
So just initialize it.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c |4 +--
drivers/gpu/drm/i915/i915_drv.h |2 ++
drivers/gpu/drm/i915/i915_gem.c |9 +-
drivers/gpu/drm/i915/i915_gpu_error.c |1 +
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 54 +++
drivers/gpu/drm/i915/intel_ringbuffer.h |4 ++-
7 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 17fbbe5..2a7842b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -282,7 +282,7 @@ static const struct intel_device_info
intel_broadwell_m_info = {
static const struct intel_device_info intel_broadwell_gt3d_info = {
.gen = 8, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
- .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.has_llc = 1,
.has_ddi = 1,
.has_fbc = 1,
@@ -292,7 +292,7 @@ static const struct intel_device_info
intel_broadwell_gt3d_info = {
static const struct intel_device_info intel_broadwell_gt3m_info = {
.gen = 8, .is_mobile = 1, .num_pipes = 3,
.need_gfx_hws = 1, .has_hotplug = 1,
- .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
.has_llc = 1,
.has_ddi = 1,
.has_fbc = 1,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92c3095..74aef6a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1833,7 +1833,9 @@ struct drm_i915_cmd_table {
#define BSD_RING (1VCS)
#define BLT_RING (1BCS)
#define VEBOX_RING (1VECS)
+#define BSD2_RING (1VCS2)
#define HAS_BSD(dev)(INTEL_INFO(dev)-ring_mask BSD_RING)
+#define HAS_BSD2(dev) (INTEL_INFO(dev)-ring_mask BSD2_RING)
#define HAS_BLT(dev)(INTEL_INFO(dev)-ring_mask BLT_RING)
#define HAS_VEBOX(dev)(INTEL_INFO(dev)-ring_mask VEBOX_RING)
#define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 85c9cf0..b4dcf2a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4374,13 +4374,20 @@ static int i915_gem_init_rings(struct drm_device *dev)
goto cleanup_blt_ring;
}
+ if (HAS_BSD2(dev)) {
+ ret = intel_init_bsd2_ring_buffer(dev);
+ if (ret)
+ goto cleanup_vebox_ring;
+ }
ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
if (ret)
- goto cleanup_vebox_ring;
+ goto cleanup_ring;
return 0;
+cleanup_ring:
+ intel_cleanup_ring_buffer(dev_priv-ring[VCS2]);
cleanup_vebox_ring:
intel_cleanup_ring_buffer(dev_priv-ring[VECS]);
cleanup_blt_ring:
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 4865ade..3cab7f9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -42,6 +42,7 @@ static const char *ring_str(int ring)
case VCS: return bsd;
case BCS: return blt;
case VECS: return vebox;
+ case VCS2: return second bsd;
default: return ;
}
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f84555..0b88508 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -760,6 +760,7 @@ enum punit_power_well {
#define RENDER_RING_BASE 0x02000
#define BSD_RING_BASE 0x04000
#define GEN6_BSD_RING_BASE 0x12000
+#define GEN8_BSD2_RING_BASE0x1c000
#define VEBOX_RING_BASE0x1a000
#define BLT_RING_BASE 0x22000
#define RING_TAIL(base)((base)+0x30)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index eb3dd26..8b9b89080 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1924,10 +1924,12 @@ int intel_init_render_ring_buffer(struct drm_device
*dev)
ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
ring-semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
+ ring-semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
ring-signal_mbox[RCS] = GEN6_NOSYNC;
ring-signal_mbox[VCS] = GEN6_VRSYNC;
Re: [Intel-gfx] The whole round of i-g-t testing cost too long running time
Ok there are a few cases where we can indeed make tests faster, but it will be work for us. And that won't really speed up much since we're adding piles more testcases at a pretty quick rate. And many of these new testcases are CRC based, so inheritely take some time to run. [He, Shuang] OK, so it takes at least n/60 in usual case to have result detected plus additional execution time, depending on how many rounds of testing. We will be absolutely happy to see more tests coming that is useful [Guang YANG] Except these CRC case, some stress case may also cost a bit of time, especiallyapp:ds:especially on some old platforms. Maybe can reduce the loop in that kind of stress case? So I think longer-term we simply need to throw more machines at the problem and run testcases in parallel on identical machines. [He, Shuang] This would be the perfect way to go if all tests are really feasible to take long time to run. If we get more identical test machines, then problem solved [Guang YANG] shuang's PRTS can cover some work for i-g-t testing and catch some regressions. Most of the i-g-t bugs are from HSW+, so I hope keep focus on these new platforms. but now we don't have enough free machine resource (such as BYT,BDW)to support one machine only run i-g-t in nightly. Wrt analyzing issues I think the right approach for moving forward is: a) switch to piglit to run tests, not just enumerate them. This will allow QA and developers to share testcase analysis. [He, Shuang] Yes, though this could not actually accelerate the test. We could directly wrap over piglit to run testing (have other control process to monitor and collecting test results) [Guang YANG] Yeah, Shuang said is what we did. Piglit have been improved more powerful, but our infrastructure have better remote control and result collecting. If it will be comfortable for Developers to see the case result from running piglit, we can discuss how to match these two framework together. b) add automated analysis for time-consuming and error prone cases like dmesg warnings and backtraces. ThomasI have just discussed a few ideas in this are in our 1:1 today. Reducing the set of igt tests we run is imo pointless: The goal of igt is to hit corner-cases, arbitrarily selecting which kinds of corner-cases we test just means that we have a nice illusion about our test coverage. [He, Shuang] I don't think select a subset of test cases to run is pointless. It's a trade-off between speed and correctness. For our nightly testing it's not so useful to run only a small set of testing. But for fast sanity testing, it should be easier, which is supposed to catch regression in major/critical functionality (So other developers and QA could continue their work). Adding more people to the discussion. Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
