Re: [Intel-gfx] [PATCH] drm/i915: Fixing eDP detection on certain platforms
On Tue, 2016-04-12 at 12:23 +0530, Shubhangi Shrivastava wrote:
> Since commit 30d9aa4265fe ("drm/i915: Read sink_count dpcd always"),
> the status of a DP connector depends on its sink count value.
> However, some eDP panels don't set that value appropriately,
> causing them to be reported as disconnected.
> Fix this by ignoring sink count for eDP.
>
> v2: Rephrased commit message. (Ander)
> In case of eDP, returning status as connected if DPCD
> read succeeds to avoid any further operations.
>
> Fixes: 30d9aa4265fe ("drm/i915: Read sink_count dpcd always")
> Cc: Ander Conselvan De Oliveira
> Cc: Jani Nikula
> Reported-by: Tvrtko Ursulin
> Signed-off-by: Sivakumar Thulasimani
> Signed-off-by: Shubhangi Shrivastava
> Tested-by: Tvrtko Ursulin
Reviewed-by: Ander Conselvan de Oliveira
> ---
> drivers/gpu/drm/i915/intel_dp.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d2..bdc7e12 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3806,7 +3806,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>* downstream port information. So, an early return here saves
>* time from performing other operations which are not required.
>*/
> - if (!intel_dp->sink_count)
> + if (!is_edp(intel_dp) && !intel_dp->sink_count)
> return false;
>
> /* Check if the panel supports PSR */
> @@ -4339,6 +4339,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> if (!intel_dp_get_dpcd(intel_dp))
> return connector_status_disconnected;
>
> + if (is_edp(intel_dp))
> + return connector_status_connected;
> +
> /* if there's no downstream port, we're done */
> if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> return connector_status_connected;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()
On Mon, 2016-04-11 at 10:11 -0700, Jim Bride wrote:
> In commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") some
> much needed clean-up was done, but unfortunately part of the change
> broke DP MST. The real issue was setting the connector state to
> disconnected in the MST case, which is good, but the code then (after
> a goto) checks if the connector state is not connected and shuts down
> MST if this is the case, which is bad. With this change both SST and
> MST seem to be happy.
>
> v2: Add removed check further up in the function to be sure that MST
> is shut down when we lose the link. (Ander)
>
> Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> cc: Sivakumar Thulasimani
> cc: Shubhangi Shrivastava
> cc: Ander Conselvan de Oliveira
> cc: Nathan D Ciobanu
> Signed-off-by: Jim Bride
Reviewed-by: Ander Conselvan de Oliveira
> ---
> drivers/gpu/drm/i915/intel_dp.c | 24 +++-
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d2..31b222a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4608,6 +4608,15 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> intel_dp->compliance_test_type = 0;
> intel_dp->compliance_test_data = 0;
>
> + if (intel_dp->is_mst) {
> + DRM_DEBUG_KMS("MST device may have disappeared %d vs
> %d\n",
> + intel_dp->is_mst,
> + intel_dp->mst_mgr.mst_state);
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> + intel_dp->is_mst);
> + }
> +
> goto out;
> }
>
> @@ -4665,20 +4674,9 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> }
>
> out:
> - if (status != connector_status_connected) {
> + if ((status != connector_status_connected) &&
> + (intel_dp->is_mst == false))
> intel_dp_unset_edid(intel_dp);
> - /*
> - * If we were in MST mode, and device is not there,
> - * get out of MST mode
> - */
> - if (intel_dp->is_mst) {
> - DRM_DEBUG_KMS("MST device may have disappeared %d vs
> %d\n",
> - intel_dp->is_mst, intel_dp
> ->mst_mgr.mst_state);
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> - intel_dp->is_mst);
> - }
> - }
>
> intel_display_power_put(to_i915(dev), power_domain);
> return;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 3/6] drm/i915/guc: refactor doorbell management code
On 07/04/16 22:26, Yu Dai wrote:
On 04/07/2016 10:21 AM, Dave Gordon wrote:
During a hibernate/resume cycle, the driver, the GuC, and the doorbell
hardware can end up in inconsistent states. This patch refactors the
driver's handling and tracking of doorbells, in preparation for a later
one which will resolve the issue.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/i915_guc_submission.c | 88
++
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2171759..2fc69f1 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -175,8 +175,48 @@ static int host2guc_sample_forcewake(struct
intel_guc *guc,
* client object which contains the page being used for the doorbell
*/
+static int guc_update_doorbell_id(struct i915_guc_client *client,
+ struct guc_doorbell_info *doorbell,
+ u16 new_id)
+{
+struct sg_table *sg = client->guc->ctx_pool_obj->pages;
+void *doorbell_bitmap = client->guc->doorbell_bitmap;
+struct guc_context_desc desc;
+size_t len;
+
+if (client->doorbell_id != GUC_INVALID_DOORBELL_ID &&
+test_bit(client->doorbell_id, doorbell_bitmap)) {
+/* Deactivate the old doorbell */
+doorbell->db_status = GUC_DOORBELL_DISABLED;
+(void)host2guc_release_doorbell(client->guc, client);
+clear_bit(client->doorbell_id, doorbell_bitmap);
+}
+
+/* Update the GuC's idea of the doorbell ID */
+len = sg_pcopy_to_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
+ sizeof(desc) * client->ctx_index);
+if (len != sizeof(desc))
+return -EFAULT;
+desc.db_id = new_id;
+len = sg_pcopy_from_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
+ sizeof(desc) * client->ctx_index);
+if (len != sizeof(desc))
+return -EFAULT;
+
We may cache the vmap of context pool for its life cycle to avoid these
copies. That is why a generic vmap helper function is really nice to have.
Alex
Yes, I'm hoping we'll make some progress with that now that Chris has
merged some new vmap code.
Meanwhile can you please review all of the patches in this series?
Thanks,
.Dave.
+client->doorbell_id = new_id;
+if (new_id == GUC_INVALID_DOORBELL_ID)
+return 0;
+
+/* Activate the new doorbell */
+set_bit(client->doorbell_id, doorbell_bitmap);
+doorbell->db_status = GUC_DOORBELL_ENABLED;
+doorbell->cookie = 0;
+return host2guc_allocate_doorbell(client->guc, client);
+}
+
static void guc_init_doorbell(struct intel_guc *guc,
- struct i915_guc_client *client)
+ struct i915_guc_client *client,
+ uint16_t db_id)
{
struct guc_doorbell_info *doorbell;
void *base;
@@ -184,8 +224,7 @@ static void guc_init_doorbell(struct intel_guc *guc,
base = kmap_atomic(i915_gem_object_get_page(client->client_obj,
0));
doorbell = base + client->doorbell_offset;
-doorbell->db_status = 1;
-doorbell->cookie = 0;
+guc_update_doorbell_id(client, doorbell, db_id);
kunmap_atomic(base);
}
@@ -193,27 +232,16 @@ static void guc_init_doorbell(struct intel_guc
*guc,
static void guc_disable_doorbell(struct intel_guc *guc,
struct i915_guc_client *client)
{
-struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct guc_doorbell_info *doorbell;
void *base;
-i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id);
-int value;
base = kmap_atomic(i915_gem_object_get_page(client->client_obj,
0));
doorbell = base + client->doorbell_offset;
-doorbell->db_status = 0;
+guc_update_doorbell_id(client, doorbell, GUC_INVALID_DOORBELL_ID);
kunmap_atomic(base);
-I915_WRITE(drbreg, I915_READ(drbreg) & ~GEN8_DRB_VALID);
-
-value = I915_READ(drbreg);
-WARN_ON((value & GEN8_DRB_VALID) != 0);
-
-I915_WRITE(GEN8_DRBREGU(client->doorbell_id), 0);
-I915_WRITE(drbreg, 0);
-
/* XXX: wait for any interrupts */
/* XXX: wait for workqueue to drain */
}
@@ -260,7 +288,7 @@ static uint16_t assign_doorbell(struct intel_guc
*guc, uint32_t priority)
if (id == end)
id = GUC_INVALID_DOORBELL_ID;
else
-bitmap_set(guc->doorbell_bitmap, id, 1);
+set_bit(id, guc->doorbell_bitmap);
DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
hi_pri ? "high" : "normal", id);
@@ -268,11 +296,6 @@ static uint16_t assign_doorbell(struct intel_guc
*guc, uint32_t priority)
return id;
}
-static void release_doorbell(struct intel_guc *guc, uint16_t id)
-{
-bitmap_clear(guc->doorbell_bitmap, id, 1);
-}
-
/*
* Initialise the process descriptor shared with the GuC firmware.
*/
@@ -667,15 +690,7 @@ static void guc_client_free(struct drm_device *dev,
if (!client)
Re: [Intel-gfx] [PATCH v2 4/6] drm/i915/shrinker: Restrict vmap purge to objects with vmaps
On 11/04/16 17:40, Chris Wilson wrote: On Mon, Apr 11, 2016 at 03:57:21PM +0100, Tvrtko Ursulin wrote: On 11/04/16 15:44, Chris Wilson wrote: On Mon, Apr 11, 2016 at 03:25:41PM +0100, Tvrtko Ursulin wrote: On 08/04/16 12:11, Chris Wilson wrote: When called because we have run out of vmap address space, we only need to recover objects that have vmappings and not all. v2: Start using is_vmalloc_addr() Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 +- 2 files changed, 10 insertions(+), 1 deletion(-) Reviewed-by: Tvrtko Ursulin Having started to use the obj->mapping cache in anger, do you think there is any left before pushing this part of the puzzle? I'm hitting an interesting scaling problem with vmalloc's lazy free list (that's a patch for later!). Looks ready and useful to me so I vote for merging it. Done. Hopefully I can make some headway on bugfixes and then start chasing some of the more interesting improvements. -Chris Yay! Now we'll try using it for the various GuC objects that could do with a long-term mapping rather than repeated kmap_atomic() :) .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse() (rev3)
== Series Details == Series: drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse() (rev3) URL : https://patchwork.freedesktop.org/series/5390/ State : failure == Summary == Series 5390v3 drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse() http://patchwork.freedesktop.org/api/1.0/series/5390/revisions/3/mbox/ Test gem_basic: Subgroup create-fd-close: incomplete -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup invalid-param-get: incomplete -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (bsw-nuc-2) Test gem_storedw_loop: Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup bad-pitch-1024: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-y-tiled: incomplete -> PASS (bsw-nuc-2) Subgroup small-bo: incomplete -> PASS (bsw-nuc-2) Subgroup unused-handle: incomplete -> PASS (bsw-nuc-2) Test kms_force_connector_basic: Subgroup prune-stale-modes: pass -> SKIP (snb-x220t) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: incomplete -> SKIP (bsw-nuc-2) Subgroup read-crc-pipe-b: incomplete -> SKIP (bsw-nuc-2) Test prime_self_import: Subgroup basic-with_one_bo: incomplete -> PASS (bsw-nuc-2) Subgroup basic-with_two_bos: incomplete -> PASS (bsw-nuc-2) bdw-nuci7total:202 pass:190 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:202 pass:179 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:201 pass:162 dwarn:0 dfail:0 fail:0 skip:39 byt-nuc total:201 pass:163 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:202 pass:178 dwarn:0 dfail:0 fail:0 skip:24 ilk-hp8440p total:202 pass:134 dwarn:0 dfail:0 fail:0 skip:68 ivb-t430stotal:202 pass:174 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:202 pass:177 dwarn:0 dfail:0 fail:0 skip:25 snb-x220ttotal:202 pass:163 dwarn:0 dfail:0 fail:1 skip:38 BOOT FAILED for snb-dellxps Results at /archive/results/CI_IGT_test/Patchwork_1864/ dc5380b5263ebb0bf251bb09db542585702b528b drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest e9b4dd341189637119d15df79a72a9a1f53c62fa drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse() ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Fixing up relocation of secure buffers?
== Series Details == Series: Fixing up relocation of secure buffers? URL : https://patchwork.freedesktop.org/series/5550/ State : failure == Summary == Series 5550v1 Fixing up relocation of secure buffers? http://patchwork.freedesktop.org/api/1.0/series/5550/revisions/1/mbox/ Test gem_basic: Subgroup create-fd-close: incomplete -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup invalid-param-get: incomplete -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (bsw-nuc-2) Test gem_storedw_loop: Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup bad-pitch-1024: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-y-tiled: incomplete -> PASS (bsw-nuc-2) Subgroup small-bo: incomplete -> PASS (bsw-nuc-2) Subgroup unused-handle: incomplete -> PASS (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (skl-i7k-2) Test kms_force_connector_basic: Subgroup prune-stale-modes: pass -> SKIP (snb-x220t) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: incomplete -> SKIP (bsw-nuc-2) Subgroup read-crc-pipe-b: incomplete -> SKIP (bsw-nuc-2) Test prime_self_import: Subgroup basic-with_one_bo: incomplete -> PASS (bsw-nuc-2) Subgroup basic-with_two_bos: incomplete -> PASS (bsw-nuc-2) bdw-nuci7total:202 pass:190 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:202 pass:179 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:201 pass:162 dwarn:0 dfail:0 fail:0 skip:39 byt-nuc total:201 pass:163 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:202 pass:178 dwarn:0 dfail:0 fail:0 skip:24 ivb-t430stotal:202 pass:174 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:202 pass:176 dwarn:1 dfail:0 fail:0 skip:25 snb-x220ttotal:202 pass:163 dwarn:0 dfail:0 fail:1 skip:38 BOOT FAILED for snb-dellxps Results at /archive/results/CI_IGT_test/Patchwork_1865/ dc5380b5263ebb0bf251bb09db542585702b528b drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest fa9c5a5d8ccb1854306ff2be8d94ae951da8b7a8 Fixing up relocation of secure buffers? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Prevent machine death on Ivybridge context switching (rev2)
== Series Details == Series: series starting with [1/4] drm/i915: Prevent machine death on Ivybridge context switching (rev2) URL : https://patchwork.freedesktop.org/series/5484/ State : warning == Summary == Series 5484v2 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/5484/revisions/2/mbox/ Test gem_basic: Subgroup create-fd-close: incomplete -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup invalid-param-get: incomplete -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (bsw-nuc-2) Test gem_storedw_loop: Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup bad-pitch-1024: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-y-tiled: incomplete -> PASS (bsw-nuc-2) Subgroup small-bo: incomplete -> PASS (bsw-nuc-2) Subgroup unused-handle: incomplete -> PASS (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (skl-i7k-2) Test kms_force_connector_basic: Subgroup force-load-detect: pass -> SKIP (ilk-hp8440p) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: incomplete -> PASS (snb-dellxps) Subgroup read-crc-pipe-a: incomplete -> SKIP (bsw-nuc-2) Subgroup read-crc-pipe-b: incomplete -> SKIP (bsw-nuc-2) Test pm_rpm: Subgroup basic-rte: pass -> DMESG-WARN (byt-nuc) UNSTABLE Test prime_self_import: Subgroup basic-with_one_bo: incomplete -> PASS (bsw-nuc-2) Subgroup basic-with_two_bos: incomplete -> PASS (bsw-nuc-2) bdw-nuci7total:202 pass:190 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:202 pass:179 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:201 pass:162 dwarn:0 dfail:0 fail:0 skip:39 byt-nuc total:201 pass:162 dwarn:1 dfail:0 fail:0 skip:38 hsw-brixbox total:202 pass:178 dwarn:0 dfail:0 fail:0 skip:24 ilk-hp8440p total:202 pass:133 dwarn:0 dfail:0 fail:0 skip:69 ivb-t430stotal:202 pass:174 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:202 pass:176 dwarn:1 dfail:0 fail:0 skip:25 snb-dellxps total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 snb-x220ttotal:202 pass:164 dwarn:0 dfail:0 fail:1 skip:37 Results at /archive/results/CI_IGT_test/Patchwork_1866/ dc5380b5263ebb0bf251bb09db542585702b528b drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest 82c52bb3831f06caddbb6f10ebaefa5e2dbdc44b drm/i915: Reorganise legacy context switch to cope with late failure 36dc69a8bc7e9f48edd60d5120416afa5973d516 drm/i915: Move the mb() following release-mmap into release-mmap 508a7170e74f9eb1436c7c05d601218ce9fb9bf4 drm/i915: Force ringbuffers to not be at offset 0 bd468dc213bc0f4d596bdc6215aedc3a0751dc15 drm/i915: Prevent machine death on Ivybridge context switching ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Try to shut up more ILK underruns
On Fri, Apr 01, 2016 at 09:53:17PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Take a bigger hammer to the underrun suppression on ILK. Instead of
> trying to suppress them at specific points in the modeset sequence just
> silence them across the entire sequence. This gets rid of some underruns
> at least on my ILK. Note that this changes SNB and IVB to follow the
> same approach just to keep the code less convoluted. The difference is
> that on those platforms we won't suppress CPU underruns for port A since
> it doesn't seem to be necessary.
>
> My ILK has port A eDP and two PCH HDMI ports, so I can't be sure this is
> as effective on other PCH port types. Perhaps we still need some of
> Daniel's extra vblank waits [2]?
>
> I've still been able to trigger an underrun on the other pipe, but
> fixing that perhaps needs the LP1+ disable trick I implemented here [1]
> which never got merged.
>
> A few details which hamper stress testing on my ILK are that sometimes
> the PCH transcoder gets messed up and refuses to shut down, and sometimes
> even the panel power sequencer apparently gets stuck on the always on
> position.
>
> [1] https://lists.freedesktop.org/archives/intel-gfx/2014-March/041317.html
> [2] https://lists.freedesktop.org/archives/intel-gfx/2016-January/086397.html
>
> v2: Add a note that we also get underruns when enabling PCH ports
>
> Cc: Daniel Vetter
> Signed-off-by: Ville Syrjälä
> Reviewed-by: Daniel Vetter (v1)
I've not been able to find any additional ILK hardware to test this on but LGTM
Reviewed-by: Patrik Jakobsson
> ---
> drivers/gpu/drm/i915/intel_display.c | 45
> ++--
> drivers/gpu/drm/i915/intel_dp.c | 12 --
> 2 files changed, 23 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e6b5ee51739b..8d2c547b57ee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4083,12 +4083,6 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> I915_WRITE(FDI_RX_TUSIZE1(pipe),
> I915_READ(PIPE_DATA_M1(pipe)) & TU_SIZE_MASK);
>
> - /*
> - * Sometimes spurious CPU pipe underruns happen during FDI
> - * training, at least with VGA+HDMI cloning. Suppress them.
> - */
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> -
> /* For PCH output, training FDI link */
> dev_priv->display.fdi_link_train(crtc);
>
> @@ -4123,8 +4117,6 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>
> intel_fdi_normal_train(crtc);
>
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -
> /* For PCH DP, enable TRANS_DP_CTL */
> if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
> const struct drm_display_mode *adjusted_mode =
> @@ -4727,6 +4719,18 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> if (WARN_ON(intel_crtc->active))
> return;
>
> + /*
> + * Sometimes spurious CPU pipe underruns happen during FDI
> + * training, at least with VGA+HDMI cloning. Suppress them.
> + *
> + * On ILK we get an occasional spurious CPU pipe underruns
> + * between eDP port A enable and vdd enable. Also PCH port
> + * enable seems to result in the occasional CPU pipe underrun.
> + *
> + * Spurious PCH underruns also occur during PCH enabling.
> + */
> + if (intel_crtc->config->has_pch_encoder || IS_GEN5(dev_priv))
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> if (intel_crtc->config->has_pch_encoder)
> intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>
> @@ -4748,8 +4752,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>
> intel_crtc->active = true;
>
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -
> for_each_encoder_on_crtc(dev, crtc, encoder)
> if (encoder->pre_enable)
> encoder->pre_enable(encoder);
> @@ -4791,6 +4793,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> if (intel_crtc->config->has_pch_encoder)
> intel_wait_for_vblank(dev, pipe);
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> }
>
> @@ -4943,8 +4946,15 @@ static void ironlake_crtc_disable(struct drm_crtc
> *crtc)
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
>
> - if (intel_crtc->config->has_pch_encoder)
> + /*
> + * Sometimes spurious CPU pipe underruns happen when the
> + * pipe is already disabled, but FDI RX/TX is still enabled.
> + * Happens at least with VGA+HDMI cloning. Suppress them.
> + */
> +
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Replace ILK eDP underrun suppression with something better
On Fri, Apr 01, 2016 at 09:53:19PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The underruns we were seeing when enabling eDP port A on ILK seem to
> have been caused by prematurely clearing the LP1+ watermark values when
> disabling LP1+ watermarks. Now that the watermarks are handled
> properly, we can rip out the underrun suppression around the port A
> enable.
>
> We still need to worry about the underruns on FDI when enabling
> the eDP PLL. But as Bspec tells us, we can avoid that by a vblank
> wait on the pipe driving FDI just prior to enabling the eDP PLL.
>
> Cc: Daniel Vetter
> Signed-off-by: Ville Syrjälä
Reviewed-by: Patrik Jakobsson
> ---
> drivers/gpu/drm/i915/intel_dp.c | 36 +---
> 1 file changed, 9 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 95fe01d55bce..7523558190d1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2215,6 +2215,15 @@ static void ironlake_edp_pll_on(struct intel_dp
> *intel_dp)
> POSTING_READ(DP_A);
> udelay(500);
>
> + /*
> + * [DevILK] Work around required when enabling DP PLL
> + * while a pipe is enabled going to FDI:
> + * 1. Wait for the start of vertical blank on the enabled pipe going to
> FDI
> + * 2. Program DP PLL enable
> + */
> + if (IS_GEN5(dev_priv))
> + intel_wait_for_vblank_if_active(dev_priv->dev, !crtc->pipe);
> +
> intel_dp->DP |= DP_PLL_ENABLE;
>
> I915_WRITE(DP_A, intel_dp->DP);
> @@ -2630,7 +2639,6 @@ static void intel_enable_dp(struct intel_encoder
> *encoder)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> uint32_t dp_reg = I915_READ(intel_dp->output_reg);
> - enum port port = dp_to_dig_port(intel_dp)->port;
> enum pipe pipe = crtc->pipe;
>
> if (WARN_ON(dp_reg & DP_PORT_EN))
> @@ -2643,17 +2651,6 @@ static void intel_enable_dp(struct intel_encoder
> *encoder)
>
> intel_dp_enable_port(intel_dp);
>
> - if (port == PORT_A && IS_GEN5(dev_priv)) {
> - /*
> - * Underrun reporting for the other pipe was disabled in
> - * g4x_pre_enable_dp(). The eDP PLL and port have now been
> - * enabled, so it's now safe to re-enable underrun reporting.
> - */
> - intel_wait_for_vblank_if_active(dev_priv->dev, !pipe);
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, !pipe, true);
> - intel_set_pch_fifo_underrun_reporting(dev_priv, !pipe, true);
> - }
> -
> edp_panel_vdd_on(intel_dp);
> edp_panel_on(intel_dp);
> edp_panel_vdd_off(intel_dp, true);
> @@ -2699,26 +2696,11 @@ static void vlv_enable_dp(struct intel_encoder
> *encoder)
>
> static void g4x_pre_enable_dp(struct intel_encoder *encoder)
> {
> - struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> enum port port = dp_to_dig_port(intel_dp)->port;
> - enum pipe pipe = to_intel_crtc(encoder->base.crtc)->pipe;
>
> intel_dp_prepare(encoder);
>
> - if (port == PORT_A && IS_GEN5(dev_priv)) {
> - /*
> - * We get FIFO underruns on the other pipe when
> - * enabling the CPU eDP PLL, and when enabling CPU
> - * eDP port. We could potentially avoid the PLL
> - * underrun with a vblank wait just prior to enabling
> - * the PLL, but that doesn't appear to help the port
> - * enable case. Just sweep it all under the rug.
> - */
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, !pipe, false);
> - intel_set_pch_fifo_underrun_reporting(dev_priv, !pipe, false);
> - }
> -
> /* Only ilk+ has port A */
> if (port == PORT_A)
> ironlake_edp_pll_on(intel_dp);
> --
> 2.7.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm,
Sweden Registration Number: 556189-6027
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Fixing eDP detection on certain platforms (rev4)
== Series Details == Series: drm/i915: Fixing eDP detection on certain platforms (rev4) URL : https://patchwork.freedesktop.org/series/5408/ State : failure == Summary == Series 5408v4 drm/i915: Fixing eDP detection on certain platforms http://patchwork.freedesktop.org/api/1.0/series/5408/revisions/4/mbox/ Test drv_module_reload_basic: pass -> INCOMPLETE (snb-dellxps) Test gem_basic: Subgroup create-fd-close: incomplete -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup invalid-param-get: incomplete -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (bsw-nuc-2) Test gem_storedw_loop: Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup bad-pitch-1024: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-y-tiled: incomplete -> PASS (bsw-nuc-2) Subgroup small-bo: incomplete -> PASS (bsw-nuc-2) Subgroup unused-handle: incomplete -> PASS (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (bsw-nuc-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: incomplete -> SKIP (bsw-nuc-2) Subgroup read-crc-pipe-b: incomplete -> SKIP (bsw-nuc-2) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> DMESG-WARN (byt-nuc) Test prime_self_import: Subgroup basic-with_one_bo: incomplete -> PASS (bsw-nuc-2) Subgroup basic-with_two_bos: incomplete -> PASS (bsw-nuc-2) bdw-nuci7total:202 pass:190 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:202 pass:179 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:201 pass:161 dwarn:0 dfail:0 fail:1 skip:39 byt-nuc total:201 pass:162 dwarn:1 dfail:0 fail:0 skip:38 hsw-brixbox total:202 pass:178 dwarn:0 dfail:0 fail:0 skip:24 ilk-hp8440p total:202 pass:133 dwarn:1 dfail:0 fail:0 skip:68 ivb-t430stotal:202 pass:174 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:202 pass:177 dwarn:0 dfail:0 fail:0 skip:25 snb-dellxps total:75 pass:59 dwarn:0 dfail:0 fail:1 skip:14 snb-x220ttotal:202 pass:164 dwarn:0 dfail:0 fail:1 skip:37 Results at /archive/results/CI_IGT_test/Patchwork_1867/ dc5380b5263ebb0bf251bb09db542585702b528b drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest 09154b99a2f86c5ae5061f82dee7f1cc8953a847 drm/i915: Fixing eDP detection on certain platforms ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
Hi Mark, On Tue, 12 Apr 2016 05:42:03 +0100 Mark Brown wrote: > On Thu, Apr 07, 2016 at 11:54:31PM +0200, Boris Brezillon wrote: > > > Is there any reason for calling set_machine_constraints() after > > device_register() in regulator_register()? > > I'm not sure there's a strong one, we don't really use the class device > for anything, but without doing a full audit I couldn't guarantee that. At first glance I don't see any problem (even the rdev_err/info/...() functions do not use dev_err/info/...()). The patch will be part of v6 (unless you want me to send it independently). Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Use new i915_gem_object_pin_map for LRC
From: Tvrtko Ursulin
We can use the new pin/lazy unpin API for simplicity
and more performance in the execlist submission paths.
v2:
* Fix error handling and convert more users.
* Compact some names for readability.
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c| 98 ++---
drivers/gpu/drm/i915/intel_lrc.h| 7 ++-
3 files changed, 60 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index fe580cb9501a..91028d9c6269 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -342,7 +342,7 @@ void i915_gem_context_reset(struct drm_device *dev)
struct intel_context *ctx;
list_for_each_entry(ctx, &dev_priv->context_list, link)
- intel_lr_context_reset(dev, ctx);
+ intel_lr_context_reset(dev_priv, ctx);
}
for (i = 0; i < I915_NUM_ENGINES; i++) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0d6dc5ec4a46..0c61d847252a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -229,8 +229,9 @@ enum {
static int intel_lr_context_pin(struct intel_context *ctx,
struct intel_engine_cs *engine);
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
- struct drm_i915_gem_object
*default_ctx_obj);
+static int
+lrc_setup_hws(struct intel_engine_cs *engine,
+ struct drm_i915_gem_object *default_ctx_obj);
/**
@@ -1093,8 +1094,8 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
struct intel_ringbuffer *ringbuf = ctx->engine[engine->id].ringbuf;
- struct page *lrc_state_page;
- uint32_t *lrc_reg_state;
+ void *obj_addr;
+ u32 *lrc_reg_state;
int ret;
WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
@@ -1104,19 +1105,20 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
if (ret)
return ret;
- lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
- if (WARN_ON(!lrc_state_page)) {
- ret = -ENODEV;
+ obj_addr = i915_gem_object_pin_map(ctx_obj);
+ if (IS_ERR(obj_addr)) {
+ ret = PTR_ERR(obj_addr);
goto unpin_ctx_obj;
}
+ lrc_reg_state = obj_addr + LRC_STATE_PN * PAGE_SIZE;
+
ret = intel_pin_and_map_ringbuffer_obj(engine->dev, ringbuf);
if (ret)
- goto unpin_ctx_obj;
+ goto unpin_map;
ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, engine);
- lrc_reg_state = kmap(lrc_state_page);
lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
ctx_obj->dirty = true;
@@ -1127,6 +1129,8 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
return ret;
+unpin_map:
+ i915_gem_object_unpin_map(ctx_obj);
unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
@@ -1159,7 +1163,7 @@ void intel_lr_context_unpin(struct intel_context *ctx,
WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
if (--ctx->engine[engine->id].pin_count == 0) {
- kunmap(kmap_to_page(ctx->engine[engine->id].lrc_reg_state));
+ i915_gem_object_unpin_map(ctx_obj);
intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
ctx->engine[engine->id].lrc_vma = NULL;
@@ -1584,9 +1588,12 @@ static int gen8_init_common_ring(struct intel_engine_cs
*engine)
struct drm_device *dev = engine->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned int next_context_status_buffer_hw;
+ int ret;
- lrc_setup_hardware_status_page(engine,
-
dev_priv->kernel_context->engine[engine->id].state);
+ ret = lrc_setup_hws(engine,
+ dev_priv->kernel_context->engine[engine->id].state);
+ if (ret)
+ return ret;
I915_WRITE_IMR(engine,
~(engine->irq_enable_mask | engine->irq_keep_mask));
@@ -2048,7 +2055,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
*engine)
i915_gem_batch_pool_fini(&engine->batch_pool);
if (engine->status_page.obj) {
- kunmap(sg_page(engine->status_page.obj->pages->sgl));
+ i915_gem_object_unpin_ma
[Intel-gfx] ✓ Fi.CI.BAT: success for mm/vmalloc: Keep a separate lazy-free list
== Series Details == Series: mm/vmalloc: Keep a separate lazy-free list URL : https://patchwork.freedesktop.org/series/5574/ State : success == Summary == Series 5574v1 mm/vmalloc: Keep a separate lazy-free list http://patchwork.freedesktop.org/api/1.0/series/5574/revisions/1/mbox/ Test gem_basic: Subgroup create-fd-close: incomplete -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup invalid-param-get: incomplete -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (bsw-nuc-2) Test gem_storedw_loop: Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup bad-pitch-1024: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-y-tiled: incomplete -> PASS (bsw-nuc-2) Subgroup small-bo: incomplete -> PASS (bsw-nuc-2) Subgroup unused-handle: incomplete -> PASS (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Test kms_force_connector_basic: Subgroup force-load-detect: pass -> SKIP (ilk-hp8440p) Subgroup prune-stale-modes: pass -> SKIP (ilk-hp8440p) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: incomplete -> PASS (snb-dellxps) Subgroup read-crc-pipe-a: incomplete -> SKIP (bsw-nuc-2) Subgroup read-crc-pipe-b: incomplete -> SKIP (bsw-nuc-2) Test prime_self_import: Subgroup basic-with_one_bo: incomplete -> PASS (bsw-nuc-2) Subgroup basic-with_two_bos: incomplete -> PASS (bsw-nuc-2) bdw-nuci7total:202 pass:190 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:202 pass:179 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:201 pass:162 dwarn:0 dfail:0 fail:0 skip:39 byt-nuc total:201 pass:163 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:202 pass:178 dwarn:0 dfail:0 fail:0 skip:24 hsw-gt2 total:202 pass:183 dwarn:0 dfail:0 fail:0 skip:19 ilk-hp8440p total:202 pass:131 dwarn:1 dfail:0 fail:0 skip:70 ivb-t430stotal:202 pass:174 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:202 pass:177 dwarn:0 dfail:0 fail:0 skip:25 skl-nuci5total:202 pass:191 dwarn:0 dfail:0 fail:0 skip:11 snb-dellxps total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 snb-x220ttotal:202 pass:164 dwarn:0 dfail:0 fail:1 skip:37 Results at /archive/results/CI_IGT_test/Patchwork_1868/ dc5380b5263ebb0bf251bb09db542585702b528b drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest 455e2ff mm/vmalloc: Keep a separate lazy-free list ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Fix up vlv/chv display irq setup
On Mon, Apr 11, 2016 at 07:29:13PM +0300, Imre Deak wrote:
> On ma, 2016-04-11 at 16:56 +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > The vlv/chv display irq setup was a bit of mess after I ran out of steam
> > when working on it last. Fix it up so that we just have a _reset() and
> > _postinstall() hooks for the display irqs, and use those consistently.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 102
> > ++--
> > 1 file changed, 24 insertions(+), 78 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 1d21ebfffd4d..a1239fedc086 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3306,13 +3306,15 @@ static void vlv_display_irq_reset(struct
> > drm_i915_private *dev_priv)
> > {
> > enum pipe pipe;
> >
> > - i915_hotplug_interrupt_update(dev_priv, 0x, 0);
> > + i915_hotplug_interrupt_update_locked(dev_priv, 0x, 0);
> > I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
> >
> > for_each_pipe(dev_priv, pipe)
> > I915_WRITE(PIPESTAT(pipe), 0x);
>
> Since vlv_display_irq_reset() will be used in place
> of valleyview_display_irqs_uninstall()/i915_disable_pipestat()
> we'll leave now stale bits in pipestat_irq_mask[pipe]. It's not a
> problem since display_irqs_enabled effectively masks these bits, but
> for consistency I'd clear them.
OTOH we can't mask PIPESTAT bits, so even if we clear them here, it's
very likely some of the bits will be set again by the time we actually
enable an interrupt.
In any case, I think I'll be posting a patch to clean up the PIPESTAT
clearing/disabling acrosss the board. It's a bit of a mess right now,
with each platform doing things slightly differently.
>
> The same goes for clearing PIPE_FIFO_UNDERRUN_STATUS.
>
> With that:
> Reviewed-by: Imre Deak
>
> >
> > GEN5_IRQ_RESET(VLV_);
> > +
> > + dev_priv->irq_mask = ~0;
> > }
>
>
>
> >
> > static void valleyview_irq_preinstall(struct drm_device *dev)
> > @@ -3323,7 +3325,9 @@ static void valleyview_irq_preinstall(struct
> > drm_device *dev)
> >
> > I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
> >
> > + spin_lock_irq(&dev_priv->irq_lock);
> > vlv_display_irq_reset(dev_priv);
> > + spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
> > @@ -3398,7 +3402,9 @@ static void cherryview_irq_preinstall(struct
> > drm_device *dev)
> >
> > I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
> >
> > + spin_lock_irq(&dev_priv->irq_lock);
> > vlv_display_irq_reset(dev_priv);
> > + spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
> > @@ -3645,7 +3651,7 @@ static int ironlake_irq_postinstall(struct drm_device
> > *dev)
> > return 0;
> > }
> >
> > -static void valleyview_display_irqs_install(struct drm_i915_private
> > *dev_priv)
> > +static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
> > {
> > u32 pipestat_mask;
> > u32 iir_mask;
> > @@ -3679,40 +3685,6 @@ static void valleyview_display_irqs_install(struct
> > drm_i915_private *dev_priv)
> > POSTING_READ(VLV_IMR);
> > }
> >
> > -static void valleyview_display_irqs_uninstall(struct drm_i915_private
> > *dev_priv)
> > -{
> > - u32 pipestat_mask;
> > - u32 iir_mask;
> > - enum pipe pipe;
> > -
> > - iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > - I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > - if (IS_CHERRYVIEW(dev_priv))
> > - iir_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
> > -
> > - dev_priv->irq_mask |= iir_mask;
> > - I915_WRITE(VLV_IMR, dev_priv->irq_mask);
> > - I915_WRITE(VLV_IER, ~dev_priv->irq_mask);
> > - I915_WRITE(VLV_IIR, iir_mask);
> > - I915_WRITE(VLV_IIR, iir_mask);
> > - POSTING_READ(VLV_IIR);
> > -
> > - pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV |
> > - PIPE_CRC_DONE_INTERRUPT_STATUS;
> > -
> > - i915_disable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
> > - for_each_pipe(dev_priv, pipe)
> > - i915_disable_pipestat(dev_priv, pipe, pipestat_mask);
> > -
> > - pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> > - PIPE_FIFO_UNDERRUN_STATUS;
> > -
> > - for_each_pipe(dev_priv, pipe)
> > - I915_WRITE(PIPESTAT(pipe), pipestat_mask);
> > - POSTING_READ(PIPESTAT(PIPE_A));
> > -}
> > -
> > void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
> > {
> > assert_spin_locked(&dev_priv->irq_lock);
> > @@ -3723,7 +3695,7 @@ void valleyview_enable_display_irqs(struct
> > drm_i915_private *dev_priv)
> > dev_priv->display_irqs_enabled = true;
> >
> > if (intel_irqs_enabled(dev_priv
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Make sure LP1+ watermarks levels are preserved when going from 1 to 2 pipes
On Fri, Apr 01, 2016 at 09:53:18PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Once again ILK is unhappy if we clear out the LP1+ watermark levels > outright, and instead we must disable the levels we don't want while > still leaving the actual programmed watermark levels intact. > > Fixes underruns on the already enabled pipe when programming watermarks > while enabling the second pipe. > > Cc: Daniel Vetter > Cc: Matt Roper > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93787 > Signed-off-by: Ville Syrjälä Reviewed-by: Patrik Jakobsson > --- > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 9bc9c25423e9..a7fd5d464838 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2483,7 +2483,7 @@ static void ilk_wm_merge(struct drm_device *dev, > /* ILK/SNB/IVB: LP1+ watermarks only w/ single pipe */ > if ((INTEL_INFO(dev)->gen <= 6 || IS_IVYBRIDGE(dev)) && > config->num_pipes_active > 1) > - return; > + last_enabled_level = 0; > > /* ILK: FBC WM must be disabled always */ > merged->fbc_wm_enabled = INTEL_INFO(dev)->gen >= 6; > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Updated drm-intel-testing
On Mon, Apr 11, 2016 at 09:45:11PM +0200, Daniel Vetter wrote: > Hi all, > > New -testing cycle with cool stuff: > - make modeset hw state checker atomic aware (Maarten) > - close races in gpu stuck detection/seqno reading (Chris) > - tons&tons of small improvements from Chris Wilson all over the gem code > - more dsi/bxt work from Ramalingam&Jani > - macro polish from Joonas > - guc fw loading fixes (Arun&Dave) > - vmap notifier (acked by Andrew) + i915 support by Chris Wilson > - create bottom half for execlist irq processing (Chris Wilson) > - vlv/chv pll cleanup (Ville) > - rework DP detection, especially sink detection (Shubhangi Shrivastava) > - make color manager support fully atomic (Maarten) > - avoid livelock on chv in execlist irq handler (Chris) The chv irq handler change needs to be backed out, or more preferably fixed in another way. Currently chv isn't in the best shape due to this. I'm trying to figure out how it actually fails. My only theory right now is that if a display interrupt happens just as we've started processing a GT interrupt, there might not be an edge for the CPU interrupt generation logic (assuming the input there is an OR of the GT and display interrupts), so the CPU interrupt won't be re-raised and thus we fail to process the display interrupt. I'll need to figure out a decent way to test that theory though. If this is the case, one potential way to fix it would be to clear VLV_IER around irq processing, as that combined with the GEN8_MASTER_IRQ disabling should guarantee an edge at the top level. So it would be similar to the PCH SDE trick we're doing on some platforms. One interesting detail I've already noticed is that, unlike gen4, IIR isn't actually double buffered on VLV/CHV. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Use new i915_gem_object_pin_map for LRC
== Series Details == Series: drm/i915: Use new i915_gem_object_pin_map for LRC URL : https://patchwork.freedesktop.org/series/5580/ State : failure == Summary == Series 5580v1 drm/i915: Use new i915_gem_object_pin_map for LRC http://patchwork.freedesktop.org/api/1.0/series/5580/revisions/1/mbox/ Test drv_module_reload_basic: pass -> DMESG-WARN (bsw-nuc-2) pass -> DMESG-WARN (skl-nuci5) pass -> DMESG-WARN (bdw-nuci7) pass -> DMESG-WARN (skl-i7k-2) pass -> DMESG-WARN (bdw-ultra) Test gem_basic: Subgroup create-fd-close: incomplete -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup invalid-param-get: incomplete -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd1: incomplete -> SKIP (bsw-nuc-2) Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (bsw-nuc-2) Test gem_storedw_loop: Subgroup basic-bsd2: incomplete -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup bad-pitch-1024: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-y-tiled: incomplete -> PASS (bsw-nuc-2) Subgroup small-bo: incomplete -> PASS (bsw-nuc-2) Subgroup unused-handle: incomplete -> PASS (bsw-nuc-2) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: incomplete -> PASS (snb-dellxps) Subgroup read-crc-pipe-a: incomplete -> SKIP (bsw-nuc-2) Subgroup read-crc-pipe-b: incomplete -> SKIP (bsw-nuc-2) Subgroup suspend-read-crc-pipe-a: pass -> INCOMPLETE (hsw-gt2) Test prime_self_import: Subgroup basic-with_one_bo: incomplete -> PASS (bsw-nuc-2) Subgroup basic-with_two_bos: incomplete -> PASS (bsw-nuc-2) bdw-nuci7total:202 pass:189 dwarn:1 dfail:0 fail:0 skip:12 bdw-ultratotal:202 pass:178 dwarn:1 dfail:0 fail:0 skip:23 bsw-nuc-2total:201 pass:161 dwarn:1 dfail:0 fail:0 skip:39 byt-nuc total:201 pass:163 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:202 pass:178 dwarn:0 dfail:0 fail:0 skip:24 hsw-gt2 total:25 pass:22 dwarn:0 dfail:0 fail:0 skip:2 ilk-hp8440p total:202 pass:134 dwarn:0 dfail:0 fail:0 skip:68 ivb-t430stotal:202 pass:174 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:202 pass:176 dwarn:1 dfail:0 fail:0 skip:25 skl-nuci5total:202 pass:190 dwarn:1 dfail:0 fail:0 skip:11 snb-dellxps total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 snb-x220ttotal:202 pass:164 dwarn:0 dfail:0 fail:1 skip:37 Results at /archive/results/CI_IGT_test/Patchwork_1869/ dc5380b5263ebb0bf251bb09db542585702b528b drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest 91ffa2a drm/i915: Use new i915_gem_object_pin_map for LRC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use new i915_gem_object_pin_map for LRC
On Tue, Apr 12, 2016 at 09:52:28AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> We can use the new pin/lazy unpin API for simplicity
> and more performance in the execlist submission paths.
>
> v2:
> * Fix error handling and convert more users.
> * Compact some names for readability.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Chris Wilson
Reviewed-by: Chris Wilson
Hmm, the stress tests that would exercise this are already currently
fail (thinking of gem_ctx_create etc). But this should not excerbate it
much!
> - lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
> - if (WARN_ON(!lrc_state_page)) {
> - ret = -ENODEV;
> + obj_addr = i915_gem_object_pin_map(ctx_obj);
Oops, there's a bug in pin_map in that we don't mark the object as
dirty. Can you send a quick obj->dirty = true patch?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/edid: Add drm_edid_get_monitor_name()
On Mon, 11 Apr 2016, Jim Bride wrote:
> In order to include monitor name information in debugfs
> output we needed to add a function that would extract the
> monitor name from the EDID, and that function needed to
> reside in the file where the rest of the EDID helper
> functions are implemented.
>
> v2: Refactor to have drm_edid_get_monitor_name() and drm_edid_to_eld()
> use a common helper function to extract the monitor name from the
> edid. [Jani] + rebase.
>
> v3: Minor changes suggested by Jani + rebase.
>
> cc: dri-de...@lists.freedesktop.org
> cc: Jani Nikula
> Signed-off-by: Jim Bride
> ---
> drivers/gpu/drm/drm_edid.c | 51
> ++
> include/drm/drm_crtc.h | 2 ++
> 2 files changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 558ef9f..da30ce3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3293,6 +3293,46 @@ monitor_name(struct detailed_timing *t, void *data)
> *(u8 **)data = t->data.other_data.data.str.str;
> }
>
> +static int get_monitor_name(struct edid *edid, char name[13])
> +{
> + char *edid_name = NULL;
> + int mnl;
> +
> + if (!edid || !name)
> + return 0;
> +
> + drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
> + for (mnl = 0; edid_name && mnl < 13; mnl++) {
> + if (edid_name[mnl] == 0x0a)
> + break;
> +
> + name[mnl] = edid_name[mnl];
> + }
> +
> + return mnl;
> +}
> +
> +/**
> + * drm_edid_get_monitor_name - fetch the monitor name from the edid
> + * @edid: monitor EDID information
> + * @name: pointer to a character array to hold the name of the monitor
> + * @bufsize: The size of the name buffer (should be at least 13 chars.)
Nitpick, should be 13 + termination = 14 bytes.
> + *
> + */
> +void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize)
> +{
> + int name_length = -1;
Nitpick, no need to initialize.
Other than those,
Reviewed-by: Jani Nikula
> + char buf[13];
> +
> + if (bufsize <= 0)
> + return;
> +
> + name_length = min(get_monitor_name(edid, buf), bufsize - 1);
> + memcpy(name, buf, name_length);
> + name[name_length] = '\0';
> +}
> +EXPORT_SYMBOL(drm_edid_get_monitor_name);
> +
> /**
> * drm_edid_to_eld - build ELD from EDID
> * @connector: connector corresponding to the HDMI/DP sink
> @@ -3306,7 +3346,6 @@ void drm_edid_to_eld(struct drm_connector *connector,
> struct edid *edid)
> {
> uint8_t *eld = connector->eld;
> u8 *cea;
> - u8 *name;
> u8 *db;
> int total_sad_count = 0;
> int mnl;
> @@ -3320,14 +3359,8 @@ void drm_edid_to_eld(struct drm_connector *connector,
> struct edid *edid)
> return;
> }
>
> - name = NULL;
> - drm_for_each_detailed_block((u8 *)edid, monitor_name, &name);
> - /* max: 13 bytes EDID, 16 bytes ELD */
> - for (mnl = 0; name && mnl < 13; mnl++) {
> - if (name[mnl] == 0x0a)
> - break;
> - eld[20 + mnl] = name[mnl];
> - }
> + mnl = get_monitor_name(edid, eld + 20);
> +
> eld[4] = (cea[1] << 5) | mnl;
> DRM_DEBUG_KMS("ELD monitor %s\n", eld + 20);
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8cb377c..6d46842 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2500,6 +2500,8 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
> extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool
> print_bad_edid,
>bool *edid_corrupt);
> extern bool drm_edid_is_valid(struct edid *edid);
> +extern void drm_edid_get_monitor_name(struct edid *edid, char *name,
> + int buflen);
>
> extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device
> *dev,
>char topology[8]);
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Use new i915_gem_object_pin_map for LRC
On Tue, Apr 12, 2016 at 09:24:53AM -, Patchwork wrote: > == Series Details == > > Series: drm/i915: Use new i915_gem_object_pin_map for LRC > URL : https://patchwork.freedesktop.org/series/5580/ > State : failure > > == Summary == > > Series 5580v1 drm/i915: Use new i915_gem_object_pin_map for LRC > http://patchwork.freedesktop.org/api/1.0/series/5580/revisions/1/mbox/ > > Test drv_module_reload_basic: > pass -> DMESG-WARN (bsw-nuc-2) > pass -> DMESG-WARN (skl-nuci5) > pass -> DMESG-WARN (bdw-nuci7) > pass -> DMESG-WARN (skl-i7k-2) > pass -> DMESG-WARN (bdw-ultra) Hmm, I guess we have a leak? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use new i915_gem_object_pin_map for LRC
On 12/04/16 10:30, Chris Wilson wrote:
> On Tue, Apr 12, 2016 at 09:52:28AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin
>>
>> We can use the new pin/lazy unpin API for simplicity
>> and more performance in the execlist submission paths.
>>
>> v2:
>>* Fix error handling and convert more users.
>>* Compact some names for readability.
>>
>> Signed-off-by: Tvrtko Ursulin
>> Cc: Chris Wilson
>
> Reviewed-by: Chris Wilson
>
> Hmm, the stress tests that would exercise this are already currently
> fail (thinking of gem_ctx_create etc). But this should not excerbate it
> much!
Something is broken here as reported by the CI:
[ 329.004489] [ cut here ]
[ 329.004508] WARNING: CPU: 1 PID: 7049 at
drivers/gpu/drm/i915/i915_gem.c:4641 i915_gem_free_object+0x3bd/0x430 [i915]
[ 329.004510] WARN_ON(obj->pages_pin_count)
[ 329.004511] Modules linked in:
[ 329.004512] snd_hda_codec_hdmi x86_pkg_temp_thermal intel_powerclamp
coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
snd_hda_codec_realtek snd_hda_codec_generic i915(-) snd_hda_codec snd_hwdep
snd_hda_core mei_me lpc_ich snd_pcm mei i2c_hid i2c_designware_platform
i2c_designware_core e1000e ptp pps_core sdhci_acpi sdhci mmc_core [last
unloaded: snd_hda_intel]
[ 329.004527] CPU: 1 PID: 7049 Comm: rmmod Tainted: G U
4.6.0-rc3-gfxbench+ #1
[ 329.004528] Hardware name: /NUC5i7RYB, BIOS
RYBDWi35.86A.0249.2015.0529.1640 05/29/2015
[ 329.004529] 880212f7bbe0 81405fa5
880212f7bc30
[ 329.004531] 880212f7bc20 81079c7c
122112f7bc28
[ 329.004533] 880211eaad90 880211eaad40 880211eaae30
8800ceb7
[ 329.004535] Call Trace:
[ 329.004539] [] dump_stack+0x67/0x92
[ 329.004541] [] __warn+0xcc/0xf0
[ 329.004542] [] warn_slowpath_fmt+0x4a/0x50
[ 329.004555] [] i915_gem_free_object+0x3bd/0x430 [i915]
[ 329.004558] [] drm_gem_object_free+0x2b/0x50
[ 329.004570] [] intel_lr_context_free+0x74/0xd0 [i915]
[ 329.004580] [] i915_gem_context_free+0x1a3/0x270 [i915]
[ 329.004589] [] i915_gem_context_fini+0x9d/0xd0 [i915]
[ 329.004603] [] i915_driver_unload+0x119/0x1d0 [i915]
[ 329.004605] [] drm_dev_unregister+0x24/0xa0
[ 329.004606] [] drm_put_dev+0x1e/0x60
[ 329.004614] [] i915_pci_remove+0x10/0x20 [i915]
[ 329.004616] [] pci_device_remove+0x34/0xb0
[ 329.004618] [] __device_release_driver+0x95/0x140
[ 329.004619] [] driver_detach+0xb6/0xc0
[ 329.004620] [] bus_remove_driver+0x53/0xd0
[ 329.004622] [] driver_unregister+0x27/0x50
[ 329.004623] [] pci_unregister_driver+0x25/0x70
[ 329.004625] [] drm_pci_exit+0x74/0x90
[ 329.004638] [] i915_exit+0x20/0x1a0 [i915]
[ 329.004640] [] SyS_delete_module+0x18f/0x1f0
[ 329.004642] [] entry_SYSCALL_64_fastpath+0x1c/0xac
[ 329.004643] ---[ end trace f0bf445e9d9a7dbe ]---
[ 329.004718] [ cut here ]
It even happened in my local BAT run but I did not spot it due excessive eDP
triggered WARNs. :(
Will look into it as soon as I stash the current work.
>> -lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
>> -if (WARN_ON(!lrc_state_page)) {
>> -ret = -ENODEV;
>> +obj_addr = i915_gem_object_pin_map(ctx_obj);
>
> Oops, there's a bug in pin_map in that we don't mark the object as
> dirty. Can you send a quick obj->dirty = true patch?
I can if you are sure we want this. Callers might only want to read so I am not
sure.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/dp/mst: Add source port info to debugfs output
On Mon, 11 Apr 2016, Jim Bride wrote: > Modify the debugfs output for i915_dp_mst_info to list the source port for > the DP MST topology in question. > > v2: rebase > v3: rebase > > cc: Jani Nikula > Signed-off-by: Jim Bride Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/i915_debugfs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 9640738..644e80b 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3446,7 +3446,8 @@ static int i915_dp_mst_info(struct seq_file *m, void > *unused) > intel_dig_port = enc_to_dig_port(encoder); > if (!intel_dig_port->dp.can_mst) > continue; > - > + seq_printf(m, "MST Source Port %c\n", > +port_name(intel_dig_port->port)); > drm_dp_mst_dump_topology(m, &intel_dig_port->dp.mst_mgr); > } > drm_modeset_unlock_all(dev); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use new i915_gem_object_pin_map for LRC
On Tue, Apr 12, 2016 at 10:36:39AM +0100, Tvrtko Ursulin wrote:
>
> On 12/04/16 10:30, Chris Wilson wrote:
> > On Tue, Apr 12, 2016 at 09:52:28AM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin
> >>
> >> We can use the new pin/lazy unpin API for simplicity
> >> and more performance in the execlist submission paths.
> >>
> >> v2:
> >>* Fix error handling and convert more users.
> >>* Compact some names for readability.
> >>
> >> Signed-off-by: Tvrtko Ursulin
> >> Cc: Chris Wilson
> >
> > Reviewed-by: Chris Wilson
> >
> > Hmm, the stress tests that would exercise this are already currently
> > fail (thinking of gem_ctx_create etc). But this should not excerbate it
> > much!
>
> Something is broken here as reported by the CI:
>
> [ 329.004489] [ cut here ]
> [ 329.004508] WARNING: CPU: 1 PID: 7049 at
> drivers/gpu/drm/i915/i915_gem.c:4641 i915_gem_free_object+0x3bd/0x430 [i915]
> [ 329.004510] WARN_ON(obj->pages_pin_count)
> [ 329.004511] Modules linked in:
> [ 329.004512] snd_hda_codec_hdmi x86_pkg_temp_thermal intel_powerclamp
> coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
> snd_hda_codec_realtek snd_hda_codec_generic i915(-) snd_hda_codec snd_hwdep
> snd_hda_core mei_me lpc_ich snd_pcm mei i2c_hid i2c_designware_platform
> i2c_designware_core e1000e ptp pps_core sdhci_acpi sdhci mmc_core [last
> unloaded: snd_hda_intel]
> [ 329.004527] CPU: 1 PID: 7049 Comm: rmmod Tainted: G U
> 4.6.0-rc3-gfxbench+ #1
> [ 329.004528] Hardware name: /NUC5i7RYB, BIOS
> RYBDWi35.86A.0249.2015.0529.1640 05/29/2015
> [ 329.004529] 880212f7bbe0 81405fa5
> 880212f7bc30
> [ 329.004531] 880212f7bc20 81079c7c
> 122112f7bc28
> [ 329.004533] 880211eaad90 880211eaad40 880211eaae30
> 8800ceb7
> [ 329.004535] Call Trace:
> [ 329.004539] [] dump_stack+0x67/0x92
> [ 329.004541] [] __warn+0xcc/0xf0
> [ 329.004542] [] warn_slowpath_fmt+0x4a/0x50
> [ 329.004555] [] i915_gem_free_object+0x3bd/0x430 [i915]
> [ 329.004558] [] drm_gem_object_free+0x2b/0x50
> [ 329.004570] [] intel_lr_context_free+0x74/0xd0 [i915]
> [ 329.004580] [] i915_gem_context_free+0x1a3/0x270 [i915]
> [ 329.004589] [] i915_gem_context_fini+0x9d/0xd0 [i915]
> [ 329.004603] [] i915_driver_unload+0x119/0x1d0 [i915]
> [ 329.004605] [] drm_dev_unregister+0x24/0xa0
> [ 329.004606] [] drm_put_dev+0x1e/0x60
> [ 329.004614] [] i915_pci_remove+0x10/0x20 [i915]
> [ 329.004616] [] pci_device_remove+0x34/0xb0
> [ 329.004618] [] __device_release_driver+0x95/0x140
> [ 329.004619] [] driver_detach+0xb6/0xc0
> [ 329.004620] [] bus_remove_driver+0x53/0xd0
> [ 329.004622] [] driver_unregister+0x27/0x50
> [ 329.004623] [] pci_unregister_driver+0x25/0x70
> [ 329.004625] [] drm_pci_exit+0x74/0x90
> [ 329.004638] [] i915_exit+0x20/0x1a0 [i915]
> [ 329.004640] [] SyS_delete_module+0x18f/0x1f0
> [ 329.004642] [] entry_SYSCALL_64_fastpath+0x1c/0xac
> [ 329.004643] ---[ end trace f0bf445e9d9a7dbe ]---
> [ 329.004718] [ cut here ]
>
> It even happened in my local BAT run but I did not spot it due excessive eDP
> triggered WARNs. :(
I guess it is in the default context.
> Will look into it as soon as I stash the current work.
>
> >> - lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
> >> - if (WARN_ON(!lrc_state_page)) {
> >> - ret = -ENODEV;
> >> + obj_addr = i915_gem_object_pin_map(ctx_obj);
> >
> > Oops, there's a bug in pin_map in that we don't mark the object as
> > dirty. Can you send a quick obj->dirty = true patch?
>
> I can if you are sure we want this. Callers might only want to read so I am
> not sure.
I agree it is slightly presumptuous, but not marking the objects as dirty
is a potential source of data loss, marking them as dirty even for pure
reads is just a missed optimisation.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use new i915_gem_object_pin_map for LRC
On Tue, Apr 12, 2016 at 09:52:28AM +0100, Tvrtko Ursulin wrote:
> -static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
> -struct drm_i915_gem_object
> *default_ctx_obj)
> +static int
> +lrc_setup_hws(struct intel_engine_cs *engine,
> + struct drm_i915_gem_object *def_ctx_obj)
> {
> struct drm_i915_private *dev_priv = engine->dev->dev_private;
> - struct page *page;
> + void *hws;
>
> /* The HWSP is part of the default context object in LRC mode. */
> - engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
> - + LRC_PPHWSP_PN * PAGE_SIZE;
> - page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
> - engine->status_page.page_addr = kmap(page);
> - engine->status_page.obj = default_ctx_obj;
> + engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(def_ctx_obj) +
> +LRC_PPHWSP_PN * PAGE_SIZE;
> + hws = i915_gem_object_pin_map(def_ctx_obj);
> + if (IS_ERR(hws))
> + return PTR_ERR(hws);
> + engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
> + engine->status_page.obj = def_ctx_obj;
>
> I915_WRITE(RING_HWS_PGA(engine->mmio_base),
> - (u32)engine->status_page.gfx_addr);
> +(u32)engine->status_page.gfx_addr);
> POSTING_READ(RING_HWS_PGA(engine->mmio_base));
> +
> + return 0;
> }
I don't see the corresonding change for tearing down the hws.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] test/gem_mocs_settings: Testing MOCS register settings
On Mon, Apr 11, 2016 at 05:50:09PM +0100, Peter Antoine wrote: > The MOCS registers were added in Gen9 and define the caching policy. > The registers are split into two sets. The first set controls the > EDRAM policy and have a set for each engine, the second set controls > the L3 policy. The two sets use the same index. > > The RCS registers and the L3CC registers are stored in the RCS context. > > The test checks that the registers are correct by checking the values by > directly reading them via MMIO, then again it tests them by reading them > from within a batch buffer. RCS engine is tested last as it programs the > registers via a batch buffer and this will invalidate the test for > workloads that don't use the render ring or don't run a render batch > first. > > v2: Reorganised the structure. > Added more tests. (Chris Wilson) > v3: Fixed a few bugs. (Chris Wilson) > v4: More Tidy-ups.(Chris Wilson) > SKL does does not have a snoop bit. (Peter Antoine) Still got the domain control wrong, but I fixed that up and pushed. I also tweaked a few of the tests along the way... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Updated drm-intel-testing
On 12/04/16 10:21, Ville Syrjälä wrote: On Mon, Apr 11, 2016 at 09:45:11PM +0200, Daniel Vetter wrote: Hi all, New -testing cycle with cool stuff: - make modeset hw state checker atomic aware (Maarten) - close races in gpu stuck detection/seqno reading (Chris) - tons&tons of small improvements from Chris Wilson all over the gem code - more dsi/bxt work from Ramalingam&Jani - macro polish from Joonas - guc fw loading fixes (Arun&Dave) - vmap notifier (acked by Andrew) + i915 support by Chris Wilson - create bottom half for execlist irq processing (Chris Wilson) - vlv/chv pll cleanup (Ville) - rework DP detection, especially sink detection (Shubhangi Shrivastava) - make color manager support fully atomic (Maarten) - avoid livelock on chv in execlist irq handler (Chris) The chv irq handler change needs to be backed out, or more preferably fixed in another way. Currently chv isn't in the best shape due to this. Could be that the revert is not a big deal since, if the tasklet is working fine, the work is not done in the irq handler any longer so the even the looping handler will not be causing huge irqoff latencies. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
On Tue, Apr 12, 2016 at 10:37:22AM +0200, Boris Brezillon wrote: > Mark Brown wrote: > > On Thu, Apr 07, 2016 at 11:54:31PM +0200, Boris Brezillon wrote: > > I'm not sure there's a strong one, we don't really use the class device > > for anything, but without doing a full audit I couldn't guarantee that. > At first glance I don't see any problem (even the rdev_err/info/...() > functions do not use dev_err/info/...()). The patch will be part of v6 > (unless you want me to send it independently). I'd rather it didn't get sucked into this series since it seems that it's getting delayed indefinitely - I can apply it to the regulator tree and create a tag that can be pulled into other trees as needed if things do get applied. signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Fix up vlv/chv display irq setup
On ti, 2016-04-12 at 12:05 +0300, Ville Syrjälä wrote:
> On Mon, Apr 11, 2016 at 07:29:13PM +0300, Imre Deak wrote:
> > On ma, 2016-04-11 at 16:56 +0300, ville.syrj...@linux.intel.com
> > wrote:
> > > From: Ville Syrjälä
> > >
> > > The vlv/chv display irq setup was a bit of mess after I ran out
> > > of steam
> > > when working on it last. Fix it up so that we just have a
> > > _reset() and
> > > _postinstall() hooks for the display irqs, and use those
> > > consistently.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/i915_irq.c | 102 ++--
> > >
> > > 1 file changed, 24 insertions(+), 78 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c
> > > index 1d21ebfffd4d..a1239fedc086 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -3306,13 +3306,15 @@ static void vlv_display_irq_reset(struct
> > > drm_i915_private *dev_priv)
> > > {
> > > enum pipe pipe;
> > >
> > > - i915_hotplug_interrupt_update(dev_priv, 0x, 0);
> > > + i915_hotplug_interrupt_update_locked(dev_priv,
> > > 0x, 0);
> > > I915_WRITE(PORT_HOTPLUG_STAT,
> > > I915_READ(PORT_HOTPLUG_STAT));
> > >
> > > for_each_pipe(dev_priv, pipe)
> > > I915_WRITE(PIPESTAT(pipe), 0x);
> >
> > Since vlv_display_irq_reset() will be used in place
> > of valleyview_display_irqs_uninstall()/i915_disable_pipestat()
> > we'll leave now stale bits in pipestat_irq_mask[pipe]. It's not a
> > problem since display_irqs_enabled effectively masks these bits,
> > but
> > for consistency I'd clear them.
>
> OTOH we can't mask PIPESTAT bits, so even if we clear them here, it's
> very likely some of the bits will be set again by the time we
> actually
> enable an interrupt.
>
> In any case, I think I'll be posting a patch to clean up the PIPESTAT
> clearing/disabling acrosss the board. It's a bit of a mess right now,
> with each platform doing things slightly differently.
Ok, you could add something about the above to the commit message. My
R-b applies in any case.
> > The same goes for clearing PIPE_FIFO_UNDERRUN_STATUS.
> >
> > With that:
> > Reviewed-by: Imre Deak
> >
> > >
> > > GEN5_IRQ_RESET(VLV_);
> > > +
> > > + dev_priv->irq_mask = ~0;
> > > }
> >
> >
> >
> > >
> > > static void valleyview_irq_preinstall(struct drm_device *dev)
> > > @@ -3323,7 +3325,9 @@ static void
> > > valleyview_irq_preinstall(struct drm_device *dev)
> > >
> > > I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
> > >
> > > + spin_lock_irq(&dev_priv->irq_lock);
> > > vlv_display_irq_reset(dev_priv);
> > > + spin_unlock_irq(&dev_priv->irq_lock);
> > > }
> > >
> > > static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
> > > @@ -3398,7 +3402,9 @@ static void
> > > cherryview_irq_preinstall(struct drm_device *dev)
> > >
> > > I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
> > >
> > > + spin_lock_irq(&dev_priv->irq_lock);
> > > vlv_display_irq_reset(dev_priv);
> > > + spin_unlock_irq(&dev_priv->irq_lock);
> > > }
> > >
> > > static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
> > > @@ -3645,7 +3651,7 @@ static int ironlake_irq_postinstall(struct
> > > drm_device *dev)
> > > return 0;
> > > }
> > >
> > > -static void valleyview_display_irqs_install(struct
> > > drm_i915_private *dev_priv)
> > > +static void vlv_display_irq_postinstall(struct drm_i915_private
> > > *dev_priv)
> > > {
> > > u32 pipestat_mask;
> > > u32 iir_mask;
> > > @@ -3679,40 +3685,6 @@ static void
> > > valleyview_display_irqs_install(struct drm_i915_private
> > > *dev_priv)
> > > POSTING_READ(VLV_IMR);
> > > }
> > >
> > > -static void valleyview_display_irqs_uninstall(struct
> > > drm_i915_private *dev_priv)
> > > -{
> > > - u32 pipestat_mask;
> > > - u32 iir_mask;
> > > - enum pipe pipe;
> > > -
> > > - iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > > - I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > > - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > > - if (IS_CHERRYVIEW(dev_priv))
> > > - iir_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
> > > -
> > > - dev_priv->irq_mask |= iir_mask;
> > > - I915_WRITE(VLV_IMR, dev_priv->irq_mask);
> > > - I915_WRITE(VLV_IER, ~dev_priv->irq_mask);
> > > - I915_WRITE(VLV_IIR, iir_mask);
> > > - I915_WRITE(VLV_IIR, iir_mask);
> > > - POSTING_READ(VLV_IIR);
> > > -
> > > - pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV |
> > > - PIPE_CRC_DONE_INTERRUPT_STATUS;
> > > -
> > > - i915_disable_pipestat(dev_priv, PIPE_A,
> > > PIPE_GMBUS_INTERRUPT_STATUS);
> > > - for_each_pipe(dev_priv, pipe)
> > > - i915_disable_pipestat(dev_priv, pipe,
> > > pipestat_mask);
> > > -
> > > - pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> > > - PIPE_FIFO_UNDERRUN_STATUS;
> > > -
> > > - for_each_pipe(dev_priv, pipe)
> > > - I915_WRITE(PIPESTAT(pipe), pipestat_mask);
> >
Re: [Intel-gfx] [PATCH v4 RESEND 0/5] Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
On Mon, 28 Mar 2016, Lyude wrote: > Resending this because it looks like replying to my previous series of patches > causes patchwork to pick up patches from the original version of this and > try to apply them along with this one. Lyude, these don't seem to apply cleanly, please rebase and repost, and we can pick them up once Ville confirms they don't break his display. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix eDP low vswing for Broadwell
On Mon, 2016-04-11 at 16:30 +0300, Ville Syrjälä wrote:
> On Thu, Mar 17, 2016 at 12:23:10PM +0200, Mika Kahola wrote:
> > It was noticed on bug #94087 that module parameter
> > i915.edp_vswing=2 that should override the VBT setting
> > to use default voltage swing (400 mV) was not applied
> > for Broadwell.
> >
> > This patch provides a fix for this by checking if default
> > i.e. higher voltage swing is requested to be used and
> > applies the DDI translations table for DP instead of eDP
> > (low vswing) table.
> >
> > v2: Combine two if statements into one (Jani)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94087
> > Signed-off-by: Mika Kahola
>
> We didn't get a CI run for this?
No, we didn't. Should I resend this patch for a CI run?
>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index ab025a5..100a532 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -443,9 +443,17 @@ void intel_prepare_ddi_buffer(struct intel_encoder
> > *encoder)
> > } else if (IS_BROADWELL(dev_priv)) {
> > ddi_translations_fdi = bdw_ddi_translations_fdi;
> > ddi_translations_dp = bdw_ddi_translations_dp;
> > - ddi_translations_edp = bdw_ddi_translations_edp;
> > +
> > + if (dev_priv->edp_low_vswing) {
> > + ddi_translations_edp = bdw_ddi_translations_edp;
> > + n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp);
> > + } else {
> > + ddi_translations_edp = bdw_ddi_translations_dp;
> > + n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
> > + }
> > +
> > ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > - n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp);
> > +
> > n_dp_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
> > n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > hdmi_default_entry = 7;
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Move vlv_init_display_clock_gating() to the display power well
On ma, 2016-04-11 at 16:56 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The registers frobbed by vlv_init_display_clock_gating() libve inside
> the disp2d power well, so frobbing them while the power well is down
> results in unclaimed register access warning (and of course the
> values
> won't stick). Let's do this setup after we know the power well is
> enabled.
>
> It's also worth noting that DSPCLK_GATE_D and CBR1_VLV lose their
> state
> when the power well goes down, but fortunately the values we've been
> writing are actually the reset defaults.
>
> MI_ARB_VLV actually retains its value even if the power well was
> turned
> off, we just can't access it while the power well is down.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94164
> Signed-off-by: Ville Syrjälä
The spec doesn't say anything about backing power wells, I assume you
checked this manually by reading the regs out while the power well was
off. Looks ok:
Reviewed-by: Imre Deak
> ---
> drivers/gpu/drm/i915/intel_pm.c | 15 ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 13 +
> 2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 43b24a1f5ee6..c80d044fe082 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6882,23 +6882,10 @@ static void
> ivybridge_init_clock_gating(struct drm_device *dev)
> gen6_check_mch_setup(dev);
> }
>
> -static void vlv_init_display_clock_gating(struct drm_i915_private
> *dev_priv)
> -{
> - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> -
> - /*
> - * Disable trickle feed and enable pnd deadline calculation
> - */
> - I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> - I915_WRITE(CBR1_VLV, 0);
> -}
> -
> static void valleyview_init_clock_gating(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - vlv_init_display_clock_gating(dev_priv);
> -
> /* WaDisableEarlyCull:vlv */
> I915_WRITE(_3D_CHICKEN3,
> _MASKED_BIT_ENABLE(_3D_CHICKEN_SF_DISABLE_OBJEND_
> CULL));
> @@ -6981,8 +6968,6 @@ static void cherryview_init_clock_gating(struct
> drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - vlv_init_display_clock_gating(dev_priv);
> -
> /* WaVSRefCountFullforceMissDisable:chv */
> /* WaDSRefCountFullforceMissDisable:chv */
> I915_WRITE(GEN7_FF_THREAD_MODE,
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 80e8bd4b43b5..8f9797f17991 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -900,6 +900,17 @@ static bool vlv_power_well_enabled(struct
> drm_i915_private *dev_priv,
> return enabled;
> }
>
> +static void vlv_init_display_clock_gating(struct drm_i915_private
> *dev_priv)
> +{
> + I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> +
> + /*
> + * Disable trickle feed and enable pnd deadline calculation
> + */
> + I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> + I915_WRITE(CBR1_VLV, 0);
> +}
> +
> static void vlv_display_power_well_init(struct drm_i915_private
> *dev_priv)
> {
> enum pipe pipe;
> @@ -922,6 +933,8 @@ static void vlv_display_power_well_init(struct
> drm_i915_private *dev_priv)
> I915_WRITE(DPLL(pipe), val);
> }
>
> + vlv_init_display_clock_gating(dev_priv);
> +
> spin_lock_irq(&dev_priv->irq_lock);
> valleyview_enable_display_irqs(dev_priv);
> spin_unlock_irq(&dev_priv->irq_lock);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
On Tue, 12 Apr 2016 11:09:38 +0100 Mark Brown wrote: > On Tue, Apr 12, 2016 at 10:37:22AM +0200, Boris Brezillon wrote: > > Mark Brown wrote: > > > On Thu, Apr 07, 2016 at 11:54:31PM +0200, Boris Brezillon wrote: > > > > I'm not sure there's a strong one, we don't really use the class device > > > for anything, but without doing a full audit I couldn't guarantee that. > > > At first glance I don't see any problem (even the rdev_err/info/...() > > functions do not use dev_err/info/...()). The patch will be part of v6 > > (unless you want me to send it independently). > > I'd rather it didn't get sucked into this series since it seems that > it's getting delayed indefinitely - I can apply it to the regulator tree > and create a tag that can be pulled into other trees as needed if things > do get applied. Done. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use new i915_gem_object_pin_map for LRC
On 12/04/16 10:43, Chris Wilson wrote:
On Tue, Apr 12, 2016 at 09:52:28AM +0100, Tvrtko Ursulin wrote:
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
- struct drm_i915_gem_object
*default_ctx_obj)
+static int
+lrc_setup_hws(struct intel_engine_cs *engine,
+ struct drm_i915_gem_object *def_ctx_obj)
{
struct drm_i915_private *dev_priv = engine->dev->dev_private;
- struct page *page;
+ void *hws;
/* The HWSP is part of the default context object in LRC mode. */
- engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
- + LRC_PPHWSP_PN * PAGE_SIZE;
- page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
- engine->status_page.page_addr = kmap(page);
- engine->status_page.obj = default_ctx_obj;
+ engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(def_ctx_obj) +
+ LRC_PPHWSP_PN * PAGE_SIZE;
+ hws = i915_gem_object_pin_map(def_ctx_obj);
+ if (IS_ERR(hws))
+ return PTR_ERR(hws);
+ engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
+ engine->status_page.obj = def_ctx_obj;
I915_WRITE(RING_HWS_PGA(engine->mmio_base),
- (u32)engine->status_page.gfx_addr);
+ (u32)engine->status_page.gfx_addr);
POSTING_READ(RING_HWS_PGA(engine->mmio_base));
+
+ return 0;
}
I don't see the corresonding change for tearing down the hws.
It is where it was, in intel_logical_ring_cleanup. Missing was unpin_map
in intel_lr_context_free. But still there is a leak somewhere. Triggered
by BAT but not individual IGTs so far.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/opregion: remove unnecessary ifdefs on CONFIG_ACPI
On Fri, 2016-04-08 at 17:59 +0300, Jani Nikula wrote:
> The whole file is ignored on CONFIG_ACPI=n.
>
> Signed-off-by: Jani Nikula
Reviewed-by: Ander Conselvan de Oliveira
> ---
> drivers/gpu/drm/i915/intel_opregion.c | 6 --
> 1 file changed, 6 deletions(-)
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index c15718b4862a..d5a4cb80273e 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -246,7 +246,6 @@ struct opregion_asle_ext {
>
> #define MAX_DSLP 1500
>
> -#ifdef CONFIG_ACPI
> static int swsci(struct drm_device *dev, u32 function, u32 parm, u32
> *parm_out)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -905,9 +904,6 @@ static void swsci_setup(struct drm_device *dev)
>opregion->swsci_gbda_sub_functions,
>opregion->swsci_sbcb_sub_functions);
> }
> -#else /* CONFIG_ACPI */
> -static inline void swsci_setup(struct drm_device *dev) {}
> -#endif /* CONFIG_ACPI */
>
> static int intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
> {
> @@ -950,9 +946,7 @@ int intel_opregion_setup(struct drm_device *dev)
> return -ENOTSUPP;
> }
>
> -#ifdef CONFIG_ACPI
> INIT_WORK(&opregion->asle_work, asle_work);
> -#endif
>
> base = memremap(asls, OPREGION_SIZE, MEMREMAP_WB);
> if (!base)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/opregion: remove unnecessary ifdefs on CONFIG_ACPI
On Fri, Apr 08, 2016 at 05:59:49PM +0300, Jani Nikula wrote: > The whole file is ignored on CONFIG_ACPI=n. That's an issue as we can't then acquire the opregion->vbt (which itself is not acpi dependent). Shrug no modern system can boot without acpi (at least not if you want more than cpu etc), so I guess we also don't care that much? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/core: Do not preserve framebuffer on rmfb, v3.
On Thu, Mar 31, 2016 at 01:26:03PM +0200, Maarten Lankhorst wrote:
> It turns out that preserving framebuffers after the rmfb call breaks
> vmwgfx userspace. This was originally introduced because it was thought
> nobody relied on the behavior, but unfortunately it seems there are
> exceptions.
>
> drm_framebuffer_remove may fail with -EINTR now, so a straight revert
> is impossible. There is no way to remove the framebuffer from the lists
> and active planes without introducing a race because of the different
> locking requirements. Instead call drm_framebuffer_remove from a
> workqueue, which is unaffected by signals.
>
> Changes since v1:
> - Add comment.
> Changes since v2:
> - Add fastpath for refcount = 1. (danvet)
>
> Cc: sta...@vger.kernel.org #v4.4+
> Fixes: 13803132818c ("drm/core: Preserve the framebuffer after removing it.")
> Testcase: kms_flip.flip-vs-rmfb-interruptible
> References:
> https://lists.freedesktop.org/archives/dri-devel/2016-March/102876.html
> Cc: Thomas Hellstrom
> Cc: David Herrmann
Reviewed-by: Daniel Vetter
But definitely want a t-b from Thomas before applying, since he reported
this regression.
-Daniel
> ---
> drivers/gpu/drm/drm_crtc.c | 29 -
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 55ffde5a3a4a..743bece1f579 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3434,6 +3434,18 @@ int drm_mode_addfb2(struct drm_device *dev,
> return 0;
> }
>
> +struct drm_mode_rmfb_work {
> + struct work_struct work;
> + struct drm_framebuffer *fb;
> +};
> +
> +static void drm_mode_rmfb_work_fn(struct work_struct *w)
> +{
> + struct drm_mode_rmfb_work *arg = container_of(w, typeof(*arg), work);
> +
> + drm_framebuffer_remove(arg->fb);
> +}
> +
> /**
> * drm_mode_rmfb - remove an FB from the configuration
> * @dev: drm device for the ioctl
> @@ -3474,7 +3486,22 @@ int drm_mode_rmfb(struct drm_device *dev,
> mutex_unlock(&dev->mode_config.fb_lock);
> mutex_unlock(&file_priv->fbs_lock);
>
> - drm_framebuffer_unreference(fb);
> + /*
> + * drm_framebuffer_remove may fail with -EINTR on pending signals,
> + * so run this in a separate stack as there's no way to correctly
> + * handle this after the fb is already removed from the lookup table.
> + */
> + if (atomic_read(&fb->refcount.refcount) > 1) {
> + struct drm_mode_rmfb_work arg;
> +
> + INIT_WORK_ONSTACK(&arg.work, drm_mode_rmfb_work_fn);
> + arg.fb = fb;
> +
> + schedule_work(&arg.work);
> + flush_work(&arg.work);
> + destroy_work_on_stack(&arg.work);
> + } else
> + drm_framebuffer_unreference(fb);
>
> return 0;
>
> --
> 2.1.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup.
On Fri, Apr 01, 2016 at 11:04:10PM +0300, Ville Syrjälä wrote: > On Tue, Mar 22, 2016 at 04:08:39PM +0100, Maarten Lankhorst wrote: > > Op 22-03-16 om 15:58 schreef Ville Syrjälä: > > > On Tue, Mar 22, 2016 at 03:42:14PM +0100, Maarten Lankhorst wrote: > > >> __drm_atomic_helper_plane_destroy_state calls > > >> drm_framebuffer_unreference, which means that if drm_framebuffer_free > > >> is called before plane->destroy freed memory will be accessed. > > >> > > >> A similar case happens for the blob list, which was freed before the > > >> crtc state was, resulting in the unreference_blob from crtc_destroy_state > > >> pointing to garbage memory causing another opportunity for a GPF. > > >> > > >> Signed-off-by: Maarten Lankhorst > > >> --- > > >> drivers/gpu/drm/drm_crtc.c | 18 +- > > >> 1 file changed, 9 insertions(+), 9 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > >> index 51c5a00ffdff..5a13b1afccbe 100644 > > >> --- a/drivers/gpu/drm/drm_crtc.c > > >> +++ b/drivers/gpu/drm/drm_crtc.c > > >> @@ -5958,6 +5958,15 @@ void drm_mode_config_cleanup(struct drm_device > > >> *dev) > > >> drm_property_destroy(dev, property); > > >> } > > > And what about props? Any chance of explosion due to those being gone? > > > > > Not as far as I'm aware of. > > > > If you use something like a crtc_x property, only the value gets written to > > crtc_state, the value is an int that would still be valid. > > I was too lazy to confirm this for all drivers. But at least i915 looked > clean on that front. So > > Reviewed-by: Ville Syrjälä Applied to drm-misc, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 01/46] pwm: rcar: make use of pwm_is_enabled()
On Wed, Mar 30, 2016 at 10:03:24PM +0200, Boris Brezillon wrote:
> Commit 5c31252c4a86 ("pwm: Add the pwm_is_enabled() helper") introduced a
> new function to test whether a PWM device is enabled or not without
> manipulating PWM internal fields.
> Hiding this is necessary if we want to smoothly move to the atomic PWM
> config approach without impacting PWM drivers.
> Fix this driver to use pwm_is_enabled() instead of directly accessing the
> ->flags field.
>
> Signed-off-by: Boris Brezillon
> ---
> drivers/pwm/pwm-rcar.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
Thierry
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 02/46] backlight: pwm_bl: remove useless call to pwm_set_period()
On Wed, Mar 30, 2016 at 10:03:25PM +0200, Boris Brezillon wrote: > The PWM period will be set when calling pwm_config. Remove this useless > call to pwm_set_period(), which might mess up with the internal PWM state. > > Signed-off-by: Boris Brezillon > Acked-by: Lee Jones > --- > drivers/video/backlight/pwm_bl.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 03/46] backlight: lm3630a_bl: stop messing with the pwm->period field
On Wed, Mar 30, 2016 at 10:03:26PM +0200, Boris Brezillon wrote: > pwm->period field is not supposed to be changed by PWM users. The only > ones authorized to change it are the PWM core and PWM drivers. > > Signed-off-by: Boris Brezillon > --- > drivers/video/backlight/lm3630a_bl.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/10] drm/virtio: Drop dummy gamma table support
On Fri, Apr 01, 2016 at 10:38:09AM +0200, Gerd Hoffmann wrote: > On Mi, 2016-03-30 at 11:51 +0200, Daniel Vetter wrote: > > No need to confuse userspace like this. > > Reviewed-by: Gerd Hoffmann This and the bochs one applied to drm-misc, thanks for your review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 04/46] pwm: get rid of pwm->lock
On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > PWM devices are not protected against concurrent accesses. The lock in > pwm_device might let PWM users think it is, but it's actually only > protecting the enabled state. > > Removing this lock should be fine as long as all PWM users are aware that > accesses to the PWM device have to be serialized, which seems to be the > case for all of them except the sysfs interface. > Patch the sysfs code by adding a lock to the pwm_export struct and making > sure it's taken for all accesses to the exported PWM device. > > Signed-off-by: Boris Brezillon > --- > drivers/pwm/core.c | 19 -- > drivers/pwm/sysfs.c | 57 > ++--- > include/linux/pwm.h | 2 -- > 3 files changed, 50 insertions(+), 28 deletions(-) This is a little overzealous. Only accesses that can cause races need to be protected by the lock. All of the *_show() callbacks don't modify the PWM device in any way, so there is no need to protect them against concurrent accesses. I've dropped the changes when applying. Thanks, Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 04/46] pwm: get rid of pwm->lock
Hi Thierry, On Tue, 12 Apr 2016 13:22:46 +0200 Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > > PWM devices are not protected against concurrent accesses. The lock in > > pwm_device might let PWM users think it is, but it's actually only > > protecting the enabled state. > > > > Removing this lock should be fine as long as all PWM users are aware that > > accesses to the PWM device have to be serialized, which seems to be the > > case for all of them except the sysfs interface. > > Patch the sysfs code by adding a lock to the pwm_export struct and making > > sure it's taken for all accesses to the exported PWM device. > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/pwm/core.c | 19 -- > > drivers/pwm/sysfs.c | 57 > > ++--- > > include/linux/pwm.h | 2 -- > > 3 files changed, 50 insertions(+), 28 deletions(-) > > This is a little overzealous. Only accesses that can cause races need to > be protected by the lock. All of the *_show() callbacks don't modify the > PWM device in any way, so there is no need to protect them against > concurrent accesses. This is probably true for this set of changes, but what will happen when we'll switch to the atomic API? There's no guarantee that pwm->state = *newstate is done atomically, and you may see a partially updated state when calling pwm_get_state() while another thread is calling pwm_apply_state(). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> Currently the PWM core mixes the current PWM state with the per-platform
> reference config (specified through the PWM lookup table, DT definition or
> directly hardcoded in PWM drivers).
>
> Create a pwm_args struct to store this reference config, so that PWM users
> can differentiate the current config from the reference one.
>
> Patch all places where pwm->args should be initialized. We keep the
> pwm_set_polarity/period() calls until all PWM users are patched to
> use pwm_args instead of pwm_get_period/polarity().
Perhaps a helper would be useful? Something like:
static inline void
pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
{
pwm_set_duty_cycle(pwm, args->duty_cycle);
pwm_set_period(pwm, args->period);
}
? That would make it slightly easier to get rid of it again after all
clients have been converted.
With the exception of pwm-clps711x all of these args are set at of_xlate
time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
might even be possible to move this call to the core, so that removal of
it will be a one-liner.
Thierry
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 04/46] pwm: get rid of pwm->lock
On Tue, Apr 12, 2016 at 01:32:55PM +0200, Boris Brezillon wrote: > Hi Thierry, > > On Tue, 12 Apr 2016 13:22:46 +0200 > Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > > > PWM devices are not protected against concurrent accesses. The lock in > > > pwm_device might let PWM users think it is, but it's actually only > > > protecting the enabled state. > > > > > > Removing this lock should be fine as long as all PWM users are aware that > > > accesses to the PWM device have to be serialized, which seems to be the > > > case for all of them except the sysfs interface. > > > Patch the sysfs code by adding a lock to the pwm_export struct and making > > > sure it's taken for all accesses to the exported PWM device. > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > drivers/pwm/core.c | 19 -- > > > drivers/pwm/sysfs.c | 57 > > > ++--- > > > include/linux/pwm.h | 2 -- > > > 3 files changed, 50 insertions(+), 28 deletions(-) > > > > This is a little overzealous. Only accesses that can cause races need to > > be protected by the lock. All of the *_show() callbacks don't modify the > > PWM device in any way, so there is no need to protect them against > > concurrent accesses. > > This is probably true for this set of changes, but what will happen > when we'll switch to the atomic API? There's no guarantee that > pwm->state = *newstate is done atomically, and you may see a partially > updated state when calling pwm_get_state() while another thread is > calling pwm_apply_state(). I'd argue that for sysfs it doesn't matter since the userspace API is non-atomic anyway. As such it doesn't really matter which part of the state you're getting because only one field from the query is exposed to userspace, hence the coherence of the state is irrelevant. All other users should be taking care of the serialization themselves already. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > The PWM state, represented by its period, duty_cycle and polarity, > is currently directly stored in the PWM device. > Declare a pwm_state structure embedding those field so that we can later > use this struct to atomically update all the PWM parameters at once. > > All pwm_get_xxx() helpers are now implemented as wrappers around > pwm_get_state(). > > Signed-off-by: Boris Brezillon > --- > drivers/pwm/core.c | 8 > include/linux/pwm.h | 54 > + > 2 files changed, 46 insertions(+), 16 deletions(-) > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index 6433059..f3f91e7 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > pwm->chip = chip; > pwm->pwm = chip->base + i; > pwm->hwpwm = i; > - pwm->polarity = polarity; > + pwm->state.polarity = polarity; Would this not more correctly be assigned to pwm->args.polarity? After all this is setting up the "initial" state, much like DT or the lookup tables would for duty cycle and period. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Move vlv_init_display_clock_gating() to the display power well
On Tue, Apr 12, 2016 at 01:25:07PM +0300, Imre Deak wrote:
> On ma, 2016-04-11 at 16:56 +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > The registers frobbed by vlv_init_display_clock_gating() libve inside
> > the disp2d power well, so frobbing them while the power well is down
> > results in unclaimed register access warning (and of course the
> > values
> > won't stick). Let's do this setup after we know the power well is
> > enabled.
> >
> > It's also worth noting that DSPCLK_GATE_D and CBR1_VLV lose their
> > state
> > when the power well goes down, but fortunately the values we've been
> > writing are actually the reset defaults.
> >
> > MI_ARB_VLV actually retains its value even if the power well was
> > turned
> > off, we just can't access it while the power well is down.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94164
> > Signed-off-by: Ville Syrjälä
>
> The spec doesn't say anything about backing power wells, I assume you
> checked this manually by reading the regs out while the power well was
> off.
Yep.
> Looks ok:
> Reviewed-by: Imre Deak
>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 15 ---
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 13 +
> > 2 files changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 43b24a1f5ee6..c80d044fe082 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6882,23 +6882,10 @@ static void
> > ivybridge_init_clock_gating(struct drm_device *dev)
> > gen6_check_mch_setup(dev);
> > }
> >
> > -static void vlv_init_display_clock_gating(struct drm_i915_private
> > *dev_priv)
> > -{
> > - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> > -
> > - /*
> > - * Disable trickle feed and enable pnd deadline calculation
> > - */
> > - I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> > - I915_WRITE(CBR1_VLV, 0);
> > -}
> > -
> > static void valleyview_init_clock_gating(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - vlv_init_display_clock_gating(dev_priv);
> > -
> > /* WaDisableEarlyCull:vlv */
> > I915_WRITE(_3D_CHICKEN3,
> > _MASKED_BIT_ENABLE(_3D_CHICKEN_SF_DISABLE_OBJEND_
> > CULL));
> > @@ -6981,8 +6968,6 @@ static void cherryview_init_clock_gating(struct
> > drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - vlv_init_display_clock_gating(dev_priv);
> > -
> > /* WaVSRefCountFullforceMissDisable:chv */
> > /* WaDSRefCountFullforceMissDisable:chv */
> > I915_WRITE(GEN7_FF_THREAD_MODE,
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 80e8bd4b43b5..8f9797f17991 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -900,6 +900,17 @@ static bool vlv_power_well_enabled(struct
> > drm_i915_private *dev_priv,
> > return enabled;
> > }
> >
> > +static void vlv_init_display_clock_gating(struct drm_i915_private
> > *dev_priv)
> > +{
> > + I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> > +
> > + /*
> > + * Disable trickle feed and enable pnd deadline calculation
> > + */
> > + I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> > + I915_WRITE(CBR1_VLV, 0);
> > +}
> > +
> > static void vlv_display_power_well_init(struct drm_i915_private
> > *dev_priv)
> > {
> > enum pipe pipe;
> > @@ -922,6 +933,8 @@ static void vlv_display_power_well_init(struct
> > drm_i915_private *dev_priv)
> > I915_WRITE(DPLL(pipe), val);
> > }
> >
> > + vlv_init_display_clock_gating(dev_priv);
> > +
> > spin_lock_irq(&dev_priv->irq_lock);
> > valleyview_enable_display_irqs(dev_priv);
> > spin_unlock_irq(&dev_priv->irq_lock);
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 16/46] pwm: move the enabled/disabled info into pwm_state
On Wed, Mar 30, 2016 at 10:03:39PM +0200, Boris Brezillon wrote:
[...]
> @@ -145,7 +146,11 @@ static inline void pwm_get_state(const struct pwm_device
> *pwm,
>
> static inline bool pwm_is_enabled(const struct pwm_device *pwm)
> {
> - return test_bit(PWMF_ENABLED, &pwm->flags);
> + struct pwm_state pstate;
No need for the p prefix, in my opinion.
Thierry
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915/mocs: Program MOCS for all engines on init
Chris, If the test is ok, can you review-by this patch. Thanks, Peter. On Tue, 22 Mar 2016, Peter Antoine wrote: Chris. Can these patches be reviewed without the tests or are they blocked waiting for the tests. Are the patches acceptable? Thanks, Peter. On Tue, 15 Mar 2016, Peter Antoine wrote: Chris, Testcases are underway, validation are working on them. Peter. On Tue, 15 Mar 2016, Chris Wilson wrote: On Mon, Mar 14, 2016 at 03:11:02PM +, Peter Antoine wrote: Allow for the MOCS to be programmed for all engines. Currently we program the MOCS when the first render batch goes through. This works on most platforms but fails on platforms that do not run a render batch early, i.e. headless servers. The patch now programs all initialised engines on init and the RCS is programmed again within the initial batch. This is done for predictable consistency with regards to the hardware context. Hardware context loading sets the values of the MOCS for RCS and L3CC. Programming them from within the batch makes sure that the render context is valid, no matter what the previous state of the saved-context was. v2: posted correct version to the mailing list. v3: moved programming to within engine->init_hw() (Chris Wilson) v4: code formatting and white-space changes. (Chris Wilson) Signed-off-by: Peter Antoine So testcase? Execute a bunch of MI_STORE_REGISTER_MEM on the various rings in a fresh context each time and confirm the ABI for the first N locations. Repeat across suspend/resume (i.e. make sure the context image maintain the register state). Then verify that freshly constructed contexts also have the correct settings after resume. -Chris -- Peter Antoine (Android Graphics Driver Software Engineer) - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 -- Peter Antoine (Android Graphics Driver Software Engineer) - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 -- Peter Antoine (Android Graphics Driver Software Engineer) - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper
On Mon, Apr 11, 2016 at 02:43:39PM +0100, Lionel Landwerlin wrote: > Color management properties are a bit of an odd use case because > they're not marked as atomic properties. Currently we're not updating > the non atomic values so the drm_crtc_state is out of sync with the > values stored in the crtc object. tbh sounds like this is the bug here - why does the lookup of the correct values stored in the crtc_state drm_atomic_crtc_get_property() not work? Duct-taping over this bug in every place we update these properties (you're just patching one of many) won't scale. Also: Where's the igt for this failure? -Daniel > > v2: Update non atomic values only if commit succeeds (Bob Paauwe) > > v3: Do not access crtc_state after commit, use crtc->state (Maarten > Lankhorst) > > Cc: Maarten Lankhorst > Cc: Bob Paauwe > Cc: > Signed-off-by: Lionel Landwerlin > --- > drivers/gpu/drm/drm_atomic_helper.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 7bf678e..13b86cf 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2979,6 +2979,14 @@ retry: > if (ret) > goto fail; > > + drm_object_property_set_value(&crtc->base, > + config->degamma_lut_property, 0); > + drm_object_property_set_value(&crtc->base, > + config->ctm_property, 0); > + drm_object_property_set_value(&crtc->base, > + config->gamma_lut_property, > + crtc->state->gamma_lut->base.id); > + > /* Driver takes ownership of state on successful commit. */ > > drm_property_unreference_blob(blob); > -- > 2.8.0.rc3 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/10] drm/i915: Move DPINVGTT setup to vlv_display_irq_reset()
On ma, 2016-04-11 at 16:56 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> DPINVGTT lives inside the disp2d power well so we can't frob it unless
> we know the power well is active. Let's this stuff into
> vlv_display_irq_reset() which is only called at the right times so that
> we don't get unclaimed register access errors.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94164
> Signed-off-by: Ville Syrjälä
Reviewed-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_irq.c | 15 +--
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 872f93dc68ff..d60c0e53f929 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3289,6 +3289,11 @@ static void vlv_display_irq_reset(struct
> drm_i915_private *dev_priv)
> {
> enum pipe pipe;
>
> + if (IS_CHERRYVIEW(dev_priv))
> + I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
> + else
> + I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
> +
> i915_hotplug_interrupt_update_locked(dev_priv, 0x,
> 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>
> @@ -3349,8 +3354,6 @@ static void valleyview_irq_preinstall(struct
> drm_device *dev)
>
> gen5_gt_irq_reset(dev);
>
> - I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
> -
> spin_lock_irq(&dev_priv->irq_lock);
> if (dev_priv->display_irqs_enabled)
> vlv_display_irq_reset(dev_priv);
> @@ -3427,8 +3430,6 @@ static void cherryview_irq_preinstall(struct
> drm_device *dev)
>
> GEN5_IRQ_RESET(GEN8_PCU_);
>
> - I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
> -
> spin_lock_irq(&dev_priv->irq_lock);
> if (dev_priv->display_irqs_enabled)
> vlv_display_irq_reset(dev_priv);
> @@ -3714,12 +3715,6 @@ static int valleyview_irq_postinstall(struct
> drm_device *dev)
>
> gen5_gt_irq_postinstall(dev);
>
> - /* ack & enable invalid PTE error interrupts */
> -#if 0 /* FIXME: add support to irq handler for checking these bits
> */
> - I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
> - I915_WRITE(DPINVGTT, DPINVGTT_EN_MASK);
> -#endif
> -
> spin_lock_irq(&dev_priv->irq_lock);
> if (dev_priv->display_irqs_enabled)
> vlv_display_irq_postinstall(dev_priv);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] Revert "drm/i915: Limit the auto arming of mmio debugs on vlv/chv"
On ma, 2016-04-11 at 16:56 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Enable the unclaimd register detection stuff on vlv/chv since we've
> now
> fixed the known problems during suspend.
>
> This reverts commit c81eeea6c14b212016104f4256c65f93ad230a86.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Imre Deak
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index ac2ac07b505b..2f7fb7d169b8 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -633,15 +633,6 @@ __unclaimed_reg_debug(struct drm_i915_private
> *dev_priv,
> const bool read,
> const bool before)
> {
> - /* XXX. We limit the auto arming traces for mmio
> - * debugs on these platforms. There are just too many
> - * revealed by these and CI/Bat suffers from the noise.
> - * Please fix and then re-enable the automatic traces.
> - */
> - if (i915.mmio_debug < 2 &&
> - (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> - return;
> -
> if (WARN(check_for_unclaimed_mmio(dev_priv),
> "Unclaimed register detected %s %s register
> 0x%x\n",
> before ? "before" : "after",
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, 12 Apr 2016 13:39:12 +0200
Thierry Reding wrote:
> On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > Currently the PWM core mixes the current PWM state with the per-platform
> > reference config (specified through the PWM lookup table, DT definition or
> > directly hardcoded in PWM drivers).
> >
> > Create a pwm_args struct to store this reference config, so that PWM users
> > can differentiate the current config from the reference one.
> >
> > Patch all places where pwm->args should be initialized. We keep the
> > pwm_set_polarity/period() calls until all PWM users are patched to
> > use pwm_args instead of pwm_get_period/polarity().
>
> Perhaps a helper would be useful? Something like:
>
> static inline void
> pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> {
> pwm_set_duty_cycle(pwm, args->duty_cycle);
> pwm_set_period(pwm, args->period);
> }
>
> ? That would make it slightly easier to get rid of it again after all
> clients have been converted.
Sure. I'll add this helper.
>
> With the exception of pwm-clps711x all of these args are set at of_xlate
> time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> might even be possible to move this call to the core, so that removal of
> it will be a one-liner.
Not sure I get that one. Some drivers are implementing their own
->of_xlate() method, how would you get rid of this pwm_apply_args() in
those custom implementations?
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 13:49:04 +0200 Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > The PWM state, represented by its period, duty_cycle and polarity, > > is currently directly stored in the PWM device. > > Declare a pwm_state structure embedding those field so that we can later > > use this struct to atomically update all the PWM parameters at once. > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > pwm_get_state(). > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/pwm/core.c | 8 > > include/linux/pwm.h | 54 > > + > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > index 6433059..f3f91e7 100644 > > --- a/drivers/pwm/core.c > > +++ b/drivers/pwm/core.c > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > > pwm->chip = chip; > > pwm->pwm = chip->base + i; > > pwm->hwpwm = i; > > - pwm->polarity = polarity; > > + pwm->state.polarity = polarity; > > Would this not more correctly be assigned to pwm->args.polarity? After > all this is setting up the "initial" state, much like DT or the lookup > tables would for duty cycle and period. Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, all the reference info should be extracted from DT, PWM lookup table or driver specific ->request() implementation, but I can definitely initialize the args.polarity here too. Should I keep the pwm->state.polarity assignment (to set the initial polarity when the driver does not support hardware readout)? -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Restore GMBUS operation after a failed bit-banging fallback
On Mon, Apr 11, 2016 at 12:50:44PM +0300, Jani Nikula wrote:
> On Mon, 07 Mar 2016, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > When the GMBUS based i2c transfer times out, we try to fall back to
> > bit-banging and retry the operation that way. However if the bit-banging
> > attempt also fails, we should probably go back to the GMBUS method for
> > the next attempt. Maybe there simply wasn't anyone one the bus at this
> > time.
> >
> > There's also a bit of a mess going on with the force_bit handling.
> > It's supposed to be a ref count actually, and it is as far as
> > intel_gmbus_force_bit() is concerned. But it's treated as just a
> > flag by the timeout based bit-banging fallback. I suppose that's
> > fine since we should never end up in the timeout fallback case
> > if force_bit was already non-zero. However now that we want to restore
> > things back to where they were after the bit-banging attempt failed,
> > we're going to have to do things a bit differently to avoid clobbering
> > the force_bit count as set up by intel_gmbus_force_bit(). So let's
> > dedicate the high bit as a flag for the low level timeout based fallback
> > and treat the rest of the bits as a ref count just as before.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_i2c.c | 10 +++---
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index f37ac120a29d..2348fea59592 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1043,6 +1043,7 @@ struct intel_fbc_work;
> >
> > struct intel_gmbus {
> > struct i2c_adapter adapter;
> > +#define GMBUS_FORCE_BIT_RETRY (1U << 31)
> > u32 force_bit;
> > u32 reg0;
> > i915_reg_t gpio_reg;
> > diff --git a/drivers/gpu/drm/i915/intel_i2c.c
> > b/drivers/gpu/drm/i915/intel_i2c.c
> > index 7bf8a485e18f..5d4b3604afd2 100644
> > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > @@ -579,7 +579,6 @@ timeout:
> > * Hardware may not support GMBUS over these pins? Try GPIO bitbanging
> > * instead. Use EAGAIN to have i2c core retry.
> > */
> > - bus->force_bit = 1;
> > ret = -EAGAIN;
> >
> > out:
> > @@ -597,10 +596,15 @@ gmbus_xfer(struct i2c_adapter *adapter, struct
> > i2c_msg *msgs, int num)
> > intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> > mutex_lock(&dev_priv->gmbus_mutex);
> >
> > - if (bus->force_bit)
> > + if (bus->force_bit) {
> > ret = i2c_bit_algo.master_xfer(adapter, msgs, num);
> > - else
> > + if (ret < 0)
> > + bus->force_bit &= ~GMBUS_FORCE_BIT_RETRY;
> > + } else {
> > ret = do_gmbus_xfer(adapter, msgs, num);
> > + if (ret == -EAGAIN)
> > + bus->force_bit |= GMBUS_FORCE_BIT_RETRY;
>
> Hmm, would this all be simpler if we did the first bit-banging retry
> here ourselves after all, and set ->force_bit only if bit-banging
> succeeds after gmbus -EAGAIN? I think moving the retry out of
> do_gmbus_xfer() was the right thing to do to, but maybe I went too far
> by pushing it all the way to i2c core?
>
> Anyway, this patch looks good, but it's just a bit subtle with the
> -EAGAIN and one retry and all.
>
> Up to you.
>
> Reviewed-by: Jani Nikula
I left it alone for now, if for no other reason than to avoid
having to redo any testing right now. We can always revisit it later.
Rest of the series pushed to dinq. Thanks for the reviews.
>
> > + }
> >
> > mutex_unlock(&dev_priv->gmbus_mutex);
> > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Get panel_type from OpRegion panel details
On Mon, Apr 11, 2016 at 11:09:57AM +0300, Jani Nikula wrote:
> On Mon, 11 Apr 2016, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > We've had problems on several occasions with using the panel type
> > from the VBT block 40. Usually it seems to be 2, which often
> > doesn't give us the correct timings for the panel. After some
> > more digging I found a way to get a panel type via the OpRegion
> > SWSCI GBDA "Get Panel Details" method. Let's try to use it.
> >
> > The spec has this to say about the output:
> > "Bits [15:8] - Panel Type
> > Bits contain the panel type user setting from CMOS
> > 00h = Not Valid, use default Panel Type & Timings from VBT
> > 01h - 0Fh = Panel Number"
> >
> > Another version of the spec lists the valid range as 1-16, which makes
> > more sense since VBT supports 16 panels. Based on actual results
> > from Rob's G45, 1-16 is what we need to accept.
> >
> > The other bits in the output don't look relevant for the problem at
> > hand.
> >
> > The input is specified as:
> > "Bits [31:4] - Reserved
> > Reserved (must be zero)
> > Bits [3:0] - Panel Number
> > These bits contain the sequential index of Panel, starting at 0 and
> > counting upwards from the first integrated Internal Flat-Panel Display
> > Encoder present, and then from the first external Display Encoder
> > (e.g., S/DVO-B then S/DVO-C) which supports Internal Flat-Panels.
> > 0h - 0Fh = Panel number"
> >
> > For now I've just hardcoded the input panel number as 0. That would seem
> > like a decent choise for LVDS. Not so sure about eDP when port != A.
> >
> > v2: Accept values 1-16
> > Filter out bogus results in opregion code (Jani)
> > Add debug logging for all the different branches (Jani)
> >
> > Cc: Jani Nikula
> > Cc: Rob Kramer
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> > Signed-off-by: Ville Syrjälä
>
> So I don't know if this is the right thing to do or not, and we'll only
> find out with testing, but the code looks good to me.
>
> With that caveat,
>
> Reviewed-by: Jani Nikula
Entire series pushed to dinq. Thanks for the reviews and testing.
>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 5 +
> > drivers/gpu/drm/i915/intel_bios.c | 20 +++-
> > drivers/gpu/drm/i915/intel_opregion.c | 28
> > 3 files changed, 48 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 53ace572b292..42234e17b789 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3421,6 +3421,7 @@ extern int intel_opregion_notify_encoder(struct
> > intel_encoder *intel_encoder,
> > bool enable);
> > extern int intel_opregion_notify_adapter(struct drm_device *dev,
> > pci_power_t state);
> > +extern int intel_opregion_get_panel_type(struct drm_device *dev);
> > #else
> > static inline int intel_opregion_setup(struct drm_device *dev) { return 0;
> > }
> > static inline void intel_opregion_init(struct drm_device *dev) { return; }
> > @@ -3436,6 +3437,10 @@ intel_opregion_notify_adapter(struct drm_device
> > *dev, pci_power_t state)
> > {
> > return 0;
> > }
> > +static inline int intel_opregion_get_panel_type(struct drm_device *dev)
> > +{
> > + return -ENODEV;
> > +}
> > #endif
> >
> > /* intel_acpi.c */
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index d595ca30a7e1..e72dd9a8d6bf 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -205,19 +205,29 @@ parse_lfp_panel_data(struct drm_i915_private
> > *dev_priv,
> > struct drm_display_mode *panel_fixed_mode;
> > int panel_type;
> > int drrs_mode;
> > + int ret;
> >
> > lvds_options = find_section(bdb, BDB_LVDS_OPTIONS);
> > if (!lvds_options)
> > return;
> >
> > dev_priv->vbt.lvds_dither = lvds_options->pixel_dither;
> > - if (lvds_options->panel_type > 0xf) {
> > - DRM_DEBUG_KMS("Invalid VBT panel type 0x%x\n",
> > - lvds_options->panel_type);
> > - return;
> > +
> > + ret = intel_opregion_get_panel_type(dev_priv->dev);
> > + if (ret >= 0) {
> > + WARN_ON(ret > 0xf);
> > + panel_type = ret;
> > + DRM_DEBUG_KMS("Panel type: %d (OpRegion)\n", panel_type);
> > + } else {
> > + if (lvds_options->panel_type > 0xf) {
> > + DRM_DEBUG_KMS("Invalid VBT panel type 0x%x\n",
> > + lvds_options->panel_type);
> > + return;
> > + }
> > + panel_type = lvds_options->panel_type;
> > + DRM_DEBUG_KMS("Panel type: %d (VBT)\n", panel_type);
> > }
> >
> > - panel_type = lvds_options->panel_type;
> > dev_priv->vbt.panel_type = panel_typ
Re: [Intel-gfx] [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, Apr 12, 2016 at 02:04:12PM +0200, Boris Brezillon wrote:
> On Tue, 12 Apr 2016 13:39:12 +0200
> Thierry Reding wrote:
>
> > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > Currently the PWM core mixes the current PWM state with the per-platform
> > > reference config (specified through the PWM lookup table, DT definition or
> > > directly hardcoded in PWM drivers).
> > >
> > > Create a pwm_args struct to store this reference config, so that PWM users
> > > can differentiate the current config from the reference one.
> > >
> > > Patch all places where pwm->args should be initialized. We keep the
> > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > use pwm_args instead of pwm_get_period/polarity().
> >
> > Perhaps a helper would be useful? Something like:
> >
> > static inline void
> > pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > {
> > pwm_set_duty_cycle(pwm, args->duty_cycle);
> > pwm_set_period(pwm, args->period);
> > }
> >
> > ? That would make it slightly easier to get rid of it again after all
> > clients have been converted.
>
> Sure. I'll add this helper.
>
> >
> > With the exception of pwm-clps711x all of these args are set at of_xlate
> > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > might even be possible to move this call to the core, so that removal of
> > it will be a one-liner.
>
> Not sure I get that one. Some drivers are implementing their own
> ->of_xlate() method, how would you get rid of this pwm_apply_args() in
> those custom implementations?
I was proposing to have pwm_apply_args() called from the core.
of_pwm_get() is where ->of_xlate() is called from, and the lookup table
arguments would be applied in pwm_get(). Taking into account clps711x,
which sets the arguments in ->request() it might be possible to simply
call pwm_apply_args() from pwm_device_request(), since that's also
called by all other request functions, even the legacy ones.
That said, the amount of code to modify isn't that large, so I'm fine if
you want to keep sprinkling the calls across multiple files, especially
since it's temporary.
Thierry
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/opregion: remove unnecessary ifdefs on CONFIG_ACPI
On Tue, 12 Apr 2016, Chris Wilson wrote: > On Fri, Apr 08, 2016 at 05:59:49PM +0300, Jani Nikula wrote: >> The whole file is ignored on CONFIG_ACPI=n. > > That's an issue as we can't then acquire the opregion->vbt (which itself > is not acpi dependent). Shrug no modern system can boot without acpi (at > least not if you want more than cpu etc), so I guess we also don't care > that much? Interesting, I thought we shouldn't touch ACPI OpRegion for CONFIG_ACPI=n, but seems that this was changed only in commit 27d50c82714f6477ac690034b37d202f76eb4f70 Author: Lv Zheng Date: Fri Dec 6 16:52:05 2013 +0800 ACPI / i915: Fix incorrect inclusions via So this patch here doesn't change the fact, and since there hasn't been complaints after the above commit, I wouldn't worry too much. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > On Tue, 12 Apr 2016 13:49:04 +0200 > Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > The PWM state, represented by its period, duty_cycle and polarity, > > > is currently directly stored in the PWM device. > > > Declare a pwm_state structure embedding those field so that we can later > > > use this struct to atomically update all the PWM parameters at once. > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > pwm_get_state(). > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > drivers/pwm/core.c | 8 > > > include/linux/pwm.h | 54 > > > + > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > index 6433059..f3f91e7 100644 > > > --- a/drivers/pwm/core.c > > > +++ b/drivers/pwm/core.c > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > > > pwm->chip = chip; > > > pwm->pwm = chip->base + i; > > > pwm->hwpwm = i; > > > - pwm->polarity = polarity; > > > + pwm->state.polarity = polarity; > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > all this is setting up the "initial" state, much like DT or the lookup > > tables would for duty cycle and period. > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > all the reference info should be extracted from DT, PWM lookup table or > driver specific ->request() implementation, but I can definitely > initialize the args.polarity here too. > > Should I keep the pwm->state.polarity assignment (to set the initial > polarity when the driver does not support hardware readout)? Wouldn't this work automatically as part of the pwm_apply_args() helper if we extended it with this setting? Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 14:21:41 +0200 Thierry Reding wrote: > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > On Tue, 12 Apr 2016 13:49:04 +0200 > > Thierry Reding wrote: > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > is currently directly stored in the PWM device. > > > > Declare a pwm_state structure embedding those field so that we can later > > > > use this struct to atomically update all the PWM parameters at once. > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > pwm_get_state(). > > > > > > > > Signed-off-by: Boris Brezillon > > > > --- > > > > drivers/pwm/core.c | 8 > > > > include/linux/pwm.h | 54 > > > > + > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > index 6433059..f3f91e7 100644 > > > > --- a/drivers/pwm/core.c > > > > +++ b/drivers/pwm/core.c > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, > > > > pwm->chip = chip; > > > > pwm->pwm = chip->base + i; > > > > pwm->hwpwm = i; > > > > - pwm->polarity = polarity; > > > > + pwm->state.polarity = polarity; > > > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > > all this is setting up the "initial" state, much like DT or the lookup > > > tables would for duty cycle and period. > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > all the reference info should be extracted from DT, PWM lookup table or > > driver specific ->request() implementation, but I can definitely > > initialize the args.polarity here too. > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > polarity when the driver does not support hardware readout)? > > Wouldn't this work automatically as part of the pwm_apply_args() helper > if we extended it with this setting? Well, as you explained in you answer to patch 5, pwm_apply_args() should be called on a per-request basis (each time a PWM device is requested), while the initial polarity setting should only be applied when registering the PWM chip (and its devices). After that, the framework takes care of keeping the PWM state in sync with the hardware state. Let's take a real (though a bit unusual) example. Say you have a single PWM device referenced by two different users. Only one user can be enabled at a time, but each of them has its own reference config (different polarity, different period). User1 calls pwm_get() and applies its own polarity and period. Then user1 is unregistered and release the PWM device, leaving the polarity and period untouched. User2 is registered and request the same PWM device, but user2 is smarter and tries to extract the current PWM state before adapting the config according to pwm_args. If you just reset pwm->state.polarity each time pwm_apply_args() is called (and you suggested to call it as part of the request procedure), then this means the PWM state is no longer in sync with the hardware state. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper
On 12/04/16 12:57, Daniel Vetter wrote: On Mon, Apr 11, 2016 at 02:43:39PM +0100, Lionel Landwerlin wrote: Color management properties are a bit of an odd use case because they're not marked as atomic properties. Currently we're not updating the non atomic values so the drm_crtc_state is out of sync with the values stored in the crtc object. tbh sounds like this is the bug here - why does the lookup of the correct values stored in the crtc_state drm_atomic_crtc_get_property() not work? This is only a problem when the kernel space modifies property values. User space ioctls do the right thing and update both places : https://cgit.freedesktop.org/drm-intel/tree/drivers/gpu/drm/drm_crtc.c#n4916 Duct-taping over this bug in every place we update these properties (you're just patching one of many) won't scale. Also: Where's the igt for this failure? -Daniel There is : https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/kms_pipe_color.c#n554 That's how we caught it. v2: Update non atomic values only if commit succeeds (Bob Paauwe) v3: Do not access crtc_state after commit, use crtc->state (Maarten Lankhorst) Cc: Maarten Lankhorst Cc: Bob Paauwe Cc: Signed-off-by: Lionel Landwerlin --- drivers/gpu/drm/drm_atomic_helper.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 7bf678e..13b86cf 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2979,6 +2979,14 @@ retry: if (ret) goto fail; + drm_object_property_set_value(&crtc->base, + config->degamma_lut_property, 0); + drm_object_property_set_value(&crtc->base, + config->ctm_property, 0); + drm_object_property_set_value(&crtc->base, + config->gamma_lut_property, + crtc->state->gamma_lut->base.id); + /* Driver takes ownership of state on successful commit. */ drm_property_unreference_blob(blob); -- 2.8.0.rc3 ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, 12 Apr 2016 14:20:29 +0200
Thierry Reding wrote:
> On Tue, Apr 12, 2016 at 02:04:12PM +0200, Boris Brezillon wrote:
> > On Tue, 12 Apr 2016 13:39:12 +0200
> > Thierry Reding wrote:
> >
> > > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > > Currently the PWM core mixes the current PWM state with the per-platform
> > > > reference config (specified through the PWM lookup table, DT definition
> > > > or
> > > > directly hardcoded in PWM drivers).
> > > >
> > > > Create a pwm_args struct to store this reference config, so that PWM
> > > > users
> > > > can differentiate the current config from the reference one.
> > > >
> > > > Patch all places where pwm->args should be initialized. We keep the
> > > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > > use pwm_args instead of pwm_get_period/polarity().
> > >
> > > Perhaps a helper would be useful? Something like:
> > >
> > > static inline void
> > > pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > > {
> > > pwm_set_duty_cycle(pwm, args->duty_cycle);
> > > pwm_set_period(pwm, args->period);
> > > }
> > >
> > > ? That would make it slightly easier to get rid of it again after all
> > > clients have been converted.
> >
> > Sure. I'll add this helper.
> >
> > >
> > > With the exception of pwm-clps711x all of these args are set at of_xlate
> > > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > > might even be possible to move this call to the core, so that removal of
> > > it will be a one-liner.
> >
> > Not sure I get that one. Some drivers are implementing their own
> > ->of_xlate() method, how would you get rid of this pwm_apply_args() in
> > those custom implementations?
>
> I was proposing to have pwm_apply_args() called from the core.
> of_pwm_get() is where ->of_xlate() is called from, and the lookup table
> arguments would be applied in pwm_get(). Taking into account clps711x,
> which sets the arguments in ->request() it might be possible to simply
> call pwm_apply_args() from pwm_device_request(), since that's also
> called by all other request functions, even the legacy ones.
>
> That said, the amount of code to modify isn't that large, so I'm fine if
> you want to keep sprinkling the calls across multiple files, especially
> since it's temporary.
No, I'm fine addressing that, but I just don't get where you'd get the
pwm_args to apply. Do you suggest to pass 'struct pwm_args *' to the
->of_xlate() and ->request() methods?
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Use new i915_gem_object_pin_map for LRC
From: Tvrtko Ursulin
We can use the new pin/lazy unpin API for simplicity
and more performance in the execlist submission paths.
v2:
* Fix error handling and convert more users.
* Compact some names for readability.
v3:
* intel_lr_context_free was not unpinning.
* Special case for GPU reset which otherwise unbalances
the HWS object pages pin count by running the engine
initialization only (not destructors).
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c| 107 +++-
drivers/gpu/drm/i915/intel_lrc.h| 7 ++-
3 files changed, 69 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index fe580cb9501a..91028d9c6269 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -342,7 +342,7 @@ void i915_gem_context_reset(struct drm_device *dev)
struct intel_context *ctx;
list_for_each_entry(ctx, &dev_priv->context_list, link)
- intel_lr_context_reset(dev, ctx);
+ intel_lr_context_reset(dev_priv, ctx);
}
for (i = 0; i < I915_NUM_ENGINES; i++) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0d6dc5ec4a46..cbb54d91eaed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -229,8 +229,9 @@ enum {
static int intel_lr_context_pin(struct intel_context *ctx,
struct intel_engine_cs *engine);
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
- struct drm_i915_gem_object
*default_ctx_obj);
+static int
+lrc_setup_hws(struct intel_engine_cs *engine,
+ struct drm_i915_gem_object *default_ctx_obj);
/**
@@ -1093,8 +1094,8 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
struct intel_ringbuffer *ringbuf = ctx->engine[engine->id].ringbuf;
- struct page *lrc_state_page;
- uint32_t *lrc_reg_state;
+ void *obj_addr;
+ u32 *lrc_reg_state;
int ret;
WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
@@ -1104,19 +1105,20 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
if (ret)
return ret;
- lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
- if (WARN_ON(!lrc_state_page)) {
- ret = -ENODEV;
+ obj_addr = i915_gem_object_pin_map(ctx_obj);
+ if (IS_ERR(obj_addr)) {
+ ret = PTR_ERR(obj_addr);
goto unpin_ctx_obj;
}
+ lrc_reg_state = obj_addr + LRC_STATE_PN * PAGE_SIZE;
+
ret = intel_pin_and_map_ringbuffer_obj(engine->dev, ringbuf);
if (ret)
- goto unpin_ctx_obj;
+ goto unpin_map;
ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, engine);
- lrc_reg_state = kmap(lrc_state_page);
lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
ctx_obj->dirty = true;
@@ -1127,6 +1129,8 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
return ret;
+unpin_map:
+ i915_gem_object_unpin_map(ctx_obj);
unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
@@ -1159,7 +1163,7 @@ void intel_lr_context_unpin(struct intel_context *ctx,
WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
if (--ctx->engine[engine->id].pin_count == 0) {
- kunmap(kmap_to_page(ctx->engine[engine->id].lrc_reg_state));
+ i915_gem_object_unpin_map(ctx_obj);
intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
ctx->engine[engine->id].lrc_vma = NULL;
@@ -1584,9 +1588,12 @@ static int gen8_init_common_ring(struct intel_engine_cs
*engine)
struct drm_device *dev = engine->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned int next_context_status_buffer_hw;
+ int ret;
- lrc_setup_hardware_status_page(engine,
-
dev_priv->kernel_context->engine[engine->id].state);
+ ret = lrc_setup_hws(engine,
+ dev_priv->kernel_context->engine[engine->id].state);
+ if (ret)
+ return ret;
I915_WRITE_IMR(engine,
~(engine->irq_enable_mask | engine->irq_keep_mask));
@@ -2048,7 +2055,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
*engi
Re: [Intel-gfx] [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, 12 Apr 2016 13:39:12 +0200
Thierry Reding wrote:
> On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > Currently the PWM core mixes the current PWM state with the per-platform
> > reference config (specified through the PWM lookup table, DT definition or
> > directly hardcoded in PWM drivers).
> >
> > Create a pwm_args struct to store this reference config, so that PWM users
> > can differentiate the current config from the reference one.
> >
> > Patch all places where pwm->args should be initialized. We keep the
> > pwm_set_polarity/period() calls until all PWM users are patched to
> > use pwm_args instead of pwm_get_period/polarity().
>
> Perhaps a helper would be useful? Something like:
>
> static inline void
> pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> {
> pwm_set_duty_cycle(pwm, args->duty_cycle);
> pwm_set_period(pwm, args->period);
> }
>
> ? That would make it slightly easier to get rid of it again after all
> clients have been converted.
>
> With the exception of pwm-clps711x all of these args are set at of_xlate
> time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> might even be possible to move this call to the core, so that removal of
> it will be a one-liner.
Okay, I think I misunderstood your suggestion. I thought you wanted
this helper to set the reference config, but you actually want to apply
a new state based on the PWM reference values.
Except that pwm_args does not contain all the required information to
apply a full config (args->duty_cycle and args->enable do not exist).
This being said, in my v6 I moved the content of
pwm_regulator_adjust_pwm_config() (patch 27) into a generic helper
(pwm_adjust_config()). This helper is doing pretty much what you're
suggesting here (but again, I'm not sure I correctly understood your
suggestion :-/).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > On Tue, 12 Apr 2016 14:21:41 +0200 > Thierry Reding wrote: > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > Thierry Reding wrote: > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > > is currently directly stored in the PWM device. > > > > > Declare a pwm_state structure embedding those field so that we can > > > > > later > > > > > use this struct to atomically update all the PWM parameters at once. > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > pwm_get_state(). > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > --- > > > > > drivers/pwm/core.c | 8 > > > > > include/linux/pwm.h | 54 > > > > > + > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > index 6433059..f3f91e7 100644 > > > > > --- a/drivers/pwm/core.c > > > > > +++ b/drivers/pwm/core.c > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip > > > > > *chip, > > > > > pwm->chip = chip; > > > > > pwm->pwm = chip->base + i; > > > > > pwm->hwpwm = i; > > > > > - pwm->polarity = polarity; > > > > > + pwm->state.polarity = polarity; > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > > > all this is setting up the "initial" state, much like DT or the lookup > > > > tables would for duty cycle and period. > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > all the reference info should be extracted from DT, PWM lookup table or > > > driver specific ->request() implementation, but I can definitely > > > initialize the args.polarity here too. > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > polarity when the driver does not support hardware readout)? > > > > Wouldn't this work automatically as part of the pwm_apply_args() helper > > if we extended it with this setting? > > Well, as you explained in you answer to patch 5, pwm_apply_args() > should be called on a per-request basis (each time a PWM device is > requested), while the initial polarity setting should only be applied > when registering the PWM chip (and its devices). After that, the > framework takes care of keeping the PWM state in sync with the hardware > state. > > Let's take a real (though a bit unusual) example. Say you have a single > PWM device referenced by two different users. Only one user can be > enabled at a time, but each of them has its own reference config > (different polarity, different period). > > User1 calls pwm_get() and applies its own polarity and period. Then > user1 is unregistered and release the PWM device, leaving the polarity > and period untouched. > > User2 is registered and request the same PWM device, but user2 is > smarter and tries to extract the current PWM state before adapting the > config according to pwm_args. If you just reset pwm->state.polarity > each time pwm_apply_args() is called (and you suggested to call it as > part of the request procedure), then this means the PWM state is no > longer in sync with the hardware state. In that case neither will be the period or duty cycle. Essentially this gets us back to square one where we need to decide how to handle current state vs. initial arguments. But I don't think this is really going to be an issue because this is all moot until we've moved over to the atomic API, at which point this is all going to go away anyway. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Use new i915_gem_object_pin_map for LRC
On Tue, Apr 12, 2016 at 02:05:05PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> We can use the new pin/lazy unpin API for simplicity
> and more performance in the execlist submission paths.
>
> v2:
> * Fix error handling and convert more users.
> * Compact some names for readability.
>
> v3:
> * intel_lr_context_free was not unpinning.
> * Special case for GPU reset which otherwise unbalances
> the HWS object pages pin count by running the engine
> initialization only (not destructors).
Ah! Light dawns...
Should we not just separate out the hws setup and hws hw_init?
> -static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
> -struct drm_i915_gem_object
> *default_ctx_obj)
> +static int
> +lrc_setup_hws(struct intel_engine_cs *engine,
> + struct drm_i915_gem_object *def_ctx_obj)
> {
> struct drm_i915_private *dev_priv = engine->dev->dev_private;
> - struct page *page;
> + void *hws;
>
> /* The HWSP is part of the default context object in LRC mode. */
> - engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
> - + LRC_PPHWSP_PN * PAGE_SIZE;
> - page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
> - engine->status_page.page_addr = kmap(page);
> - engine->status_page.obj = default_ctx_obj;
> + engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(def_ctx_obj) +
> +LRC_PPHWSP_PN * PAGE_SIZE;
> + hws = i915_gem_object_pin_map(def_ctx_obj);
> + if (IS_ERR(hws))
> + return PTR_ERR(hws);
> + engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
> + engine->status_page.obj = def_ctx_obj;
>
> I915_WRITE(RING_HWS_PGA(engine->mmio_base),
> - (u32)engine->status_page.gfx_addr);
> +(u32)engine->status_page.gfx_addr);
> POSTING_READ(RING_HWS_PGA(engine->mmio_base));
> +
> + return 0;
> }
>
> /**
> @@ -2688,10 +2700,9 @@ error_deref_obj:
> return ret;
> }
>
> -void intel_lr_context_reset(struct drm_device *dev,
> - struct intel_context *ctx)
> +void intel_lr_context_reset(struct drm_i915_private *dev_priv,
> + struct intel_context *ctx)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *engine;
>
> for_each_engine(engine, dev_priv) {
> @@ -2699,23 +2710,31 @@ void intel_lr_context_reset(struct drm_device *dev,
> ctx->engine[engine->id].state;
> struct intel_ringbuffer *ringbuf =
> ctx->engine[engine->id].ringbuf;
> + void *obj_addr;
> uint32_t *reg_state;
> - struct page *page;
>
> if (!ctx_obj)
> continue;
>
> - if (i915_gem_object_get_pages(ctx_obj)) {
> - WARN(1, "Failed get_pages for context obj\n");
> + obj_addr = i915_gem_object_pin_map(ctx_obj);
> + if (WARN_ON(IS_ERR(obj_addr)))
> continue;
> - }
> - page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
> - reg_state = kmap_atomic(page);
> +
> + reg_state = obj_addr + LRC_STATE_PN * PAGE_SIZE;
> + ctx_obj->dirty = true;
>
> reg_state[CTX_RING_HEAD+1] = 0;
> reg_state[CTX_RING_TAIL+1] = 0;
>
> - kunmap_atomic(reg_state);
> + i915_gem_object_unpin_map(ctx_obj);
> +
> + /*
> + * Kernel context will pin_map the HWS after reset so we
> + * have to drop the pin count here in order ->pages_pin_count
> + * remains balanced.
> + */
> + if (ctx == dev_priv->kernel_context)
> + i915_gem_object_unpin_map(engine->status_page.obj);
Then we wouldn't have this kludge. That's going to be worth the time and
we can then split out the bug fix for the current code.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/46] pwm: introduce the pwm_args concept
On Tue, Apr 12, 2016 at 03:06:27PM +0200, Boris Brezillon wrote:
> On Tue, 12 Apr 2016 13:39:12 +0200
> Thierry Reding wrote:
>
> > On Wed, Mar 30, 2016 at 10:03:28PM +0200, Boris Brezillon wrote:
> > > Currently the PWM core mixes the current PWM state with the per-platform
> > > reference config (specified through the PWM lookup table, DT definition or
> > > directly hardcoded in PWM drivers).
> > >
> > > Create a pwm_args struct to store this reference config, so that PWM users
> > > can differentiate the current config from the reference one.
> > >
> > > Patch all places where pwm->args should be initialized. We keep the
> > > pwm_set_polarity/period() calls until all PWM users are patched to
> > > use pwm_args instead of pwm_get_period/polarity().
> >
> > Perhaps a helper would be useful? Something like:
> >
> > static inline void
> > pwm_apply_args(struct pwm_device *pwm, const struct pwm_args *args)
> > {
> > pwm_set_duty_cycle(pwm, args->duty_cycle);
> > pwm_set_period(pwm, args->period);
> > }
> >
> > ? That would make it slightly easier to get rid of it again after all
> > clients have been converted.
> >
> > With the exception of pwm-clps711x all of these args are set at of_xlate
> > time (for DT) or from the lookup table in pwm_get() (for non-DT), so it
> > might even be possible to move this call to the core, so that removal of
> > it will be a one-liner.
>
> Okay, I think I misunderstood your suggestion. I thought you wanted
> this helper to set the reference config, but you actually want to apply
> a new state based on the PWM reference values.
>
> Except that pwm_args does not contain all the required information to
> apply a full config (args->duty_cycle and args->enable do not exist).
>
> This being said, in my v6 I moved the content of
> pwm_regulator_adjust_pwm_config() (patch 27) into a generic helper
> (pwm_adjust_config()). This helper is doing pretty much what you're
> suggesting here (but again, I'm not sure I correctly understood your
> suggestion :-/).
I'm not suggesting that pwm_apply_args() apply any state. I think we
both agreed earlier that the initial state (represented by pwm_args) was
never to be automatically applied.
What I was suggesting is that we move all the calls to pwm_set_period()
and pwm_set_duty_cycle() into a central location to make it easier to
remove them later in the series. This is really only temporary, so I
don't mind if we leave the calls sprinkled all over the place. At least
that way I hope we'll avoid confusion about what we're talking about =)
Thierry
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Only grab correct forcewake for the engine with execlists
On Thu, Apr 07, 2016 at 04:56:00PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Rather than blindly waking up all forcewake domains on command
> submission, we can teach each engine what is (or are) the correct
> one to take.
>
> On platforms with multiple forcewake domains like VLV, CHV, SKL
> and BXT, this has the potential of lowering the GPU and CPU
> power use and submission latency.
>
> To implement it we add a function named
> intel_uncore_forcewake_for_reg whose purpose is to query which
> forcewake domains need to be taken to read or write a specific
> register with raw mmio accessors.
>
> These enables the execlists engine setup to query which
> forcewake domains are relevant per engine on the currently
> running platform.
>
> v2:
> * Kerneldoc.
> * Split from intel_uncore.c macro extraction, WARN_ON,
> no warns on old platforms. (Chris Wilson)
>
> v3:
> * Single domain per engine, mention all registers,
> bi-directional function and a new name, fix handling
> of gen6 and gen7 writes. (Chris Wilson)
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Chris Wilson
> +/**
> + * intel_uncore_forcewake_for_reg - which forcewake domains are needed to
> access
> + * a register
> + * @dev_priv: pointer to struct drm_i915_private
> + * @reg: register in question
> + * @op: operation bitmask of FW_REG_READ and/or FW_REG_WRITE
> + *
> + * Returns a set of forcewake domains required to be taken with for example
> + * intel_uncore_forcewake_get for the specified register to be accessible in
> the
> + * specified mode (read, write or read/write) with raw mmio accessors.
> + *
> + * NOTE: On Gen6 and Gen7 write forcewake domain (FORCEWAKE_RENDER) requires
> the
> + * callers to do FIFO management on their own or risk losing writes.
> + */
> +enum forcewake_domains
> +intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
> +i915_reg_t reg, unsigned int op)
> +{
> + enum forcewake_domains fw_domains = 0;
> +
> + WARN_ON(!op);
> +
> + if (op & FW_REG_READ)
> + fw_domains = intel_uncore_forcewake_for_read(dev_priv, reg);
> +
> + if (op & FW_REG_WRITE)
> + fw_domains |= intel_uncore_forcewake_for_write(dev_priv, reg);
> +
> + return fw_domains;
> +}
Like it, like it a lot.
Reviewed-by: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Use new i915_gem_object_pin_map for LRC
On 12/04/16 14:12, Chris Wilson wrote:
On Tue, Apr 12, 2016 at 02:05:05PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
We can use the new pin/lazy unpin API for simplicity
and more performance in the execlist submission paths.
v2:
* Fix error handling and convert more users.
* Compact some names for readability.
v3:
* intel_lr_context_free was not unpinning.
* Special case for GPU reset which otherwise unbalances
the HWS object pages pin count by running the engine
initialization only (not destructors).
Ah! Light dawns...
Should we not just separate out the hws setup and hws hw_init?
Okay...
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
- struct drm_i915_gem_object
*default_ctx_obj)
+static int
+lrc_setup_hws(struct intel_engine_cs *engine,
+ struct drm_i915_gem_object *def_ctx_obj)
{
struct drm_i915_private *dev_priv = engine->dev->dev_private;
- struct page *page;
+ void *hws;
/* The HWSP is part of the default context object in LRC mode. */
- engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
- + LRC_PPHWSP_PN * PAGE_SIZE;
- page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
- engine->status_page.page_addr = kmap(page);
- engine->status_page.obj = default_ctx_obj;
+ engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(def_ctx_obj) +
+ LRC_PPHWSP_PN * PAGE_SIZE;
+ hws = i915_gem_object_pin_map(def_ctx_obj);
+ if (IS_ERR(hws))
+ return PTR_ERR(hws);
+ engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
+ engine->status_page.obj = def_ctx_obj;
... so above here is setup and below is init, correct?
I915_WRITE(RING_HWS_PGA(engine->mmio_base),
- (u32)engine->status_page.gfx_addr);
+ (u32)engine->status_page.gfx_addr);
POSTING_READ(RING_HWS_PGA(engine->mmio_base));
+
+ return 0;
}
/**
@@ -2688,10 +2700,9 @@ error_deref_obj:
return ret;
}
-void intel_lr_context_reset(struct drm_device *dev,
- struct intel_context *ctx)
+void intel_lr_context_reset(struct drm_i915_private *dev_priv,
+ struct intel_context *ctx)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
for_each_engine(engine, dev_priv) {
@@ -2699,23 +2710,31 @@ void intel_lr_context_reset(struct drm_device *dev,
ctx->engine[engine->id].state;
struct intel_ringbuffer *ringbuf =
ctx->engine[engine->id].ringbuf;
+ void *obj_addr;
uint32_t *reg_state;
- struct page *page;
if (!ctx_obj)
continue;
- if (i915_gem_object_get_pages(ctx_obj)) {
- WARN(1, "Failed get_pages for context obj\n");
+ obj_addr = i915_gem_object_pin_map(ctx_obj);
+ if (WARN_ON(IS_ERR(obj_addr)))
continue;
- }
- page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
- reg_state = kmap_atomic(page);
+
+ reg_state = obj_addr + LRC_STATE_PN * PAGE_SIZE;
+ ctx_obj->dirty = true;
reg_state[CTX_RING_HEAD+1] = 0;
reg_state[CTX_RING_TAIL+1] = 0;
- kunmap_atomic(reg_state);
+ i915_gem_object_unpin_map(ctx_obj);
+
+ /*
+* Kernel context will pin_map the HWS after reset so we
+* have to drop the pin count here in order ->pages_pin_count
+* remains balanced.
+*/
+ if (ctx == dev_priv->kernel_context)
+ i915_gem_object_unpin_map(engine->status_page.obj);
Then we wouldn't have this kludge. That's going to be worth the time and
we can then split out the bug fix for the current code.
Yes should not be adding more mines to the minefield. :)
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 15:11:18 +0200 Thierry Reding wrote: > On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > > On Tue, 12 Apr 2016 14:21:41 +0200 > > Thierry Reding wrote: > > > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > > Thierry Reding wrote: > > > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > > > is currently directly stored in the PWM device. > > > > > > Declare a pwm_state structure embedding those field so that we can > > > > > > later > > > > > > use this struct to atomically update all the PWM parameters at once. > > > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > > pwm_get_state(). > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > --- > > > > > > drivers/pwm/core.c | 8 > > > > > > include/linux/pwm.h | 54 > > > > > > + > > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > > index 6433059..f3f91e7 100644 > > > > > > --- a/drivers/pwm/core.c > > > > > > +++ b/drivers/pwm/core.c > > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip > > > > > > *chip, > > > > > > pwm->chip = chip; > > > > > > pwm->pwm = chip->base + i; > > > > > > pwm->hwpwm = i; > > > > > > - pwm->polarity = polarity; > > > > > > + pwm->state.polarity = polarity; > > > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? After > > > > > all this is setting up the "initial" state, much like DT or the lookup > > > > > tables would for duty cycle and period. > > > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > > all the reference info should be extracted from DT, PWM lookup table or > > > > driver specific ->request() implementation, but I can definitely > > > > initialize the args.polarity here too. > > > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > > polarity when the driver does not support hardware readout)? > > > > > > Wouldn't this work automatically as part of the pwm_apply_args() helper > > > if we extended it with this setting? > > > > Well, as you explained in you answer to patch 5, pwm_apply_args() > > should be called on a per-request basis (each time a PWM device is > > requested), while the initial polarity setting should only be applied > > when registering the PWM chip (and its devices). After that, the > > framework takes care of keeping the PWM state in sync with the hardware > > state. > > > > Let's take a real (though a bit unusual) example. Say you have a single > > PWM device referenced by two different users. Only one user can be > > enabled at a time, but each of them has its own reference config > > (different polarity, different period). > > > > User1 calls pwm_get() and applies its own polarity and period. Then > > user1 is unregistered and release the PWM device, leaving the polarity > > and period untouched. > > > > User2 is registered and request the same PWM device, but user2 is > > smarter and tries to extract the current PWM state before adapting the > > config according to pwm_args. If you just reset pwm->state.polarity > > each time pwm_apply_args() is called (and you suggested to call it as > > part of the request procedure), then this means the PWM state is no > > longer in sync with the hardware state. > > In that case neither will be the period or duty cycle. Essentially this > gets us back to square one where we need to decide how to handle current > state vs. initial arguments. That's not true. Now we clearly differentiate the reference config (content of pwm_args which is only a subset of what you'll find in pwm_state) and the PWM state (represented by pwm_state). We should be safe as long as we keep those 2 elements as 2 orthogonal concepts: - pwm_args is supposed to give some hint to the PWM user to help him configure it's PWM appropriately - pwm_state is here to reflect the real PWM state, and apply new configs > > But I don't think this is really going to be an issue because this is > all moot until we've moved over to the atomic API, at which point this > is all going to go away anyway. As stated in my answer to patch 5, I think I misunderstood your suggestion. pwm_apply_args() is supposed to adjust the PWM config to match the period and polarity specified in pwm_args, right? If that's the case, my question is, should we really call this function each time a new user requests a PWM instead of letting those users call the function on-demand (not all users want to adapt the current PWM config to the pwm_args, some may just want to apply a completely new
Re: [Intel-gfx] [PATCH v3] drm/i915: Use new i915_gem_object_pin_map for LRC
On Tue, Apr 12, 2016 at 02:19:54PM +0100, Tvrtko Ursulin wrote:
>
> On 12/04/16 14:12, Chris Wilson wrote:
> >On Tue, Apr 12, 2016 at 02:05:05PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin
> >>
> >>We can use the new pin/lazy unpin API for simplicity
> >>and more performance in the execlist submission paths.
> >>
> >>v2:
> >> * Fix error handling and convert more users.
> >> * Compact some names for readability.
> >>
> >>v3:
> >> * intel_lr_context_free was not unpinning.
> >> * Special case for GPU reset which otherwise unbalances
> >> the HWS object pages pin count by running the engine
> >> initialization only (not destructors).
> >
> >Ah! Light dawns...
> >
> >Should we not just separate out the hws setup and hws hw_init?
>
> Okay...
>
> >>-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
> >>- struct drm_i915_gem_object
> >>*default_ctx_obj)
> >>+static int
> >>+lrc_setup_hws(struct intel_engine_cs *engine,
> >>+ struct drm_i915_gem_object *def_ctx_obj)
> >> {
> >>struct drm_i915_private *dev_priv = engine->dev->dev_private;
> >>- struct page *page;
> >>+ void *hws;
> >>
> >>/* The HWSP is part of the default context object in LRC mode. */
> >>- engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
> >>- + LRC_PPHWSP_PN * PAGE_SIZE;
> >>- page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
> >>- engine->status_page.page_addr = kmap(page);
> >>- engine->status_page.obj = default_ctx_obj;
> >>+ engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(def_ctx_obj) +
> >>+ LRC_PPHWSP_PN * PAGE_SIZE;
> >>+ hws = i915_gem_object_pin_map(def_ctx_obj);
> >>+ if (IS_ERR(hws))
> >>+ return PTR_ERR(hws);
> >>+ engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
> >>+ engine->status_page.obj = def_ctx_obj;
>
> ... so above here is setup and below is init, correct?
>
Yes, allocating the mapping is setup; writing the register is hw_init.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [resend-for-CI,1/3] drm/i915: Use consistent forcewake auto-release timeout across kernel configs
On 08/04/16 08:02, Patchwork wrote: == Series Details == Series: series starting with [resend-for-CI,1/3] drm/i915: Use consistent forcewake auto-release timeout across kernel configs URL : https://patchwork.freedesktop.org/series/5424/ State : failure == Summary == Series 5424v1 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/5424/revisions/1/mbox/ Test drv_getparams_basic: Subgroup basic-subslice-total: dmesg-warn -> PASS (bsw-nuc-2) Test drv_module_reload_basic: incomplete -> PASS (bsw-nuc-2) Test gem_basic: Subgroup bad-close: dmesg-warn -> PASS (bsw-nuc-2) Subgroup create-close: dmesg-warn -> PASS (bsw-nuc-2) Subgroup create-fd-close: dmesg-warn -> PASS (bsw-nuc-2) Test gem_ctx_param_basic: Subgroup basic: dmesg-warn -> PASS (bsw-nuc-2) Subgroup invalid-ctx-set: dmesg-warn -> PASS (bsw-nuc-2) Subgroup invalid-param-get: dmesg-warn -> PASS (bsw-nuc-2) Subgroup invalid-param-set: dmesg-warn -> PASS (bsw-nuc-2) Subgroup root-set: dmesg-warn -> PASS (bsw-nuc-2) Test gem_ctx_switch: Subgroup basic-default: skip -> PASS (bsw-nuc-2) Test gem_exec_basic: Subgroup basic-blt: skip -> PASS (bsw-nuc-2) Subgroup gtt-bsd: skip -> PASS (bsw-nuc-2) Subgroup readonly-bsd: skip -> PASS (bsw-nuc-2) Subgroup readonly-render: skip -> PASS (bsw-nuc-2) Subgroup readonly-vebox: skip -> PASS (bsw-nuc-2) Test gem_exec_store: Subgroup basic-bsd: skip -> PASS (bsw-nuc-2) Subgroup basic-default: skip -> PASS (bsw-nuc-2) Subgroup basic-vebox: skip -> PASS (bsw-nuc-2) Test gem_exec_whisper: Subgroup basic: skip -> PASS (bsw-nuc-2) Test gem_mmap: Subgroup basic-small-bo: dmesg-warn -> PASS (bsw-nuc-2) Test gem_mmap_gtt: Subgroup basic-write: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-write-gtt: dmesg-warn -> PASS (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-hang: skip -> PASS (bsw-nuc-2) Test gem_sync: Subgroup basic-blt: skip -> PASS (bsw-nuc-2) Subgroup basic-vebox: skip -> PASS (bsw-nuc-2) Test gem_tiled_pread_basic: dmesg-warn -> PASS (bsw-nuc-2) Test kms_addfb_basic: Subgroup addfb25-modifier-no-flag: dmesg-warn -> PASS (bsw-nuc-2) Subgroup addfb25-y-tiled: dmesg-warn -> PASS (bsw-nuc-2) Subgroup bad-pitch-65536: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-x-tiled: dmesg-warn -> PASS (bsw-nuc-2) Subgroup framebuffer-vs-set-tiling: dmesg-warn -> PASS (bsw-nuc-2) Subgroup too-wide: dmesg-warn -> PASS (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Old friend unclaimed access prior to suspending: https://bugs.freedesktop.org/show_bug.cgi?id=94164 Subgroup basic-flip-vs-wf_vblank: dmesg-fail -> PASS (bsw-nuc-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-c-frame-sequence: dmesg-fail -> PASS (bsw-nuc-2) Subgroup suspend-read-crc-pipe-c: dmesg-warn -> PASS (bsw-nuc-2) Test prime_self_import: Subgroup basic-with_fd_dup: dmesg-warn -> PASS (bsw-nuc-2) Subgroup basic-with_one_bo_two_files: dmesg-warn -> PASS (bsw-nuc-2) bdw-ultratotal:196 pass:175 dwarn:0 dfail:0 fail:0 skip:21 bsw-nuc-2total:196 pass:158 dwarn:1 dfail:0 fail:0 skip:37 byt-nuc total:196 pass:161 dwarn:0 dfail:0 fail:0 skip:35 hsw-brixbox total:196 pass:174 dwarn:0 dfail:0 fail:0 skip:22 hsw-gt2 total:196 pass:179 dwarn:0 dfail:0 fail:0 skip:17 ilk-hp8440p total:196 pass:131 dwarn:1 dfail:0 fail:0 skip:64 skl-i7k-2total:196 pass:173 dwarn:0 dfail:0 fail:0 skip:23 snb-dellxps total:196 pass:162 dwarn:0 dfail:0 fail:0 skip:34 BOOT FAILED for bdw-nuci7 Results at /archive/results/CI_IGT_test/Patchwork_1835/ 851708c7e97537ed618fadbe5d342eaf8fa5146d drm-intel-ni
[Intel-gfx] [PATCH resend-for-CI 2/3] drm/i915: Remove forcewake request registers from the shadowed table
From: Tvrtko Ursulin
Chris Wilson points out that we can remove them from the array
since they are always written to with raw accessors.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_uncore.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 6b98b8a6d64a..41dd30f6ddbb 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -588,7 +588,6 @@ void assert_forcewakes_inactive(struct drm_i915_private
*dev_priv)
})
static const i915_reg_t gen8_shadowed_regs[] = {
- FORCEWAKE_MT,
GEN6_RPNSWREQ,
GEN6_RC_VIDEO_FREQ,
RING_TAIL(RENDER_RING_BASE),
@@ -724,9 +723,6 @@ static const i915_reg_t gen9_shadowed_regs[] = {
RING_TAIL(GEN6_BSD_RING_BASE),
RING_TAIL(VEBOX_RING_BASE),
RING_TAIL(BLT_RING_BASE),
- FORCEWAKE_BLITTER_GEN9,
- FORCEWAKE_RENDER_GEN9,
- FORCEWAKE_MEDIA_GEN9,
GEN6_RPNSWREQ,
GEN6_RC_VIDEO_FREQ,
/* TODO: Other registers are not yet used */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH resend-for-CI 1/3] drm/i915: Extract knowledge of register forcewake domains
From: Tvrtko Ursulin
Knowledge of which register per platform belonds in which
forcewake domain was embedded in the MMIO accessors themselves.
Extract it into standalone macros so they can be used from
new code in the following patches.
This causes GCC to compile some of the MMIO accessors slightly
differently and grows the code a tiny amount. But none of the
growth is on the fast-path so it does not matter hugely.
Affected sizes before:
26f0 01a5 t gen6_read16
2390 01a5 t gen6_read32
28a0 01a5 t gen6_read64
61d0 019e t gen8_write16
6510 019d t gen8_write32
6370 019d t gen8_write64
21f0 019d t gen8_write8
Affected sizes after:
2840 01aa t gen6_read16
24e0 01a9 t gen6_read32
29f0 01a9 t gen6_read64
4f20 01b5 t gen8_write16
4ba0 01b4 t gen8_write32
50e0 01b4 t gen8_write64
4d60 01b4 t gen8_write8
Other MMIO accessors are not affected in size.
Signed-off-by: Tvrtko Ursulin
Acked-by: Chris Wilson
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_uncore.c | 255 ++--
1 file changed, 155 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index dcf38bb5a097..6b98b8a6d64a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -552,6 +552,16 @@ void assert_forcewakes_inactive(struct drm_i915_private
*dev_priv)
/* We give fast paths for the really cool registers */
#define NEEDS_FORCE_WAKE(reg) ((reg) < 0x4)
+#define __gen6_reg_read_fw_domains(offset) \
+({ \
+ enum forcewake_domains __fwd; \
+ if (NEEDS_FORCE_WAKE(offset)) \
+ __fwd = FORCEWAKE_RENDER; \
+ else \
+ __fwd = 0; \
+ __fwd; \
+})
+
#define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
#define FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg) \
@@ -565,6 +575,49 @@ void assert_forcewakes_inactive(struct drm_i915_private
*dev_priv)
REG_RANGE((reg), 0x22000, 0x24000) || \
REG_RANGE((reg), 0x3, 0x4))
+#define __vlv_reg_read_fw_domains(offset) \
+({ \
+ enum forcewake_domains __fwd = 0; \
+ if (!NEEDS_FORCE_WAKE(offset)) \
+ __fwd = 0; \
+ else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_RENDER; \
+ else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_MEDIA; \
+ __fwd; \
+})
+
+static const i915_reg_t gen8_shadowed_regs[] = {
+ FORCEWAKE_MT,
+ GEN6_RPNSWREQ,
+ GEN6_RC_VIDEO_FREQ,
+ RING_TAIL(RENDER_RING_BASE),
+ RING_TAIL(GEN6_BSD_RING_BASE),
+ RING_TAIL(VEBOX_RING_BASE),
+ RING_TAIL(BLT_RING_BASE),
+ /* TODO: Other registers are not yet used */
+};
+
+static bool is_gen8_shadowed(u32 offset)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++)
+ if (offset == gen8_shadowed_regs[i].reg)
+ return true;
+
+ return false;
+}
+
+#define __gen8_reg_write_fw_domains(offset) \
+({ \
+ enum forcewake_domains __fwd; \
+ if (NEEDS_FORCE_WAKE(offset) && !is_gen8_shadowed(offset)) \
+ __fwd = FORCEWAKE_RENDER; \
+ else \
+ __fwd = 0; \
+ __fwd; \
+})
+
#define FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg) \
(REG_RANGE((reg), 0x2000, 0x4000) || \
REG_RANGE((reg), 0x5200, 0x8000) || \
@@ -587,6 +640,34 @@ void assert_forcewakes_inactive(struct drm_i915_private
*dev_priv)
REG_RANGE((reg), 0x9000, 0xB000) || \
REG_RANGE((reg), 0xF000, 0x1))
+#define __chv_reg_read_fw_domains(offset) \
+({ \
+ enum forcewake_domains __fwd = 0; \
+ if (!NEEDS_FORCE_WAKE(offset)) \
+ __fwd = 0; \
+ else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_RENDER; \
+ else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_MEDIA; \
+ else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
+ __fwd; \
+})
+
+#define __chv_reg_write_fw_domains(offset) \
+({ \
+ enum forcewake_domains __fwd = 0; \
+ if (!NEEDS_FORCE_WAKE(offset) || is_gen8_shadowed(offset)) \
+ __fwd = 0; \
+ else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_RENDER; \
+ else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_MEDIA; \
+ else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(offset)) \
+ __fwd = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
[Intel-gfx] [PATCH resend-for-CI 3/3] drm/i915: Only grab correct forcewake for the engine with execlists
From: Tvrtko Ursulin
Rather than blindly waking up all forcewake domains on command
submission, we can teach each engine what is (or are) the correct
one to take.
On platforms with multiple forcewake domains like VLV, CHV, SKL
and BXT, this has the potential of lowering the GPU and CPU
power use and submission latency.
To implement it we add a function named
intel_uncore_forcewake_for_reg whose purpose is to query which
forcewake domains need to be taken to read or write a specific
register with raw mmio accessors.
These enables the execlists engine setup to query which
forcewake domains are relevant per engine on the currently
running platform.
v2:
* Kerneldoc.
* Split from intel_uncore.c macro extraction, WARN_ON,
no warns on old platforms. (Chris Wilson)
v3:
* Single domain per engine, mention all registers,
bi-directional function and a new name, fix handling
of gen6 and gen7 writes. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++
drivers/gpu/drm/i915/intel_lrc.c| 27 ++--
drivers/gpu/drm/i915/intel_lrc.h| 1 +
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 108
5 files changed, 139 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1e15c21257ea..f5c91b01194f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -634,6 +634,13 @@ enum forcewake_domains {
FORCEWAKE_MEDIA)
};
+#define FW_REG_READ (1)
+#define FW_REG_WRITE (2)
+
+enum forcewake_domains
+intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
+ i915_reg_t reg, unsigned int op);
+
struct intel_uncore_funcs {
void (*force_wake_get)(struct drm_i915_private *dev_priv,
enum forcewake_domains
domains);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0d6dc5ec4a46..e6e69c2f2386 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -418,6 +418,7 @@ static void execlists_submit_requests(struct
drm_i915_gem_request *rq0,
struct drm_i915_gem_request *rq1)
{
struct drm_i915_private *dev_priv = rq0->i915;
+ unsigned int fw_domains = rq0->engine->fw_domains;
execlists_update_context(rq0);
@@ -425,11 +426,11 @@ static void execlists_submit_requests(struct
drm_i915_gem_request *rq0,
execlists_update_context(rq1);
spin_lock_irq(&dev_priv->uncore.lock);
- intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
+ intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
execlists_elsp_write(rq0, rq1);
- intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
spin_unlock_irq(&dev_priv->uncore.lock);
}
@@ -552,7 +553,7 @@ static void intel_lrc_irq_handler(unsigned long data)
unsigned int csb_read = 0, i;
unsigned int submit_contexts = 0;
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
status_pointer = I915_READ_FW(RING_CONTEXT_STATUS_PTR(engine));
@@ -577,7 +578,7 @@ static void intel_lrc_irq_handler(unsigned long data)
_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK,
engine->next_context_status_buffer << 8));
- intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put(dev_priv, engine->fw_domains);
spin_lock(&engine->execlist_lock);
@@ -2089,7 +2090,9 @@ logical_ring_default_irqs(struct intel_engine_cs *engine,
unsigned shift)
static int
logical_ring_init(struct drm_device *dev, struct intel_engine_cs *engine)
{
- struct intel_context *dctx = to_i915(dev)->kernel_context;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_context *dctx = dev_priv->kernel_context;
+ enum forcewake_domains fw_domains;
int ret;
/* Intentionally left blank. */
@@ -2111,6 +2114,20 @@ logical_ring_init(struct drm_device *dev, struct
intel_engine_cs *engine)
logical_ring_init_platform_invariants(engine);
+ fw_domains = intel_uncore_forcewake_for_reg(dev_priv,
+ RING_ELSP(engine),
+ FW_REG_WRITE);
+
+ fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+
RING_CONTEXT_STATUS_PTR(engine),
+FW_REG_READ |
FW_REG_WRITE);
+
+ fw_domains |= intel_uncore_forcewake_for_r
[Intel-gfx] [PATCH 1/2] drm/i915: Fix up ERR_PTR handling for pinning the ringbuffer
In commit 0a798eb92e6dcc1cba45d13d7b75a523e5d0fc4c
Author: Chris Wilson
Date: Fri Apr 8 12:11:11 2016 +0100
drm/i915: Refactor duplicate object vmap functions
the vmap function that returned NULL on error was replaced by one that
returned an error pointer instead. Not all callsites were updated...
Reported-by: Dave Gordon
Signed-off-by: Chris Wilson
Cc: Dave Gordon
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 41b604e69db7..15064a8f706b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2108,8 +2108,9 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device
*dev,
goto err_unpin;
ringbuf->virtual_start = i915_gem_object_pin_map(obj);
- if (ringbuf->virtual_start == NULL) {
- ret = -ENOMEM;
+ if (IS_ERR(ringbuf->virtual_start)) {
+ ret = PTR_ERR(ringbuf->virtual_start);
+ ringbuf->virtual_start = NULL;
goto err_unpin;
}
} else {
--
2.8.0.rc3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Mark obj->mapping as dirtying the backing storage
When reviewing some of Tvrtko's usage for i915_gem_object_pin_map(), he suggested replacing some use of kmap(i915_gem_object_get_dirty_page()) with a plain i915_gem_object_pin_map(). This raised the question of who should mark the page as dirty (or the mapping case, the object). We can write simpler, safer code if we mark the entire object as dirty upon obtaining the obj->mapping. (The counter-argument is that the caller should be marking the object as dirty itself, or we should be passing in a direction parameter.) Signed-off-by: Chris Wilson Cc: Dave Gordon Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b37ffea8b458..c0a4e38d6392 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2448,6 +2448,8 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj) } } + /* Presume the caller is going to write into the map */ + obj->dirty = true; return obj->mapping; } -- 2.8.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [resend-for-CI,1/3] drm/i915: Use consistent forcewake auto-release timeout across kernel configs
On Tue, Apr 12, 2016 at 02:33:45PM +0100, Tvrtko Ursulin wrote: > > On 08/04/16 08:02, Patchwork wrote: > > Test kms_flip: > > Subgroup basic-flip-vs-dpms: > > pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE > > Old friend unclaimed access prior to suspending: > https://bugs.freedesktop.org/show_bug.cgi?id=94164 o_O ILK doesn't even have a way to detect such things. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: check for ERR_PTR from i915_gem_object_pin_map()
The newly-introduced function i915_gem_object_pin_map() returns an
ERR_PTR (not NULL) if the pin-and-map opertaion fails, so that's what we
must check for. And it's nicer not to assign such a pointer-or-error to
a structure being filled in until after it's been validated, so we
should keep it local and avoid exporting a bogus pointer. Also, for
clarity and symmetry, we should clear 'virtual_start' along with 'vma'
when unmapping a ringbuffer.
Signed-off-by: Dave Gordon
Cc: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_drv.h | 6 --
drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +--
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a9c8211..bc8f0a3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3000,9 +3000,11 @@ static inline void i915_gem_object_unpin_pages(struct
drm_i915_gem_object *obj)
* pages and then returns a contiguous mapping of the backing storage into
* the kernel address space.
*
- * The caller must hold the struct_mutex.
+ * The caller must hold the struct_mutex, and is responsible for calling
+ * i915_gem_object_unpin_map() when the mapping is no longer required.
*
- * Returns the pointer through which to access the backing storage.
+ * Returns the pointer through which to access the mapped object, or an
+ * ERR_PTR() on error.
*/
void *__must_check i915_gem_object_pin_map(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 41b604e..35231ff 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2086,6 +2086,7 @@ void intel_unpin_ringbuffer_obj(struct intel_ringbuffer
*ringbuf)
i915_gem_object_unpin_map(ringbuf->obj);
else
iounmap(ringbuf->virtual_start);
+ ringbuf->virtual_start = NULL;
ringbuf->vma = NULL;
i915_gem_object_ggtt_unpin(ringbuf->obj);
}
@@ -2096,6 +2097,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device
*dev,
struct drm_i915_private *dev_priv = to_i915(dev);
struct i915_ggtt *ggtt = &dev_priv->ggtt;
struct drm_i915_gem_object *obj = ringbuf->obj;
+ void *addr;
int ret;
if (HAS_LLC(dev_priv) && !obj->stolen) {
@@ -2107,9 +2109,9 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device
*dev,
if (ret)
goto err_unpin;
- ringbuf->virtual_start = i915_gem_object_pin_map(obj);
- if (ringbuf->virtual_start == NULL) {
- ret = -ENOMEM;
+ addr = i915_gem_object_pin_map(obj);
+ if (IS_ERR(addr)) {
+ ret = PTR_ERR(addr);
goto err_unpin;
}
} else {
@@ -2124,14 +2126,15 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device
*dev,
/* Access through the GTT requires the device to be awake. */
assert_rpm_wakelock_held(dev_priv);
- ringbuf->virtual_start = ioremap_wc(ggtt->mappable_base +
-
i915_gem_obj_ggtt_offset(obj), ringbuf->size);
- if (ringbuf->virtual_start == NULL) {
+ addr = ioremap_wc(ggtt->mappable_base +
+ i915_gem_obj_ggtt_offset(obj), ringbuf->size);
+ if (addr == NULL) {
ret = -ENOMEM;
goto err_unpin;
}
}
+ ringbuf->virtual_start = addr;
ringbuf->vma = i915_gem_obj_to_ggtt(obj);
return 0;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [resend-for-CI,1/3] drm/i915: Use consistent forcewake auto-release timeout across kernel configs
On 12/04/16 14:43, Ville Syrjälä wrote: On Tue, Apr 12, 2016 at 02:33:45PM +0100, Tvrtko Ursulin wrote: On 08/04/16 08:02, Patchwork wrote: Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Old friend unclaimed access prior to suspending: https://bugs.freedesktop.org/show_bug.cgi?id=94164 o_O ILK doesn't even have a way to detect such things. :D No idea, must be copy and paste error, wrong tab, stack overflow on (mental) context switching.. Real one is sporadic fifo underrun: https://bugs.freedesktop.org/show_bug.cgi?id=93787 Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915: Adjust size of PIPE_CONTROL used for gen8 render seqno write
We started to use PIPE_CONTROL to write render ring seqno in order to
combat seqno write vs interrupt generation problems. This was introduced
by commit 7c17d377374d ("drm/i915: Use ordered seqno write interrupt
generation on gen8+ execlists").
On gen8+ size of PIPE_CONTROL with Post Sync Operation should be
6 dwords. When we're using older 5-dword variant it's possible to
observe inconsistent values written by PIPE_CONTROL with Post
Sync Operation from user batches, resulting in rendering corruptions.
v2: Fix BAT failures
v3: Comments on alignment and thrashing high dword of seqno (Chris)
v4: Updated commit msg (Mika)
Testcase: igt/gem_pipe_control_store_loop/*-qword-write
Issue: VIZ-7393
Cc: sta...@vger.kernel.org
Cc: Chris Wilson
Cc: Mika Kuoppala
Cc: Abdiel Janulgue
Signed-off-by: Michał Winiarski
---
drivers/gpu/drm/i915/intel_lrc.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0d6dc5e..30abe53 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1945,15 +1945,18 @@ static int gen8_emit_request_render(struct
drm_i915_gem_request *request)
struct intel_ringbuffer *ringbuf = request->ringbuf;
int ret;
- ret = intel_logical_ring_begin(request, 6 + WA_TAIL_DWORDS);
+ ret = intel_logical_ring_begin(request, 8 + WA_TAIL_DWORDS);
if (ret)
return ret;
+ /* We're using qword write, seqno should be aligned to 8 bytes. */
+ BUILD_BUG_ON(I915_GEM_HWS_INDEX & 1);
+
/* w/a for post sync ops following a GPGPU operation we
* need a prior CS_STALL, which is emitted by the flush
* following the batch.
*/
- intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(5));
+ intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(6));
intel_logical_ring_emit(ringbuf,
(PIPE_CONTROL_GLOBAL_GTT_IVB |
PIPE_CONTROL_CS_STALL |
@@ -1961,7 +1964,10 @@ static int gen8_emit_request_render(struct
drm_i915_gem_request *request)
intel_logical_ring_emit(ringbuf, hws_seqno_address(request->engine));
intel_logical_ring_emit(ringbuf, 0);
intel_logical_ring_emit(ringbuf, i915_gem_request_get_seqno(request));
+ /* We're thrashing one dword of HWS. */
+ intel_logical_ring_emit(ringbuf, 0);
intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
return intel_logical_ring_advance_and_submit(request);
}
--
2.8.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: check for ERR_PTR from i915_gem_object_pin_map()
On Tue, Apr 12, 2016 at 02:46:16PM +0100, Dave Gordon wrote: > The newly-introduced function i915_gem_object_pin_map() returns an > ERR_PTR (not NULL) if the pin-and-map opertaion fails, so that's what we > must check for. And it's nicer not to assign such a pointer-or-error to > a structure being filled in until after it's been validated, so we > should keep it local and avoid exporting a bogus pointer. Also, for > clarity and symmetry, we should clear 'virtual_start' along with 'vma' > when unmapping a ringbuffer. > > Signed-off-by: Dave Gordon > Cc: Chris Wilson > Cc: Tvrtko Ursulin Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: Adjust size of PIPE_CONTROL used for gen8 render seqno write
Michał Winiarski writes:
> [ text/plain ]
> We started to use PIPE_CONTROL to write render ring seqno in order to
> combat seqno write vs interrupt generation problems. This was introduced
> by commit 7c17d377374d ("drm/i915: Use ordered seqno write interrupt
> generation on gen8+ execlists").
>
> On gen8+ size of PIPE_CONTROL with Post Sync Operation should be
> 6 dwords. When we're using older 5-dword variant it's possible to
> observe inconsistent values written by PIPE_CONTROL with Post
> Sync Operation from user batches, resulting in rendering corruptions.
>
> v2: Fix BAT failures
> v3: Comments on alignment and thrashing high dword of seqno (Chris)
> v4: Updated commit msg (Mika)
>
> Testcase: igt/gem_pipe_control_store_loop/*-qword-write
> Issue: VIZ-7393
> Cc: sta...@vger.kernel.org
> Cc: Chris Wilson
> Cc: Mika Kuoppala
> Cc: Abdiel Janulgue
> Signed-off-by: Michał Winiarski
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 0d6dc5e..30abe53 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1945,15 +1945,18 @@ static int gen8_emit_request_render(struct
> drm_i915_gem_request *request)
> struct intel_ringbuffer *ringbuf = request->ringbuf;
> int ret;
>
> - ret = intel_logical_ring_begin(request, 6 + WA_TAIL_DWORDS);
> + ret = intel_logical_ring_begin(request, 8 + WA_TAIL_DWORDS);
> if (ret)
> return ret;
>
> + /* We're using qword write, seqno should be aligned to 8 bytes. */
> + BUILD_BUG_ON(I915_GEM_HWS_INDEX & 1);
> +
> /* w/a for post sync ops following a GPGPU operation we
>* need a prior CS_STALL, which is emitted by the flush
>* following the batch.
>*/
> - intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(5));
> + intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(6));
> intel_logical_ring_emit(ringbuf,
> (PIPE_CONTROL_GLOBAL_GTT_IVB |
>PIPE_CONTROL_CS_STALL |
> @@ -1961,7 +1964,10 @@ static int gen8_emit_request_render(struct
> drm_i915_gem_request *request)
> intel_logical_ring_emit(ringbuf, hws_seqno_address(request->engine));
> intel_logical_ring_emit(ringbuf, 0);
> intel_logical_ring_emit(ringbuf, i915_gem_request_get_seqno(request));
> + /* We're thrashing one dword of HWS. */
> + intel_logical_ring_emit(ringbuf, 0);
> intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> return intel_logical_ring_advance_and_submit(request);
> }
>
> --
> 2.8.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/10] drm/virtio: Drop dummy gamma table support
On 30 March 2016 at 10:51, Daniel Vetter wrote:
> No need to confuse userspace like this.
>
> Cc: Gerd Hoffmann
> Cc: Dave Airlie
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/virtio/virtgpu_display.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index 4854dac87e24..12b72e29678a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -38,13 +38,6 @@
> #define XRES_MAX 8192
> #define YRES_MAX 8192
>
> -static void virtio_gpu_crtc_gamma_set(struct drm_crtc *crtc,
> - u16 *red, u16 *green, u16 *blue,
> - uint32_t start, uint32_t size)
> -{
> - /* TODO */
> -}
> -
> static void
> virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev,
>struct virtio_gpu_output *output)
> @@ -173,7 +166,6 @@ static int virtio_gpu_page_flip(struct drm_crtc *crtc,
> static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> .cursor_set2= virtio_gpu_crtc_cursor_set,
> .cursor_move= virtio_gpu_crtc_cursor_move,
> - .gamma_set = virtio_gpu_crtc_gamma_set,
> .set_config = drm_atomic_helper_set_config,
> .destroy= drm_crtc_cleanup,
>
> @@ -416,7 +408,6 @@ static int vgdev_output_init(struct virtio_gpu_device
> *vgdev, int index)
> return PTR_ERR(plane);
> drm_crtc_init_with_planes(dev, crtc, plane, NULL,
> &virtio_gpu_crtc_funcs, NULL);
> - drm_mode_crtc_set_gamma_size(crtc, 256);
> drm_crtc_helper_add(crtc, &virtio_gpu_crtc_helper_funcs);
> plane->crtc = crtc;
>
Out of curiosity:
Coccinelle should be able to handle/generate such patches, shouldn't
it ? I believe in the past people used it for similar
refactoring/cleanups, yet not (m)any of them [the cocci files] got
checked in the kernel tree.
Thinking about future drivers derived from outdated sources - do you
think it's a good/bad idea to check/run them along side the existing
ones ?
-Emil
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, Apr 12, 2016 at 03:26:44PM +0200, Boris Brezillon wrote: > On Tue, 12 Apr 2016 15:11:18 +0200 > Thierry Reding wrote: > > > On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > > > On Tue, 12 Apr 2016 14:21:41 +0200 > > > Thierry Reding wrote: > > > > > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > > > Thierry Reding wrote: > > > > > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > > > The PWM state, represented by its period, duty_cycle and polarity, > > > > > > > is currently directly stored in the PWM device. > > > > > > > Declare a pwm_state structure embedding those field so that we > > > > > > > can later > > > > > > > use this struct to atomically update all the PWM parameters at > > > > > > > once. > > > > > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > > > pwm_get_state(). > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > > > > > > --- > > > > > > > drivers/pwm/core.c | 8 > > > > > > > include/linux/pwm.h | 54 > > > > > > > + > > > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > > > index 6433059..f3f91e7 100644 > > > > > > > --- a/drivers/pwm/core.c > > > > > > > +++ b/drivers/pwm/core.c > > > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip > > > > > > > *chip, > > > > > > > pwm->chip = chip; > > > > > > > pwm->pwm = chip->base + i; > > > > > > > pwm->hwpwm = i; > > > > > > > - pwm->polarity = polarity; > > > > > > > + pwm->state.polarity = polarity; > > > > > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? > > > > > > After > > > > > > all this is setting up the "initial" state, much like DT or the > > > > > > lookup > > > > > > tables would for duty cycle and period. > > > > > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > > > all the reference info should be extracted from DT, PWM lookup table > > > > > or > > > > > driver specific ->request() implementation, but I can definitely > > > > > initialize the args.polarity here too. > > > > > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > > > polarity when the driver does not support hardware readout)? > > > > > > > > Wouldn't this work automatically as part of the pwm_apply_args() helper > > > > if we extended it with this setting? > > > > > > Well, as you explained in you answer to patch 5, pwm_apply_args() > > > should be called on a per-request basis (each time a PWM device is > > > requested), while the initial polarity setting should only be applied > > > when registering the PWM chip (and its devices). After that, the > > > framework takes care of keeping the PWM state in sync with the hardware > > > state. > > > > > > Let's take a real (though a bit unusual) example. Say you have a single > > > PWM device referenced by two different users. Only one user can be > > > enabled at a time, but each of them has its own reference config > > > (different polarity, different period). > > > > > > User1 calls pwm_get() and applies its own polarity and period. Then > > > user1 is unregistered and release the PWM device, leaving the polarity > > > and period untouched. > > > > > > User2 is registered and request the same PWM device, but user2 is > > > smarter and tries to extract the current PWM state before adapting the > > > config according to pwm_args. If you just reset pwm->state.polarity > > > each time pwm_apply_args() is called (and you suggested to call it as > > > part of the request procedure), then this means the PWM state is no > > > longer in sync with the hardware state. > > > > In that case neither will be the period or duty cycle. Essentially this > > gets us back to square one where we need to decide how to handle current > > state vs. initial arguments. > > That's not true. Now we clearly differentiate the reference config > (content of pwm_args which is only a subset of what you'll find in > pwm_state) and the PWM state (represented by pwm_state). > > We should be safe as long as we keep those 2 elements as 2 orthogonal > concepts: > - pwm_args is supposed to give some hint to the PWM user to help him > configure it's PWM appropriately > - pwm_state is here to reflect the real PWM state, and apply new > configs > > > > > But I don't think this is really going to be an issue because this is > > all moot until we've moved over to the atomic API, at which point this > > is all going to go away anyway. > > As stated in my answer to patch 5, I think I misunderstood your > suggestion. pwm_apply_args() is supposed to adjust the PWM config to > match the period
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Use new i915_gem_object_pin_map for LRC (rev2)
== Series Details == Series: drm/i915: Use new i915_gem_object_pin_map for LRC (rev2) URL : https://patchwork.freedesktop.org/series/5580/ State : failure == Summary == Series 5580v2 drm/i915: Use new i915_gem_object_pin_map for LRC http://patchwork.freedesktop.org/api/1.0/series/5580/revisions/2/mbox/ Test drv_hangman: Subgroup error-state-basic: pass -> SKIP (bsw-nuc-2) fail -> PASS (ilk-hp8440p) Test drv_module_reload_basic: pass -> DMESG-WARN (bsw-nuc-2) pass -> DMESG-WARN (skl-nuci5) pass -> DMESG-WARN (bdw-nuci7) pass -> DMESG-WARN (skl-i7k-2) pass -> DMESG-WARN (bdw-ultra) Test gem_ctx_param_basic: Subgroup invalid-size-set: pass -> DMESG-WARN (bsw-nuc-2) Test gem_exec_basic: Subgroup gtt-vebox: pass -> SKIP (bsw-nuc-2) Subgroup readonly-default: pass -> SKIP (bsw-nuc-2) Test gem_mmap_gtt: Subgroup basic-write-no-prefault: pass -> DMESG-WARN (bsw-nuc-2) Test gem_ringfill: Subgroup basic-default-forked: pass -> SKIP (bsw-nuc-2) Subgroup basic-default-interruptible: pass -> SKIP (bsw-nuc-2) Test kms_addfb_basic: Subgroup basic-x-tiled: pass -> DMESG-WARN (bsw-nuc-2) Subgroup bo-too-small: pass -> DMESG-WARN (bsw-nuc-2) Subgroup bo-too-small-due-to-tiling: pass -> DMESG-WARN (bsw-nuc-2) Subgroup small-bo: pass -> DMESG-WARN (bsw-nuc-2) Subgroup unused-pitches: pass -> DMESG-WARN (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-dpms: dmesg-warn -> PASS (ilk-hp8440p) UNSTABLE Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Subgroup basic-flip-vs-wf_vblank: pass -> DMESG-FAIL (bsw-nuc-2) Subgroup basic-plain-flip: pass -> DMESG-FAIL (bsw-nuc-2) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> DMESG-WARN (bsw-nuc-2) Subgroup basic-rte: dmesg-warn -> PASS (byt-nuc) UNSTABLE Test prime_self_import: Subgroup basic-with_one_bo_two_files: pass -> DMESG-WARN (bsw-nuc-2) bdw-nuci7total:203 pass:190 dwarn:1 dfail:0 fail:0 skip:12 bdw-ultratotal:203 pass:179 dwarn:1 dfail:0 fail:0 skip:23 bsw-nuc-2total:202 pass:146 dwarn:10 dfail:2 fail:0 skip:44 byt-nuc total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:203 pass:179 dwarn:0 dfail:0 fail:0 skip:24 hsw-gt2 total:203 pass:184 dwarn:0 dfail:0 fail:0 skip:19 ilk-hp8440p total:203 pass:134 dwarn:1 dfail:0 fail:0 skip:68 ivb-t430stotal:203 pass:175 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2total:203 pass:177 dwarn:1 dfail:0 fail:0 skip:25 skl-nuci5total:203 pass:191 dwarn:1 dfail:0 fail:0 skip:11 snb-x220ttotal:203 pass:165 dwarn:0 dfail:0 fail:1 skip:37 BOOT FAILED for snb-dellxps Results at /archive/results/CI_IGT_test/Patchwork_1870/ 8f2e41ba8d25b39e6a057d3244481771f6054764 drm-intel-nightly: 2016y-04m-12d-12h-14m-24s UTC integration manifest d321b00 drm/i915: Use new i915_gem_object_pin_map for LRC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 03/46] backlight: lm3630a_bl: stop messing with the pwm->period field
On Tue, 12 Apr 2016, Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:26PM +0200, Boris Brezillon wrote: > > pwm->period field is not supposed to be changed by PWM users. The only > > ones authorized to change it are the PWM core and PWM drivers. > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/video/backlight/lm3630a_bl.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > Applied, thanks. Applied? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 15/46] pwm: introduce the pwm_state concept
On Tue, 12 Apr 2016 16:05:46 +0200 Thierry Reding wrote: > On Tue, Apr 12, 2016 at 03:26:44PM +0200, Boris Brezillon wrote: > > On Tue, 12 Apr 2016 15:11:18 +0200 > > Thierry Reding wrote: > > > > > On Tue, Apr 12, 2016 at 02:45:08PM +0200, Boris Brezillon wrote: > > > > On Tue, 12 Apr 2016 14:21:41 +0200 > > > > Thierry Reding wrote: > > > > > > > > > On Tue, Apr 12, 2016 at 02:17:18PM +0200, Boris Brezillon wrote: > > > > > > On Tue, 12 Apr 2016 13:49:04 +0200 > > > > > > Thierry Reding wrote: > > > > > > > > > > > > > On Wed, Mar 30, 2016 at 10:03:38PM +0200, Boris Brezillon wrote: > > > > > > > > The PWM state, represented by its period, duty_cycle and > > > > > > > > polarity, > > > > > > > > is currently directly stored in the PWM device. > > > > > > > > Declare a pwm_state structure embedding those field so that we > > > > > > > > can later > > > > > > > > use this struct to atomically update all the PWM parameters at > > > > > > > > once. > > > > > > > > > > > > > > > > All pwm_get_xxx() helpers are now implemented as wrappers around > > > > > > > > pwm_get_state(). > > > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > > > > > > > > --- > > > > > > > > drivers/pwm/core.c | 8 > > > > > > > > include/linux/pwm.h | 54 > > > > > > > > + > > > > > > > > 2 files changed, 46 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > > > > > > > index 6433059..f3f91e7 100644 > > > > > > > > --- a/drivers/pwm/core.c > > > > > > > > +++ b/drivers/pwm/core.c > > > > > > > > @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct > > > > > > > > pwm_chip *chip, > > > > > > > > pwm->chip = chip; > > > > > > > > pwm->pwm = chip->base + i; > > > > > > > > pwm->hwpwm = i; > > > > > > > > - pwm->polarity = polarity; > > > > > > > > + pwm->state.polarity = polarity; > > > > > > > > > > > > > > Would this not more correctly be assigned to pwm->args.polarity? > > > > > > > After > > > > > > > all this is setting up the "initial" state, much like DT or the > > > > > > > lookup > > > > > > > tables would for duty cycle and period. > > > > > > > > > > > > Yes, I wasn't sure about the pwm_add_with_polarity() meaning. To me, > > > > > > all the reference info should be extracted from DT, PWM lookup > > > > > > table or > > > > > > driver specific ->request() implementation, but I can definitely > > > > > > initialize the args.polarity here too. > > > > > > > > > > > > Should I keep the pwm->state.polarity assignment (to set the initial > > > > > > polarity when the driver does not support hardware readout)? > > > > > > > > > > Wouldn't this work automatically as part of the pwm_apply_args() > > > > > helper > > > > > if we extended it with this setting? > > > > > > > > Well, as you explained in you answer to patch 5, pwm_apply_args() > > > > should be called on a per-request basis (each time a PWM device is > > > > requested), while the initial polarity setting should only be applied > > > > when registering the PWM chip (and its devices). After that, the > > > > framework takes care of keeping the PWM state in sync with the hardware > > > > state. > > > > > > > > Let's take a real (though a bit unusual) example. Say you have a single > > > > PWM device referenced by two different users. Only one user can be > > > > enabled at a time, but each of them has its own reference config > > > > (different polarity, different period). > > > > > > > > User1 calls pwm_get() and applies its own polarity and period. Then > > > > user1 is unregistered and release the PWM device, leaving the polarity > > > > and period untouched. > > > > > > > > User2 is registered and request the same PWM device, but user2 is > > > > smarter and tries to extract the current PWM state before adapting the > > > > config according to pwm_args. If you just reset pwm->state.polarity > > > > each time pwm_apply_args() is called (and you suggested to call it as > > > > part of the request procedure), then this means the PWM state is no > > > > longer in sync with the hardware state. > > > > > > In that case neither will be the period or duty cycle. Essentially this > > > gets us back to square one where we need to decide how to handle current > > > state vs. initial arguments. > > > > That's not true. Now we clearly differentiate the reference config > > (content of pwm_args which is only a subset of what you'll find in > > pwm_state) and the PWM state (represented by pwm_state). > > > > We should be safe as long as we keep those 2 elements as 2 orthogonal > > concepts: > > - pwm_args is supposed to give some hint to the PWM user to help him > > configure it's PWM appropriately > > - pwm_state is here to reflect the real PWM state, and apply new > > configs > > > > > > > > But I don't think thi
Re: [Intel-gfx] [PATCH v5 02/46] backlight: pwm_bl: remove useless call to pwm_set_period()
On Tue, 12 Apr 2016, Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:25PM +0200, Boris Brezillon wrote: > > The PWM period will be set when calling pwm_config. Remove this useless > > call to pwm_set_period(), which might mess up with the internal PWM state. > > > > Signed-off-by: Boris Brezillon > > Acked-by: Lee Jones > > --- > > drivers/video/backlight/pwm_bl.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > Applied, thanks. Applied? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 02/46] backlight: pwm_bl: remove useless call to pwm_set_period()
On Tue, Apr 12, 2016 at 03:16:53PM +0100, Lee Jones wrote: > On Tue, 12 Apr 2016, Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:25PM +0200, Boris Brezillon wrote: > > > The PWM period will be set when calling pwm_config. Remove this useless > > > call to pwm_set_period(), which might mess up with the internal PWM state. > > > > > > Signed-off-by: Boris Brezillon > > > Acked-by: Lee Jones > > > --- > > > drivers/video/backlight/pwm_bl.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > Applied, thanks. > > Applied? You Acked it, so I assumed you were fine with me taking it through the PWM tree to take care of the dependencies. Did I assume wrongly? Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 03/46] backlight: lm3630a_bl: stop messing with the pwm->period field
On Tue, Apr 12, 2016 at 03:16:13PM +0100, Lee Jones wrote: > On Tue, 12 Apr 2016, Thierry Reding wrote: > > > On Wed, Mar 30, 2016 at 10:03:26PM +0200, Boris Brezillon wrote: > > > pwm->period field is not supposed to be changed by PWM users. The only > > > ones authorized to change it are the PWM core and PWM drivers. > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > drivers/video/backlight/lm3630a_bl.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > Applied, thanks. > > Applied? You didn't specifically Ack this one, but I presumed that since the change is essentially the same as for pwm-backlight, and this is another prerequisite for the remainder of the series it should go in through the PWM tree as well. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gen9: implement WaEnableSamplerGPGPUPreemptionSupport
From: Tim Gore
WaEnableSamplerGPGPUPreemptionSupport fixes a problem
related to mid thread pre-emption.
Signed-off-by: Tim Gore
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cea5a39..ff83c64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7161,6 +7161,7 @@ enum skl_disp_power_wells {
#define GEN9_HALF_SLICE_CHICKEN7 _MMIO(0xe194)
#define GEN9_ENABLE_YV12_BUGFIX (1<<4)
+#define GEN9_ENABLE_GPGPU_PREEMPTION (1<<2)
/* Audio */
#define G4X_AUD_VID_DID
_MMIO(dev_priv->info.display_mmio_offset + 0x62020)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 41b604e..c2603f7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -959,9 +959,10 @@ static int gen9_init_workarounds(struct intel_engine_cs
*engine)
}
/* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt */
- if (IS_SKL_REVID(dev, SKL_REVID_C0, REVID_FOREVER) || IS_BROXTON(dev))
- WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
- GEN9_ENABLE_YV12_BUGFIX);
+ /* WaEnableSamplerGPGPUPreemptionSupport:skl,bxt */
+ WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
+ GEN9_ENABLE_YV12_BUGFIX |
+ GEN9_ENABLE_GPGPU_PREEMPTION);
/* Wa4x4STCOptimizationDisable:skl,bxt */
/* WaDisablePartialResolveInVc:skl,bxt */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [resend-for-CI,1/3] drm/i915: Extract knowledge of register forcewake domains
== Series Details == Series: series starting with [resend-for-CI,1/3] drm/i915: Extract knowledge of register forcewake domains URL : https://patchwork.freedesktop.org/series/5593/ State : failure == Summary == Series 5593v1 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/5593/revisions/1/mbox/ Test drv_hangman: Subgroup error-state-basic: fail -> PASS (ilk-hp8440p) Test kms_flip: Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Test pm_rpm: Subgroup basic-rte: dmesg-warn -> PASS (bsw-nuc-2) bdw-nuci7total:203 pass:191 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:203 pass:180 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:202 pass:163 dwarn:0 dfail:0 fail:0 skip:39 byt-nuc total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 hsw-gt2 total:203 pass:184 dwarn:0 dfail:0 fail:0 skip:19 ilk-hp8440p total:203 pass:134 dwarn:1 dfail:0 fail:0 skip:68 skl-i7k-2total:203 pass:178 dwarn:0 dfail:0 fail:0 skip:25 skl-nuci5total:203 pass:192 dwarn:0 dfail:0 fail:0 skip:11 snb-x220ttotal:203 pass:165 dwarn:0 dfail:0 fail:1 skip:37 BOOT FAILED for snb-dellxps Results at /archive/results/CI_IGT_test/Patchwork_1871/ d89f227a17b175fce74e11b2d5fa2a41f86fc489 drm-intel-nightly: 2016y-04m-12d-13h-31m-26s UTC integration manifest 9bd8d7e drm/i915: Only grab correct forcewake for the engine with execlists 9c6a1ec drm/i915: Remove forcewake request registers from the shadowed table c6211b2 drm/i915: Extract knowledge of register forcewake domains ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/core: Add drm_accurate_vblank_count, v3.
This function is useful for gen2 intel devices which have no frame
counter, but need a way to determine the current vblank count without
racing with the vblank interrupt handler.
intel_pipe_update_start checks if no vblank interrupt will occur
during vblank evasion, but cannot check whether the vblank handler has
run to completion. This function uses the timestamps to determine
when the last vblank has happened, and interpolates from there.
Changes since v1:
- Take vblank_time_lock and don't use drm_vblank_count_and_time.
Changes since v2:
- Don't return time of last vblank.
Cc: Mario Kleiner
Cc: Ville Syrjälä
Signed-off-by: Maarten Lankhorst
---
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3c1a6f18e71c..32bfa4bb8f41 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -303,6 +303,32 @@ static void drm_update_vblank_count(struct drm_device
*dev, unsigned int pipe,
store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
}
+/**
+ * drm_accurate_vblank_count - retrieve the master vblank counter
+ * @crtc: which counter to retrieve
+ * @tv_ret: last time counter was updated
+ *
+ * This function is similar to @drm_crtc_vblank_count but this
+ * function interpolates to handle a race with vblank irq's.
+ */
+
+u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ u32 vblank, pipe = drm_crtc_index(crtc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->vblank_time_lock, flags);
+
+ drm_update_vblank_count(dev, pipe, 0);
+ vblank = dev->vblank[pipe].count;
+
+ spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
+
+ return vblank;
+}
+EXPORT_SYMBOL(drm_accurate_vblank_count);
+
/*
* Disable vblank irq's on crtc, make sure that last vblank count
* of hardware and corresponding consistent software vblank counter
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 31483c2fef51..747c80da70e5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
extern void drm_vblank_cleanup(struct drm_device *dev);
+extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv3 2/4] drm/i915: Store the dpll config in crtc_state->shared_dpll
>-Original Message- >From: Conselvan De Oliveira, Ander >Sent: Monday, April 11, 2016 6:07 PM >To: intel-gfx@lists.freedesktop.org; R, Durgadoss >Cc: ville.syrj...@linux.intel.com >Subject: Re: [PATCHv3 2/4] drm/i915: Store the dpll config in >crtc_state->shared_dpll > >On Wed, 2016-04-06 at 17:23 +0530, Durgadoss R wrote: >> Currently, the required shared dpll is saved in the crtc_state. >> Similarly, this patch saves the dpll config values also, so that >> these values (through crtc_state->shared_dpll->config.hw_state) >> can be used for upfront link training. >> >> Signed-off-by: Durgadoss R >> --- >> drivers/gpu/drm/i915/intel_dpll_mgr.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c >> b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> index 1175eeb..cad10f2 100644 >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> @@ -248,6 +248,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll >> *pll, >> pipe_name(crtc->pipe)); >> >> intel_shared_dpll_config_get(shared_dpll, pll, crtc); >> +crtc_state->shared_dpll->config = shared_dpll[i]; > >This overwrites the state stored in dev_priv->shared_dpll[i].config, so it >means >we loose the current state set in the hardware. If the atomic check fails after >this, the software tracking of the hw state gets messed up. But we need the computed dpll_state and crtc_mask set for pll enabling. So, the only other way I see is to call shared_dpll_commit() in ddi_upfront() code after we do get_shared_dpll(). What do you think of this ? Or any other options ? Thanks, Durga > >Ander > >> } >> >> void intel_shared_dpll_commit(struct drm_atomic_state *state) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [resend-for-CI,1/3] drm/i915: Extract knowledge of register forcewake domains
On 12/04/16 15:29, Patchwork wrote: == Series Details == Series: series starting with [resend-for-CI,1/3] drm/i915: Extract knowledge of register forcewake domains URL : https://patchwork.freedesktop.org/series/5593/ State : failure == Summary == Series 5593v1 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/5593/revisions/1/mbox/ Test drv_hangman: Subgroup error-state-basic: fail -> PASS (ilk-hp8440p) Test kms_flip: Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE Sporadic ILK fifo underruns: https://bugs.freedesktop.org/show_bug.cgi?id=93787 Test pm_rpm: Subgroup basic-rte: dmesg-warn -> PASS (bsw-nuc-2) bdw-nuci7total:203 pass:191 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultratotal:203 pass:180 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2total:202 pass:163 dwarn:0 dfail:0 fail:0 skip:39 byt-nuc total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 hsw-gt2 total:203 pass:184 dwarn:0 dfail:0 fail:0 skip:19 ilk-hp8440p total:203 pass:134 dwarn:1 dfail:0 fail:0 skip:68 skl-i7k-2total:203 pass:178 dwarn:0 dfail:0 fail:0 skip:25 skl-nuci5total:203 pass:192 dwarn:0 dfail:0 fail:0 skip:11 snb-x220ttotal:203 pass:165 dwarn:0 dfail:0 fail:1 skip:37 BOOT FAILED for snb-dellxps Results at /archive/results/CI_IGT_test/Patchwork_1871/ d89f227a17b175fce74e11b2d5fa2a41f86fc489 drm-intel-nightly: 2016y-04m-12d-13h-31m-26s UTC integration manifest 9bd8d7e drm/i915: Only grab correct forcewake for the engine with execlists 9c6a1ec drm/i915: Remove forcewake request registers from the shadowed table c6211b2 drm/i915: Extract knowledge of register forcewake domains Squeaky clean - merged. Thanks for the review. Hope this makes a difference on some platform. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Split execlists hardware status page initialisation from setup
From: Tvrtko Ursulin
Split the hardware status page into setup and initialisation,
where setup means setting up the driver state to support the
engine, and initialization means programming the hardware
with the before set up state.
This way the design matches the design of the engine setup/init
code which is split in the same fashion and it enables the
stages to be used in a balanced fashion (engine setup - hws
setup, engine init - hws init).
This will enable the upcoming improvements to slot in without
any kludges on the GPU reset path.
Signed-off-by: Tvrtko Ursulin
Suggested-by: Chris Wilson
Cc: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c | 50 ++--
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e6e69c2f2386..3fd2ae6ce8e7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -229,9 +229,6 @@ enum {
static int intel_lr_context_pin(struct intel_context *ctx,
struct intel_engine_cs *engine);
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
- struct drm_i915_gem_object
*default_ctx_obj);
-
/**
* intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
@@ -1580,14 +1577,22 @@ out:
return ret;
}
+static void lrc_init_hws(struct intel_engine_cs *engine)
+{
+ struct drm_i915_private *dev_priv = engine->dev->dev_private;
+
+ I915_WRITE(RING_HWS_PGA(engine->mmio_base),
+ (u32)engine->status_page.gfx_addr);
+ POSTING_READ(RING_HWS_PGA(engine->mmio_base));
+}
+
static int gen8_init_common_ring(struct intel_engine_cs *engine)
{
struct drm_device *dev = engine->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned int next_context_status_buffer_hw;
- lrc_setup_hardware_status_page(engine,
-
dev_priv->kernel_context->engine[engine->id].state);
+ lrc_init_hws(engine);
I915_WRITE_IMR(engine,
~(engine->irq_enable_mask | engine->irq_keep_mask));
@@ -2087,6 +2092,20 @@ logical_ring_default_irqs(struct intel_engine_cs
*engine, unsigned shift)
engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
}
+static void
+lrc_setup_hws(struct intel_engine_cs *engine,
+ struct drm_i915_gem_object *dctx_obj)
+{
+ struct page *page;
+
+ /* The HWSP is part of the default context object in LRC mode. */
+ engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(dctx_obj) +
+ LRC_PPHWSP_PN * PAGE_SIZE;
+ page = i915_gem_object_get_page(dctx_obj, LRC_PPHWSP_PN);
+ engine->status_page.page_addr = kmap(page);
+ engine->status_page.obj = dctx_obj;
+}
+
static int
logical_ring_init(struct drm_device *dev, struct intel_engine_cs *engine)
{
@@ -2145,6 +2164,9 @@ logical_ring_init(struct drm_device *dev, struct
intel_engine_cs *engine)
goto error;
}
+ /* And setup the hardware status page. */
+ lrc_setup_hws(engine, dctx->engine[engine->id].state);
+
return 0;
error:
@@ -2605,24 +2627,6 @@ uint32_t intel_lr_context_size(struct intel_engine_cs
*engine)
return ret;
}
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
- struct drm_i915_gem_object
*default_ctx_obj)
-{
- struct drm_i915_private *dev_priv = engine->dev->dev_private;
- struct page *page;
-
- /* The HWSP is part of the default context object in LRC mode. */
- engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
- + LRC_PPHWSP_PN * PAGE_SIZE;
- page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
- engine->status_page.page_addr = kmap(page);
- engine->status_page.obj = default_ctx_obj;
-
- I915_WRITE(RING_HWS_PGA(engine->mmio_base),
- (u32)engine->status_page.gfx_addr);
- POSTING_READ(RING_HWS_PGA(engine->mmio_base));
-}
-
/**
* intel_lr_context_deferred_alloc() - create the LRC specific bits of a
context
* @ctx: LR context to create.
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Use new i915_gem_object_pin_map for LRC
From: Tvrtko Ursulin
We can use the new pin/lazy unpin API for simplicity
and more performance in the execlist submission paths.
v2:
* Fix error handling and convert more users.
* Compact some names for readability.
v3:
* intel_lr_context_free was not unpinning.
* Special case for GPU reset which otherwise unbalances
the HWS object pages pin count by running the engine
initialization only (not destructors).
v4:
* Rebased on top of hws setup/init split.
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c| 82 ++---
drivers/gpu/drm/i915/intel_lrc.h| 7 ++-
3 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index fe580cb9501a..91028d9c6269 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -342,7 +342,7 @@ void i915_gem_context_reset(struct drm_device *dev)
struct intel_context *ctx;
list_for_each_entry(ctx, &dev_priv->context_list, link)
- intel_lr_context_reset(dev, ctx);
+ intel_lr_context_reset(dev_priv, ctx);
}
for (i = 0; i < I915_NUM_ENGINES; i++) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3fd2ae6ce8e7..b61f8da5d6f3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1091,8 +1091,8 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
struct intel_ringbuffer *ringbuf = ctx->engine[engine->id].ringbuf;
- struct page *lrc_state_page;
- uint32_t *lrc_reg_state;
+ void *obj_addr;
+ u32 *lrc_reg_state;
int ret;
WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
@@ -1102,19 +1102,20 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
if (ret)
return ret;
- lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
- if (WARN_ON(!lrc_state_page)) {
- ret = -ENODEV;
+ obj_addr = i915_gem_object_pin_map(ctx_obj);
+ if (IS_ERR(obj_addr)) {
+ ret = PTR_ERR(obj_addr);
goto unpin_ctx_obj;
}
+ lrc_reg_state = obj_addr + LRC_STATE_PN * PAGE_SIZE;
+
ret = intel_pin_and_map_ringbuffer_obj(engine->dev, ringbuf);
if (ret)
- goto unpin_ctx_obj;
+ goto unpin_map;
ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, engine);
- lrc_reg_state = kmap(lrc_state_page);
lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
ctx_obj->dirty = true;
@@ -1125,6 +1126,8 @@ static int intel_lr_context_do_pin(struct intel_context
*ctx,
return ret;
+unpin_map:
+ i915_gem_object_unpin_map(ctx_obj);
unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
@@ -1157,7 +1160,7 @@ void intel_lr_context_unpin(struct intel_context *ctx,
WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
if (--ctx->engine[engine->id].pin_count == 0) {
- kunmap(kmap_to_page(ctx->engine[engine->id].lrc_reg_state));
+ i915_gem_object_unpin_map(ctx_obj);
intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
ctx->engine[engine->id].lrc_vma = NULL;
@@ -2054,7 +2057,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
*engine)
i915_gem_batch_pool_fini(&engine->batch_pool);
if (engine->status_page.obj) {
- kunmap(sg_page(engine->status_page.obj->pages->sgl));
+ i915_gem_object_unpin_map(engine->status_page.obj);
engine->status_page.obj = NULL;
}
@@ -2092,18 +2095,22 @@ logical_ring_default_irqs(struct intel_engine_cs
*engine, unsigned shift)
engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
}
-static void
+static int
lrc_setup_hws(struct intel_engine_cs *engine,
struct drm_i915_gem_object *dctx_obj)
{
- struct page *page;
+ void *hws;
/* The HWSP is part of the default context object in LRC mode. */
engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(dctx_obj) +
LRC_PPHWSP_PN * PAGE_SIZE;
- page = i915_gem_object_get_page(dctx_obj, LRC_PPHWSP_PN);
- engine->status_page.page_addr = kmap(page);
+ hws = i915_gem_object_pin_map(dctx_obj);
+ if (IS_ERR(hws))
+ return PT
Re: [Intel-gfx] [PATCH 07/10] drm/virtio: Drop dummy gamma table support
On Tue, 12 Apr 2016, Emil Velikov wrote:
> On 30 March 2016 at 10:51, Daniel Vetter wrote:
> > No need to confuse userspace like this.
> >
> > Cc: Gerd Hoffmann
> > Cc: Dave Airlie
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/virtio/virtgpu_display.c | 9 -
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> > b/drivers/gpu/drm/virtio/virtgpu_display.c
> > index 4854dac87e24..12b72e29678a 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> > @@ -38,13 +38,6 @@
> > #define XRES_MAX 8192
> > #define YRES_MAX 8192
> >
> > -static void virtio_gpu_crtc_gamma_set(struct drm_crtc *crtc,
> > - u16 *red, u16 *green, u16 *blue,
> > - uint32_t start, uint32_t size)
> > -{
> > - /* TODO */
> > -}
> > -
> > static void
> > virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev,
> >struct virtio_gpu_output *output)
> > @@ -173,7 +166,6 @@ static int virtio_gpu_page_flip(struct drm_crtc *crtc,
> > static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> > .cursor_set2= virtio_gpu_crtc_cursor_set,
> > .cursor_move= virtio_gpu_crtc_cursor_move,
> > - .gamma_set = virtio_gpu_crtc_gamma_set,
> > .set_config = drm_atomic_helper_set_config,
> > .destroy= drm_crtc_cleanup,
> >
> > @@ -416,7 +408,6 @@ static int vgdev_output_init(struct virtio_gpu_device
> > *vgdev, int index)
> > return PTR_ERR(plane);
> > drm_crtc_init_with_planes(dev, crtc, plane, NULL,
> > &virtio_gpu_crtc_funcs, NULL);
> > - drm_mode_crtc_set_gamma_size(crtc, 256);
> > drm_crtc_helper_add(crtc, &virtio_gpu_crtc_helper_funcs);
> > plane->crtc = crtc;
> >
> Out of curiosity:
>
> Coccinelle should be able to handle/generate such patches, shouldn't
> it ? I believe in the past people used it for similar
> refactoring/cleanups, yet not (m)any of them [the cocci files] got
> checked in the kernel tree.
>
> Thinking about future drivers derived from outdated sources - do you
> think it's a good/bad idea to check/run them along side the existing
> ones ?
The issue is that there is no point to put an empty function in a
structure?
It would be a bit subtle for Coccinelle, because it requires also knowing
that one is allowed to leave that particular field empty.
julia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/core: Add drm_accurate_vblank_count, v3.
On Tue, Apr 12, 2016 at 04:32:19PM +0200, Maarten Lankhorst wrote:
> This function is useful for gen2 intel devices which have no frame
> counter, but need a way to determine the current vblank count without
> racing with the vblank interrupt handler.
>
> intel_pipe_update_start checks if no vblank interrupt will occur
> during vblank evasion, but cannot check whether the vblank handler has
> run to completion. This function uses the timestamps to determine
> when the last vblank has happened, and interpolates from there.
>
> Changes since v1:
> - Take vblank_time_lock and don't use drm_vblank_count_and_time.
> Changes since v2:
> - Don't return time of last vblank.
>
> Cc: Mario Kleiner
> Cc: Ville Syrjälä
> Signed-off-by: Maarten Lankhorst
> ---
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3c1a6f18e71c..32bfa4bb8f41 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -303,6 +303,32 @@ static void drm_update_vblank_count(struct drm_device
> *dev, unsigned int pipe,
> store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
> }
>
> +/**
> + * drm_accurate_vblank_count - retrieve the master vblank counter
> + * @crtc: which counter to retrieve
> + * @tv_ret: last time counter was updated
> + *
> + * This function is similar to @drm_crtc_vblank_count but this
> + * function interpolates to handle a race with vblank irq's.
> + */
> +
> +u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + u32 vblank, pipe = drm_crtc_index(crtc);
pipe should be 'unsigned int' for consistency.
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->vblank_time_lock, flags);
> +
> + drm_update_vblank_count(dev, pipe, 0);
One thing that came to mind is some callers might end up doing the
update twice if the irq wasn't yet enabled when drm_vblank_get() was
called. Eg. drm_wait_one_vblank() might do that. So it might be a bit
more efficient to add a drm_vblank_get_and_update() instead, or something
like that. But I don't really care too much.
Reviewed-by: Ville Syrjälä
> + vblank = dev->vblank[pipe].count;
> +
> + spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
> +
> + return vblank;
> +}
> +EXPORT_SYMBOL(drm_accurate_vblank_count);
> +
> /*
> * Disable vblank irq's on crtc, make sure that last vblank count
> * of hardware and corresponding consistent software vblank counter
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 31483c2fef51..747c80da70e5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
> extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
> extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
> extern void drm_vblank_cleanup(struct drm_device *dev);
> +extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
> extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int
> pipe);
>
> extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
