Re: [Intel-gfx] [PATCH v8 05/35] drm/i915: MEI interface definition
On 12/7/2018 11:22 AM, C, Ramalingam wrote:
On 12/6/2018 3:53 PM, Daniel Vetter wrote:
On Tue, Nov 27, 2018 at 04:13:03PM +0530, Ramalingam C wrote:
Defining the mei-i915 interface functions and initialization of
the interface.
Signed-off-by: Ramalingam C
Signed-off-by: Tomas Winkler
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_drv.h | 7 +
drivers/gpu/drm/i915/intel_hdcp.c | 442 +-
include/drm/i915_component.h | 71 ++
4 files changed, 521 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f763b30f98d9..b68bc980b7cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2015,6 +2015,8 @@ struct drm_i915_private {
struct i915_pmu pmu;
+ struct i915_hdcp_component_master *hdcp_comp;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85a526598096..bde82f3ada85 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include "i915_drv.h"
#include
@@ -379,6 +380,9 @@ struct intel_hdcp_shim {
/* Detects panel's hdcp capability. This is optional for HDMI. */
int (*hdcp_capable)(struct intel_digital_port *intel_dig_port,
bool *hdcp_capable);
+
+ /* Detects the HDCP protocol(DP/HDMI) required on the port */
+ enum mei_hdcp_wired_protocol (*hdcp_protocol)(void);
Looking ahead, this seems hardwired to constant return value? Or why do we
need a function here?
This is hardwired based on the connector type(DP/HDMI).
Since we have the shim for hdcp's connector based work, I have added this
function.
Could have done this just with connector_type check, but in that way whole
hdcp_shim
can be done in that way. So going with the larger design here.
};
struct intel_hdcp {
@@ -399,6 +403,9 @@ struct intel_hdcp {
* content can flow only through a link protected by HDCP2.2.
*/
u8 content_type;
+
+ /* mei interface related information */
+ struct mei_hdcp_data mei_data;
};
struct intel_connector {
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
b/drivers/gpu/drm/i915/intel_hdcp.c
index 99dddb540958..760780f1105c 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -8,14 +8,20 @@
#include
#include
+#include
#include
#include
+#include
#include "intel_drv.h"
#include "i915_reg.h"
#define KEY_LOAD_TRIES 5
#define TIME_FOR_ENCRYPT_STATUS_CHANGE50
+#define GET_MEI_DDI_INDEX(p) ({\
+ typeof(p) __p = (p); \
+ __p == PORT_A ? MEI_DDI_A : (enum mei_hdcp_ddi)__p;\
+})
static
bool intel_hdcp_is_ksv_valid(u8 *ksv)
@@ -833,6 +839,417 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv,
enum port port)
!IS_CHERRYVIEW(dev_priv) && port < PORT_E);
}
+static __attribute__((unused)) int
+hdcp2_prepare_ake_init(struct intel_connector *connector,
+ struct hdcp2_ake_init *ake_data)
+{
+ struct mei_hdcp_data *data = >hdcp.mei_data;
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp;
+ int ret;
+
+ if (!comp)
+ return -EINVAL;
+
+ mutex_lock(>mutex);
+ if (!comp->ops || !comp->dev) {
+ mutex_unlock(>mutex);
+ return -EINVAL;
+ }
+
+ if (data->port == MEI_DDI_INVALID_PORT && connector->encoder)
+ data->port = GET_MEI_DDI_INDEX(connector->encoder->port);
+
+ /* Clear ME FW instance for the port, just incase */
+ comp->ops->close_hdcp_session(comp->dev, data);
Sounds like a bug somewhere if we need this? I'd put this code into the
->initiate_hdcp2_session, with a big WARN_ON if it's actually needed.
Not really. Lets say you have progressed beyond the first step of HDCP2.2 auth
along with ME FW.
Now if authentication failed due to some reason, then the HDCP2.2 season
created with
ME FW for that port is not closed yet.
So we need to call close_hdcp_session() explicitly on authentication failures.
Session has to be closed before restarting the auth on that port with
initialite_hdcp_session.
If we are closing the hdcp session of the port on all auth errors, we dont need
this just before
start of the hdcp session.
"Just in case" papering over programming bugs of our own just makes
debugging harder.
+
+ ret = comp->ops->initiate_hdcp2_session(comp->dev,
+ data, ake_data);
We should set
Re: [Intel-gfx] [PATCH v8 05/35] drm/i915: MEI interface definition
On 12/7/2018 11:22 AM, C, Ramalingam wrote:
On 12/6/2018 3:53 PM, Daniel Vetter wrote:
On Tue, Nov 27, 2018 at 04:13:03PM +0530, Ramalingam C wrote:
Defining the mei-i915 interface functions and initialization of
the interface.
Signed-off-by: Ramalingam C
Signed-off-by: Tomas Winkler
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_drv.h | 7 +
drivers/gpu/drm/i915/intel_hdcp.c | 442 +-
include/drm/i915_component.h | 71 ++
4 files changed, 521 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f763b30f98d9..b68bc980b7cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2015,6 +2015,8 @@ struct drm_i915_private {
struct i915_pmu pmu;
+ struct i915_hdcp_component_master *hdcp_comp;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85a526598096..bde82f3ada85 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include "i915_drv.h"
#include
@@ -379,6 +380,9 @@ struct intel_hdcp_shim {
/* Detects panel's hdcp capability. This is optional for HDMI. */
int (*hdcp_capable)(struct intel_digital_port *intel_dig_port,
bool *hdcp_capable);
+
+ /* Detects the HDCP protocol(DP/HDMI) required on the port */
+ enum mei_hdcp_wired_protocol (*hdcp_protocol)(void);
Looking ahead, this seems hardwired to constant return value? Or why do we
need a function here?
This is hardwired based on the connector type(DP/HDMI).
Since we have the shim for hdcp's connector based work, I have added this
function.
Could have done this just with connector_type check, but in that way whole
hdcp_shim
can be done in that way. So going with the larger design here.
};
struct intel_hdcp {
@@ -399,6 +403,9 @@ struct intel_hdcp {
* content can flow only through a link protected by HDCP2.2.
*/
u8 content_type;
+
+ /* mei interface related information */
+ struct mei_hdcp_data mei_data;
};
struct intel_connector {
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
b/drivers/gpu/drm/i915/intel_hdcp.c
index 99dddb540958..760780f1105c 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -8,14 +8,20 @@
#include
#include
+#include
#include
#include
+#include
#include "intel_drv.h"
#include "i915_reg.h"
#define KEY_LOAD_TRIES 5
#define TIME_FOR_ENCRYPT_STATUS_CHANGE50
+#define GET_MEI_DDI_INDEX(p) ({\
+ typeof(p) __p = (p); \
+ __p == PORT_A ? MEI_DDI_A : (enum mei_hdcp_ddi)__p;\
+})
static
bool intel_hdcp_is_ksv_valid(u8 *ksv)
@@ -833,6 +839,417 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv,
enum port port)
!IS_CHERRYVIEW(dev_priv) && port < PORT_E);
}
+static __attribute__((unused)) int
+hdcp2_prepare_ake_init(struct intel_connector *connector,
+ struct hdcp2_ake_init *ake_data)
+{
+ struct mei_hdcp_data *data = >hdcp.mei_data;
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp;
+ int ret;
+
+ if (!comp)
+ return -EINVAL;
+
+ mutex_lock(>mutex);
+ if (!comp->ops || !comp->dev) {
+ mutex_unlock(>mutex);
+ return -EINVAL;
+ }
+
+ if (data->port == MEI_DDI_INVALID_PORT && connector->encoder)
+ data->port = GET_MEI_DDI_INDEX(connector->encoder->port);
+
+ /* Clear ME FW instance for the port, just incase */
+ comp->ops->close_hdcp_session(comp->dev, data);
Sounds like a bug somewhere if we need this? I'd put this code into the
->initiate_hdcp2_session, with a big WARN_ON if it's actually needed.
Not really. Lets say you have progressed beyond the first step of HDCP2.2 auth
along with ME FW.
Now if authentication failed due to some reason, then the HDCP2.2 season
created with
ME FW for that port is not closed yet.
So we need to call close_hdcp_session() explicitly on authentication failures.
Session has to be closed before restarting the auth on that port with
initialite_hdcp_session.
If we are closing the hdcp session of the port on all auth errors, we dont need
this just before
start of the hdcp session.
"Just in case" papering over programming bugs of our own just makes
debugging harder.
+
+ ret = comp->ops->initiate_hdcp2_session(comp->dev,
+ data, ake_data);
We should set
Re: [Intel-gfx] [PATCH] drm/i915: Skip the ERR_PTR error state
On 07/12/2018 09:16, Chris Wilson wrote:
Although commit fb6f0b64e455 ("drm/i915: Prevent machine hang from
Broxton's vtd w/a and error capture") applied cleanly after a 24 month
hiatus, the code had moved on with new methods for peeking and fetching
the captured gpu info. Make sure we catch all uses of the stashed error
state and avoid dereferencing the error pointer.
v2: Move error pointer determination into i915_gpu_capture_state
Fixes: fb6f0b64e455 ("drm/i915: Prevent machine hang from Broxton's vtd w/a and
error capture")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 +---
drivers/gpu/drm/i915/i915_gpu_error.c | 26 ++
drivers/gpu/drm/i915/i915_sysfs.c | 4 +++-
3 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 38dcee1ca062..40a61ef9aac1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -984,8 +984,8 @@ static int i915_gpu_info_open(struct inode *inode, struct
file *file)
intel_runtime_pm_get(i915);
gpu = i915_capture_gpu_state(i915);
intel_runtime_pm_put(i915);
- if (!gpu)
- return -ENOMEM;
+ if (IS_ERR(gpu))
+ return PTR_ERR(gpu);
file->private_data = gpu;
return 0;
@@ -1018,7 +1018,13 @@ i915_error_state_write(struct file *filp,
static int i915_error_state_open(struct inode *inode, struct file *file)
{
- file->private_data = i915_first_error_state(inode->i_private);
+ struct i915_gpu_state *error;
+
+ error = i915_first_error_state(inode->i_private);
+ if (IS_ERR(error))
+ return PTR_ERR(error);
+
+ file->private_data = error;
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 07465123c166..9dc6600544c5 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1907,9 +1907,16 @@ i915_capture_gpu_state(struct drm_i915_private *i915)
{
struct i915_gpu_state *error;
+ /* Check if GPU capture has been disabled */
+ error = READ_ONCE(i915->gpu_error.first_error);
+ if (IS_ERR(error))
+ return error;
+
error = kzalloc(sizeof(*error), GFP_ATOMIC);
- if (!error)
- return NULL;
+ if (!error) {
+ i915_disable_error_state(i915, -ENOMEM);
+ return ERR_PTR(-ENOMEM);
+ }
kref_init(>ref);
error->i915 = i915;
@@ -1941,15 +1948,9 @@ void i915_capture_error_state(struct drm_i915_private
*i915,
if (!i915_modparams.error_capture)
return;
- if (READ_ONCE(i915->gpu_error.first_error))
- return;
-
Will it now overwrite the first error on subsequent hangs?
i915_capture_gpu_state only does early exit on stored error.
Regards,
Tvrtko
error = i915_capture_gpu_state(i915);
- if (!error) {
- DRM_DEBUG_DRIVER("out of memory, not capturing error state\n");
- i915_disable_error_state(i915, -ENOMEM);
+ if (IS_ERR(error))
return;
- }
i915_error_capture_msg(i915, error, engine_mask, error_msg);
DRM_INFO("%s\n", error->error_msg);
@@ -1987,7 +1988,7 @@ i915_first_error_state(struct drm_i915_private *i915)
spin_lock_irq(>gpu_error.lock);
error = i915->gpu_error.first_error;
- if (error)
+ if (!IS_ERR_OR_NULL(error))
i915_gpu_state_get(error);
spin_unlock_irq(>gpu_error.lock);
@@ -2000,10 +2001,11 @@ void i915_reset_error_state(struct drm_i915_private *i915)
spin_lock_irq(>gpu_error.lock);
error = i915->gpu_error.first_error;
- i915->gpu_error.first_error = NULL;
+ if (error != ERR_PTR(-ENODEV)) /* if disabled, always disabled */
+ i915->gpu_error.first_error = NULL;
spin_unlock_irq(>gpu_error.lock);
- if (!IS_ERR(error))
+ if (!IS_ERR_OR_NULL(error))
i915_gpu_state_put(error);
}
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 535caebd9813..c0cfe7ae2ba5 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -521,7 +521,9 @@ static ssize_t error_state_read(struct file *filp, struct
kobject *kobj,
ssize_t ret;
gpu = i915_first_error_state(i915);
- if (gpu) {
+ if (IS_ERR(gpu)) {
+ ret = PTR_ERR(gpu);
+ } else if (gpu) {
ret = i915_gpu_state_copy_to_buffer(gpu, buf, off, count);
i915_gpu_state_put(gpu);
} else {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, Of course one severe fix appeared on Wednesday, so here it is (with the GVT fix). Fix for system crash after GPU hang (Bugzilla #107945) and GVT fix for guest graphics corruption (https://github.com/intel/gvt-linux/issues/61) Regards, Joonas *** drm-intel-fixes-2018-12-07: - Fix for system crash after GPU hang (Bugzilla #107945) - GVT fix for guest graphics corruption (https://github.com/intel/gvt-linux/issues/61) The following changes since commit 2595646791c319cadfdbf271563aac97d0843dc7: Linux 4.20-rc5 (2018-12-02 15:07:55 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2018-12-07 for you to fetch changes up to d76b21ebf8ff0ec5030e98c64c50dca2254474f3: Merge tag 'gvt-fixes-2018-12-04' of https://github.com/intel/gvt-linux into drm-intel-fixes (2018-12-05 15:51:47 +0200) - Fix for system crash after GPU hang (Bugzilla #107945) - GVT fix for guest graphics corruption (https://github.com/intel/gvt-linux/issues/61) Joonas Lahtinen (1): Merge tag 'gvt-fixes-2018-12-04' of https://github.com/intel/gvt-linux into drm-intel-fixes Tina Zhang (1): drm/i915/gvt: Fix tiled memory decoding bug on BDW Tvrtko Ursulin (2): drm/i915: Record GT workarounds in a list drm/i915: Introduce per-engine workarounds drivers/gpu/drm/i915/gvt/fb_decoder.c| 2 +- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 + drivers/gpu/drm/i915/intel_lrc.c | 4 + drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + drivers/gpu/drm/i915/intel_workarounds.c | 591 --- drivers/gpu/drm/i915/intel_workarounds.h | 26 +- 9 files changed, 430 insertions(+), 204 deletions(-) ___ 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 igt: add timeline test cases (rev2)
Quoting zhoucm1 (2018-12-07 10:36:37) > > > On 2018年12月07日 18:29, Chris Wilson wrote: > > Quoting Patchwork (2018-12-07 10:27:46) > >> == Series Details == > >> > >> Series: igt: add timeline test cases (rev2) > >> URL : https://patchwork.freedesktop.org/series/53743/ > >> State : failure > >> > >> == Summary == > >> > >> CI Bug Log - changes from CI_DRM_5281 -> IGTPW_2133 > >> > >> > >> Summary > >> --- > >> > >>**FAILURE** > >> > >>Serious unknown changes coming with IGTPW_2133 absolutely need to be > >>verified manually. > >> > >>If you think the reported changes have nothing to do with the changes > >>introduced in IGTPW_2133, please notify your bug team to allow them > >>to document this new failure mode, which will reduce false positives in > >> CI. > >> > >>External URL: > >> https://patchwork.freedesktop.org/api/1.0/series/53743/revisions/2/mbox/ > >> > >> Possible new issues > >> --- > >> > >>Here are the unknown changes that may have been introduced in > >> IGTPW_2133: > >> > >> ### IGT changes ### > >> > >> Possible regressions > >> > >>* igt@amdgpu/amd_basic@userptr: > >> - fi-kbl-8809g: PASS -> DMESG-WARN > > What fortuitous timing! Maybe you would like to take a stab at the > > use-after-free in amdgpu's mmu_notifier. > It's totally new test cases and should be nothing with others, Do you > mean I need to resend it after sometime? It's an unrelated failure that has been plaguing us ever since we added a kbl-g to our farm. But as it is a use-after-free with core mm coupling, it's quite serious. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Pipeline PDP updates for Braswell
Quoting Tvrtko Ursulin (2018-12-07 10:30:53) > Is BSW fixed now? Or just a tiny bit better? For some value of a lot better. gem_concurrent_blit is still getting the odd false-positive missed-breadcrumb report, the occasional incoherency (I think is insufficient mb around fence register changes) and a rare invalid TLB GPU hang (EMIT_INVALIDATE harder?). On the positive side, the machine hasn't died for a few days (the LRI forcewake errors kill the machine), and at its worst gem_concurrent_blit could GPU hang within a few seconds. However, not once have I been able to reproduce the GPU hangs with gem_ctx_create and friends as seen by CI, so more than likely we are looking at more than one problem. Fingers crossed this is sufficient to shut CI up (with the exception of those rare gem_mmap_gtt incoherency failures). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Pipeline PDP updates for Braswell
On 07/12/2018 09:02, Chris Wilson wrote:
Currently we face a severe problem on Braswell that manifests as invalid
ppGTT accesses. The code tries to maintain the PDP (page directory
pointers) inside the context in two ways, direct write into the context
and a pipelined LRI update. The direct write into the context is
fundamentally racy as it is unserialised with any access (read or write)
the GPU is doing. By asserting that Braswell is not used with vGPU
(currently an unsupported platform) we can eliminate the dangerous
direct write into the context image and solely use the pipelined update.
However, the LRI of the PDP fouls up the GPU, causing it to freeze and
take out the machine with "forcewake ack timeouts". This seems possible
to workaround by preventing the GPU from sleeping (via means of
disabling the power-state management interface, i.e. forcing each ring
to remain awake) around the update.
Changelog is missing.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108656
References: https://bugs.freedesktop.org/show_bug.cgi?id=108714
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 -
drivers/gpu/drm/i915/intel_lrc.c| 140 ++--
2 files changed, 69 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index add1fe7aeb93..62bde517d383 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1423,8 +1423,6 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space
*vm,
gen8_initialize_pd(vm, pd);
gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
GEM_BUG_ON(pdp->used_pdpes > i915_pdpes_per_pdp(vm));
-
- mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
}
ret = gen8_ppgtt_alloc_pd(vm, pd, start, length);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b1f5db3442eb..c84bdc21bcce 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -363,31 +363,12 @@ execlists_context_schedule_out(struct i915_request *rq,
unsigned long status)
trace_i915_request_out(rq);
}
-static void
-execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state)
-{
- ASSIGN_CTX_PDP(ppgtt, reg_state, 3);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 2);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 1);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
-}
-
static u64 execlists_update_context(struct i915_request *rq)
{
- struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt;
struct intel_context *ce = rq->hw_context;
- u32 *reg_state = ce->lrc_reg_state;
- reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
-
- /*
-* True 32b PPGTT with dynamic page allocation: update PDP
-* registers and point the unallocated PDPs to scratch page.
-* PML4 is allocated during ppgtt init, so this is not needed
-* in 48-bit mode.
-*/
- if (!i915_vm_is_48bit(>vm))
- execlists_update_context_pdps(ppgtt, reg_state);
+ ce->lrc_reg_state[CTX_RING_TAIL + 1] =
+ intel_ring_set_tail(rq->ring, rq->tail);
/*
* Make sure the context image is complete before we submit it to HW.
@@ -1247,6 +1228,59 @@ execlists_context_pin(struct intel_engine_cs *engine,
return __execlists_context_pin(engine, ctx, ce);
}
+static int emit_pdps(struct i915_request *rq)
+{
+ const struct intel_engine_cs * const engine = rq->engine;
+ struct i915_hw_ppgtt * const ppgtt = rq->gem_context->ppgtt;
+ int err, i;
+ u32 *cs;
+
+ GEM_BUG_ON(intel_vgpu_active(rq->i915));
+
+ /*
+* Beware ye of the dragons, this sequence is magic!
+*
+* Small changes to this sequence can cause anything from
+* GPU hangs to forcewake errors and machine lockups!
+*/
+
+ /* Flush any residual operations from the context load */
+ err = engine->emit_flush(rq, EMIT_FLUSH);
+ if (err)
+ return err;
+
+ /* Magic required to prevent forcewake errors! */
+ err = engine->emit_flush(rq, EMIT_INVALIDATE);
+ if (err)
+ return err;
+
+ cs = intel_ring_begin(rq, 4 * GEN8_3LVL_PDPES + 2);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+
+ /* Ensure the LRI have landed before we invalidate & continue */
+ *cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED;
+ for (i = GEN8_3LVL_PDPES; i--; ) {
+ const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
+
+ *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(engine, i));
+ *cs++ = upper_32_bits(pd_daddr);
+ *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(engine, i));
+ *cs++ =
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for igt: add timeline test cases (rev2)
Quoting Patchwork (2018-12-07 10:27:46) > == Series Details == > > Series: igt: add timeline test cases (rev2) > URL : https://patchwork.freedesktop.org/series/53743/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_5281 -> IGTPW_2133 > > > Summary > --- > > **FAILURE** > > Serious unknown changes coming with IGTPW_2133 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in IGTPW_2133, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: > https://patchwork.freedesktop.org/api/1.0/series/53743/revisions/2/mbox/ > > Possible new issues > --- > > Here are the unknown changes that may have been introduced in IGTPW_2133: > > ### IGT changes ### > > Possible regressions > > * igt@amdgpu/amd_basic@userptr: > - fi-kbl-8809g: PASS -> DMESG-WARN What fortuitous timing! Maybe you would like to take a stab at the use-after-free in amdgpu's mmu_notifier. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFT i-g-t 1/2] tests/gem_shrink: Background, direct and OOM shrinker plus userptr tests
On 07/12/2018 10:17, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-12-07 10:14:53)
+static void trigger_oom(void)
+{
+ const char *cmd = "f";
+int fd;
+
+fd = open("/proc/sysrq-trigger", O_WRONLY);
+igt_assert_fd(fd);
/proc/sysrq-trigger may not exist. subgroup fixture igt_require?
I guess so. Marking for TODO. First I need to re-visit whether my test
is actually exercising all three reclaim path (or better say shrink_lock
paths), since afterwards I figured out there is some "misleadingness" there.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for igt: add timeline test cases (rev2)
== Series Details ==
Series: igt: add timeline test cases (rev2)
URL : https://patchwork.freedesktop.org/series/53743/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5281 -> IGTPW_2133
Summary
---
**FAILURE**
Serious unknown changes coming with IGTPW_2133 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_2133, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/53743/revisions/2/mbox/
Possible new issues
---
Here are the unknown changes that may have been introduced in IGTPW_2133:
### IGT changes ###
Possible regressions
* igt@amdgpu/amd_basic@userptr:
- fi-kbl-8809g: PASS -> DMESG-WARN
Known issues
Here are the changes found in IGTPW_2133 that come from known issues:
### IGT changes ###
Issues hit
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@gem_exec_suspend@basic-s4-devices:
- fi-ivb-3520m: PASS -> FAIL [fdo#108880]
* igt@i915_selftest@live_evict:
- fi-bsw-kefka: PASS -> DMESG-WARN [fdo#107709]
* {igt@runner@aborted}:
- fi-bsw-kefka: NOTRUN -> FAIL [fdo#107709]
Possible fixes
* igt@i915_selftest@live_execlists:
- fi-apl-guc: INCOMPLETE [fdo#103927] -> PASS
* igt@i915_selftest@live_hangcheck:
- fi-bwr-2160:DMESG-FAIL [fdo#108735] -> PASS
- fi-skl-gvtdvm: INCOMPLETE [fdo#105600] / [fdo#108744] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#105600]: https://bugs.freedesktop.org/show_bug.cgi?id=105600
[fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880
Participating hosts (50 -> 44)
--
Additional (1): fi-byt-j1900
Missing(7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600 fi-icl-y
Build changes
-
* IGT: IGT_4743 -> IGTPW_2133
CI_DRM_5281: 678f4e6d807098fc02d94627490658d17be1080d @
git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2133: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2133/
IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Testlist changes ==
+++ 118 lines
--- 0 lines
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2133/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/ringbuffer: EMIT_INVALIDATE after switch context
On 07/12/2018 09:02, Chris Wilson wrote: The recommend procedure was to switch contexts (and mm) then invalidate the TLBs. Make it so. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 16084749adf5..74a4d587c312 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1827,12 +1827,12 @@ static int ring_request_alloc(struct i915_request *request) */ request->reserved_space += LEGACY_REQUEST_SIZE; - /* Unconditionally invalidate GPU caches and TLBs. */ - ret = request->engine->emit_flush(request, EMIT_INVALIDATE); + ret = switch_context(request); if (ret) return ret; - ret = switch_context(request); + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = request->engine->emit_flush(request, EMIT_INVALIDATE); if (ret) return ret; Before my time I'm afraid, but makes some sense on the logical level: Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ 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: Push EMIT_INVALIDATE at request start to backends
On 07/12/2018 09:02, Chris Wilson wrote: Move the common engine->emit_flush(EMIT_INVALIDATE) back to the backends (where it was once previously) as we seek to specialise it in future patches. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_request.c | 5 - drivers/gpu/drm/i915/intel_lrc.c| 9 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 -- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index ca95ab2f4cfa..8ab8e8e6a086 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -719,11 +719,6 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) */ rq->head = rq->ring->emit; - /* Unconditionally invalidate GPU caches and TLBs. */ - ret = engine->emit_flush(rq, EMIT_INVALIDATE); - if (ret) - goto err_unwind; - ret = engine->request_alloc(rq); if (ret) goto err_unwind; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 27d3a780611a..b1f5db3442eb 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1253,17 +1253,20 @@ static int execlists_request_alloc(struct i915_request *request) GEM_BUG_ON(!request->hw_context->pin_count); - /* Flush enough space to reduce the likelihood of waiting after + /* +* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just * have to repeat work. */ request->reserved_space += EXECLISTS_REQUEST_SIZE; - ret = intel_ring_wait_for_space(request->ring, request->reserved_space); + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = request->engine->emit_flush(request, EMIT_INVALIDATE); if (ret) return ret; - /* Note that after this point, we have committed to using + /* +* Note that after this point, we have committed to using * this request as it is being used to both track the * state of engine initialisation and liveness of the * golden renderstate above. Think twice before you try diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c5eb26a7ee79..16084749adf5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1820,13 +1820,15 @@ static int ring_request_alloc(struct i915_request *request) GEM_BUG_ON(!request->hw_context->pin_count); - /* Flush enough space to reduce the likelihood of waiting after + /* +* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just * have to repeat work. */ request->reserved_space += LEGACY_REQUEST_SIZE; - ret = intel_ring_wait_for_space(request->ring, request->reserved_space); + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = request->engine->emit_flush(request, EMIT_INVALIDATE); if (ret) return ret; intel_ring_wait_for_space is the bit paranoid me actually wanted to have split out. But okay, maybe I did not say it clear enough. This already helps singling out that change should something unexpected happen. Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFT i-g-t 1/2] tests/gem_shrink: Background, direct and OOM shrinker plus userptr tests
Quoting Tvrtko Ursulin (2018-12-07 10:14:53)
> +static void trigger_oom(void)
> +{
> + const char *cmd = "f";
> +int fd;
> +
> +fd = open("/proc/sysrq-trigger", O_WRONLY);
> +igt_assert_fd(fd);
/proc/sysrq-trigger may not exist. subgroup fixture igt_require?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFT i-g-t 1/2] tests/gem_shrink: Background, direct and OOM shrinker plus userptr tests
From: Tvrtko Ursulin
...
Signed-off-by: Tvrtko Ursulin
---
lib/igt_core.c | 18
lib/igt_core.h | 1 +
tests/i915/gem_shrink.c | 213
3 files changed, 232 insertions(+)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 64883d6402af..d8fa0c83e279 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1680,6 +1680,24 @@ void igt_stop_helper(struct igt_helper_process *proc)
assert(helper_was_alive(proc, status));
}
+/**
+ * igt_try_stop_helper:
+ * @proc: #igt_helper_process structure
+ *
+ * Terminates a helper process if it is still running.
+ */
+void igt_try_stop_helper(struct igt_helper_process *proc)
+{
+ int status;
+
+ /* failure here means the pid is already dead and so waiting is safe */
+ kill(proc->pid, proc->use_SIGKILL ? SIGKILL : SIGTERM);
+
+ status = igt_wait_helper(proc);
+ if (!helper_was_alive(proc, status))
+ igt_debug("Helper died too early with status=%d\n", status);
+}
+
static void children_exit_handler(int sig)
{
int status;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 6f8c3852a686..beec34667524 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -795,6 +795,7 @@ bool __igt_fork_helper(struct igt_helper_process *proc);
for (; __igt_fork_helper(proc); exit(0))
int igt_wait_helper(struct igt_helper_process *proc);
void igt_stop_helper(struct igt_helper_process *proc);
+void igt_try_stop_helper(struct igt_helper_process *proc);
/* exit handler code */
diff --git a/tests/i915/gem_shrink.c b/tests/i915/gem_shrink.c
index c8e05814ee70..0071c1ae21ff 100644
--- a/tests/i915/gem_shrink.c
+++ b/tests/i915/gem_shrink.c
@@ -26,6 +26,9 @@
*
* Exercise the shrinker by overallocating GEM objects
*/
+#include
+#include
+#include
#include "igt.h"
#include "igt_gt.h"
@@ -366,6 +369,210 @@ static void reclaim(unsigned engine, int timeout)
close(fd);
}
+static unsigned long get_meminfo(const char *info, const char *tag)
+{
+ const char *str;
+ unsigned long val;
+
+ str = strstr(info, tag);
+ if (str && sscanf(str + strlen(tag), " %lu", ) == 1)
+ return val >> 10;
+
+ igt_warn("Unrecognised /proc/meminfo field: '%s'\n", tag);
+ return 0;
+}
+
+static unsigned long get_avail_ram_mb(void)
+{
+ int fd;
+ int ret;
+ char buf[4096];
+ unsigned long ram;
+
+ fd = open("/proc/meminfo", O_RDONLY);
+ igt_assert_fd(fd);
+
+ ret = read(fd, buf, sizeof(buf));
+ igt_assert(ret >= 0);
+
+ close(fd);
+
+ ram = get_meminfo(buf, "MemAvailable:");
+ ram += get_meminfo(buf, "Buffers:");
+ ram += get_meminfo(buf, "Cached:");
+ ram += get_meminfo(buf, "SwapCached:");
+
+ return ram;
+}
+
+struct test {
+#define TEST_BO(1)
+#define TEST_USERPTR (2)
+ unsigned int flags;
+ int fd;
+};
+
+static uint32_t __get_pages(int fd, unsigned long alloc)
+{
+ uint32_t handle = gem_create(fd, alloc);
+
+ gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, 0);
+ gem_madvise(fd, handle, I915_MADV_DONTNEED);
+
+ return handle;
+}
+
+struct test_obj {
+ void *ptr;
+ uint32_t handle;
+};
+
+static void
+__get_userptr(int fd, struct test_obj *obj, unsigned long sz)
+{
+ struct local_i915_gem_userptr userptr = { };
+ void *ptr;
+
+ igt_assert_eq(sz & 4095, 0);
+
+ ptr = mmap(NULL, sz, PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ assert(ptr != MAP_FAILED);
+
+ userptr.user_size = sz;
+ userptr.user_ptr = to_user_pointer(ptr);
+ do_ioctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, );
+
+ gem_set_domain(fd, userptr.handle, I915_GEM_DOMAIN_GTT, 0);
+
+ obj->ptr = ptr;
+ obj->handle = userptr.handle;
+}
+
+#define PAGE_SIZE 4096
+static void *mempressure(void *arg)
+{
+ struct test_obj *list = NULL;
+ struct test *test = arg;
+ const unsigned int sz_mb = 2;
+ const unsigned int sz = sz_mb << 20;
+ unsigned int n = 0, max = 0;
+ unsigned int blocks;
+
+ while (true) {
+ unsigned long ram_mb = get_avail_ram_mb();
+
+ if (!list) {
+ blocks = ram_mb / sz_mb;
+ list = calloc(blocks, sizeof(*list));
+ igt_assert(list);
+ } else if (ram_mb < 256) {
+ blocks = max + 1;
+ }
+
+ if (list[n].ptr || list[n].handle) {
+ if (test->flags & TEST_BO)
+ gem_close(test->fd, list[n].handle);
+ else
+ munmap(list[n].ptr, sz);
+ }
+
+ if (test->flags & TEST_BO) {
+ list[n].handle = __get_pages(test->fd, sz);
+ } else if (test->flags & TEST_USERPTR) {
+
[Intel-gfx] [PATCH i-g-t] igt: add timeline test cases
Signed-off-by: Chunming Zhou
---
include/drm-uapi/drm.h | 33 ++
lib/igt_syncobj.c| 204 +++
lib/igt_syncobj.h| 19 +
tests/meson.build|1 +
tests/syncobj_timeline.c | 1100 ++
5 files changed, 1357 insertions(+)
create mode 100644 tests/syncobj_timeline.c
diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h
index f0bd91de..0f5df0b6 100644
--- a/include/drm-uapi/drm.h
+++ b/include/drm-uapi/drm.h
@@ -715,6 +715,8 @@ struct drm_syncobj_handle {
#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
+/* wait for time point to become available */
+#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2)
struct drm_syncobj_wait {
__u64 handles;
/* absolute timeout */
@@ -725,11 +727,37 @@ struct drm_syncobj_wait {
__u32 pad;
};
+struct drm_syncobj_timeline_wait {
+__u64 handles;
+/* wait on specific timeline point for every handles*/
+__u64 points;
+/* absolute timeout */
+__s64 timeout_nsec;
+__u32 count_handles;
+__u32 flags;
+__u32 first_signaled; /* only valid when not waiting all */
+__u32 pad;
+};
+
struct drm_syncobj_array {
__u64 handles;
__u32 count_handles;
__u32 pad;
};
+struct drm_syncobj_timeline_array {
+ __u64 handles;
+ __u64 points;
+ __u32 count_handles;
+ __u32 pad;
+};
+
+struct drm_syncobj_transfer {
+__u32 binary_handle;
+__u32 timeline_handle;
+__u64 point;
+__u32 flags;
+__u32 pad;
+};
/* Query current scanout sequence number */
struct drm_crtc_get_sequence {
@@ -886,6 +914,11 @@ extern "C" {
#define DRM_IOCTL_MODE_LIST_LESSEESDRM_IOWR(0xC7, struct
drm_mode_list_lessees)
#define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct
drm_mode_get_lease)
#define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct
drm_mode_revoke_lease)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct
drm_syncobj_timeline_wait)
+#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct
drm_syncobj_timeline_array)
+#define DRM_IOCTL_SYNCOBJ_BINARY_TO_TIMELINEDRM_IOWR(0xCC, struct
drm_syncobj_transfer)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_TO_BINARYDRM_IOWR(0xCD, struct
drm_syncobj_transfer)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNALDRM_IOWR(0xCE, struct
drm_syncobj_timeline_array)
/**
* Device specific ioctls should only be in their respective headers
diff --git a/lib/igt_syncobj.c b/lib/igt_syncobj.c
index d9114ca8..a3b6a90b 100644
--- a/lib/igt_syncobj.c
+++ b/lib/igt_syncobj.c
@@ -286,3 +286,207 @@ syncobj_signal(int fd, uint32_t *handles, uint32_t count)
{
igt_assert_eq(__syncobj_signal(fd, handles, count), 0);
}
+
+static int
+__syncobj_timeline_signal(int fd, uint32_t *handles, uint64_t *points,
uint32_t count)
+{
+ struct drm_syncobj_timeline_array array = { 0 };
+ int err = 0;
+
+ array.handles = to_user_pointer(handles);
+ array.points = to_user_pointer(points);
+ array.count_handles = count;
+ if (drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, ))
+ err = -errno;
+ return err;
+}
+
+/**
+ * syncobj_signal:
+ * @fd: The DRM file descriptor.
+ * @handles: Array of syncobj handles to signal
+ * @points: List of point of handles to signal.
+ * @count: Count of syncobj handles.
+ *
+ * Signal a set of syncobjs.
+ */
+void
+syncobj_timeline_signal(int fd, uint32_t *handles, uint64_t *points, uint32_t
count)
+{
+ igt_assert_eq(__syncobj_timeline_signal(fd, handles, points, count), 0);
+}
+int
+__syncobj_timeline_wait_ioctl(int fd, struct drm_syncobj_timeline_wait *args)
+{
+ int err = 0;
+ if (drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, args))
+ err = -errno;
+ return err;
+}
+static int
+__syncobj_timeline_wait(int fd, uint32_t *handles, uint64_t *points,
+ unsigned num_handles,
+ int64_t timeout_nsec, unsigned flags,
+ uint32_t *first_signaled)
+{
+ struct drm_syncobj_timeline_wait args;
+ int ret;
+
+ args.handles = to_user_pointer(handles);
+ args.points = (uint64_t)to_user_pointer(points);
+ args.timeout_nsec = timeout_nsec;
+ args.count_handles = num_handles;
+ args.flags = flags;
+ args.first_signaled = 0;
+ args.pad = 0;
+
+ ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, );
+ if (ret < 0)
+ return -errno;
+
+ if (first_signaled)
+ *first_signaled = args.first_signaled;
+
+ return ret;
+}
+int
+syncobj_timeline_wait_err(int fd, uint32_t *handles, uint64_t *points,
+ unsigned num_handles,
+ int64_t timeout_nsec, unsigned flags)
+{
+ return __syncobj_timeline_wait(fd, handles, points,
Re: [Intel-gfx] [PATCH] drivers/base: use a worker for sysfs unbind
Quoting Daniel Vetter (2018-12-07 09:31:33)
> +void unbind_work_fn(struct work_struct *work)
> +{
> + struct unbind_work *unbind_work =
> + container_of(work, struct unbind_work, work);
> +
> + device_release_driver(unbind_work->dev);
> + put_device(unbind_work->dev);
> +}
> +
> /* Manually detach a device from its associated driver. */
> static ssize_t unbind_store(struct device_driver *drv, const char *buf,
> size_t count)
> {
> struct bus_type *bus = bus_get(drv->bus);
> + struct unbind_work *unbind_work;
> struct device *dev;
> int err = -ENODEV;
>
> dev = bus_find_device_by_name(bus, NULL, buf);
> if (dev && dev->driver == drv) {
> - if (dev->parent && dev->bus->need_parent_lock)
> - device_lock(dev->parent);
Do we not need to keep this locking in the worker?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drivers/base: use a worker for sysfs unbind
Drivers might want to remove some sysfs files, which needs the same
locks and ends up angering lockdep. Relevant snippet of the stack
trace:
kernfs_remove_by_name_ns+0x3b/0x80
bus_remove_driver+0x92/0xa0
acpi_video_unregister+0x24/0x40
i915_driver_unload+0x42/0x130 [i915]
i915_pci_remove+0x19/0x30 [i915]
pci_device_remove+0x36/0xb0
device_release_driver_internal+0x185/0x250
unbind_store+0xaf/0x180
kernfs_fop_write+0x104/0x190
I've stumbled over this because some new patches by Ram connect the
snd-hda-intel unload (where we do use sysfs unbind) with the locking
chains in the i915 unload code (but without creating a new loop),
which upset our CI. But the bug is already there and can be easily
reproduced by unbind i915 directly.
No idea whether this is the correct place to fix this, should at least
get CI happy again.
Cc: Ramalingam C
Signed-off-by: Daniel Vetter
---
drivers/base/bus.c | 33 +++--
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 8bfd27ec73d6..864412df86a9 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -174,22 +174,43 @@ static const struct kset_uevent_ops bus_uevent_ops = {
static struct kset *bus_kset;
+struct unbind_work {
+ struct work_struct work;
+ struct device *dev;
+};
+
+void unbind_work_fn(struct work_struct *work)
+{
+ struct unbind_work *unbind_work =
+ container_of(work, struct unbind_work, work);
+
+ device_release_driver(unbind_work->dev);
+ put_device(unbind_work->dev);
+}
+
/* Manually detach a device from its associated driver. */
static ssize_t unbind_store(struct device_driver *drv, const char *buf,
size_t count)
{
struct bus_type *bus = bus_get(drv->bus);
+ struct unbind_work *unbind_work;
struct device *dev;
int err = -ENODEV;
dev = bus_find_device_by_name(bus, NULL, buf);
if (dev && dev->driver == drv) {
- if (dev->parent && dev->bus->need_parent_lock)
- device_lock(dev->parent);
- device_release_driver(dev);
- if (dev->parent && dev->bus->need_parent_lock)
- device_unlock(dev->parent);
- err = count;
+ unbind_work = kmalloc(sizeof(*unbind_work), GFP_KERNEL);
+ if (unbind_work) {
+ unbind_work->dev = dev;
+ get_device(dev);
+ INIT_WORK(_work->work, unbind_work_fn);
+
+ schedule_work(_work->work);
+
+ err = count;
+ } else {
+ err = -ENOMEM;
+ }
}
put_device(dev);
bus_put(bus);
--
2.20.0.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev4)
== Series Details == Series: Change KVMGT into self loadable module (rev4) URL : https://patchwork.freedesktop.org/series/53379/ State : success == Summary == CI Bug Log - changes from CI_DRM_5280 -> Patchwork_11043 Summary --- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/53379/revisions/4/mbox/ Known issues Here are the changes found in Patchwork_11043 that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_create@basic-files: - fi-bsw-kefka: PASS -> FAIL [fdo#108656] * igt@gem_exec_suspend@basic-s3: - fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718] [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656 Participating hosts (45 -> 42) -- Additional (2): fi-skl-guc fi-skl-6600u Missing(5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y Build changes - * Linux: CI_DRM_5280 -> Patchwork_11043 CI_DRM_5280: 6047933c2fafdfd42353d735b213e74826d5a939 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_11043: 4517d0efeed2dfbf726017b688f12a5ba479e6ec @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 4517d0efeed2 drm/i915/gvt: Change KVMGT as self load module c9a054b04171 drm/i915/gvt: remove unused parameter for hypervisor's host_exit call ebe9b6bbbd81 drm/i915/gvt: mandatory require hypervisor's host_init == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11043/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Skip the ERR_PTR error state
Although commit fb6f0b64e455 ("drm/i915: Prevent machine hang from
Broxton's vtd w/a and error capture") applied cleanly after a 24 month
hiatus, the code had moved on with new methods for peeking and fetching
the captured gpu info. Make sure we catch all uses of the stashed error
state and avoid dereferencing the error pointer.
v2: Move error pointer determination into i915_gpu_capture_state
Fixes: fb6f0b64e455 ("drm/i915: Prevent machine hang from Broxton's vtd w/a and
error capture")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 +---
drivers/gpu/drm/i915/i915_gpu_error.c | 26 ++
drivers/gpu/drm/i915/i915_sysfs.c | 4 +++-
3 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 38dcee1ca062..40a61ef9aac1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -984,8 +984,8 @@ static int i915_gpu_info_open(struct inode *inode, struct
file *file)
intel_runtime_pm_get(i915);
gpu = i915_capture_gpu_state(i915);
intel_runtime_pm_put(i915);
- if (!gpu)
- return -ENOMEM;
+ if (IS_ERR(gpu))
+ return PTR_ERR(gpu);
file->private_data = gpu;
return 0;
@@ -1018,7 +1018,13 @@ i915_error_state_write(struct file *filp,
static int i915_error_state_open(struct inode *inode, struct file *file)
{
- file->private_data = i915_first_error_state(inode->i_private);
+ struct i915_gpu_state *error;
+
+ error = i915_first_error_state(inode->i_private);
+ if (IS_ERR(error))
+ return PTR_ERR(error);
+
+ file->private_data = error;
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 07465123c166..9dc6600544c5 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1907,9 +1907,16 @@ i915_capture_gpu_state(struct drm_i915_private *i915)
{
struct i915_gpu_state *error;
+ /* Check if GPU capture has been disabled */
+ error = READ_ONCE(i915->gpu_error.first_error);
+ if (IS_ERR(error))
+ return error;
+
error = kzalloc(sizeof(*error), GFP_ATOMIC);
- if (!error)
- return NULL;
+ if (!error) {
+ i915_disable_error_state(i915, -ENOMEM);
+ return ERR_PTR(-ENOMEM);
+ }
kref_init(>ref);
error->i915 = i915;
@@ -1941,15 +1948,9 @@ void i915_capture_error_state(struct drm_i915_private
*i915,
if (!i915_modparams.error_capture)
return;
- if (READ_ONCE(i915->gpu_error.first_error))
- return;
-
error = i915_capture_gpu_state(i915);
- if (!error) {
- DRM_DEBUG_DRIVER("out of memory, not capturing error state\n");
- i915_disable_error_state(i915, -ENOMEM);
+ if (IS_ERR(error))
return;
- }
i915_error_capture_msg(i915, error, engine_mask, error_msg);
DRM_INFO("%s\n", error->error_msg);
@@ -1987,7 +1988,7 @@ i915_first_error_state(struct drm_i915_private *i915)
spin_lock_irq(>gpu_error.lock);
error = i915->gpu_error.first_error;
- if (error)
+ if (!IS_ERR_OR_NULL(error))
i915_gpu_state_get(error);
spin_unlock_irq(>gpu_error.lock);
@@ -2000,10 +2001,11 @@ void i915_reset_error_state(struct drm_i915_private
*i915)
spin_lock_irq(>gpu_error.lock);
error = i915->gpu_error.first_error;
- i915->gpu_error.first_error = NULL;
+ if (error != ERR_PTR(-ENODEV)) /* if disabled, always disabled */
+ i915->gpu_error.first_error = NULL;
spin_unlock_irq(>gpu_error.lock);
- if (!IS_ERR(error))
+ if (!IS_ERR_OR_NULL(error))
i915_gpu_state_put(error);
}
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 535caebd9813..c0cfe7ae2ba5 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -521,7 +521,9 @@ static ssize_t error_state_read(struct file *filp, struct
kobject *kobj,
ssize_t ret;
gpu = i915_first_error_state(i915);
- if (gpu) {
+ if (IS_ERR(gpu)) {
+ ret = PTR_ERR(gpu);
+ } else if (gpu) {
ret = i915_gpu_state_copy_to_buffer(gpu, buf, off, count);
i915_gpu_state_put(gpu);
} else {
--
2.20.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/icl: Fix pipe config mismatch warnings
Fixes hblank, vblank, vsync_start/vsync_end,
hsync_start//hsync_end, pipe_bpp, port clock,
pixel rate mismatches for dsi which
happen during pipe_config comparation in
intel_atomic_check.
v2: Removed duplicate code, unused variables
and unneeded const modifier for pipe_config.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/icl_dsi.c | 90 +-
1 file changed, 78 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 4dd793b78996..951c9823b971 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -760,13 +760,6 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
vsync_end = adjusted_mode->crtc_vsync_end;
vsync_shift = hsync_start - htotal / 2;
- if (intel_dsi->dual_link) {
- hactive /= 2;
- if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
- hactive += intel_dsi->pixel_overlap;
- htotal /= 2;
- }
-
/* minimum hactive as per bspec: 256 pixels */
if (adjusted_mode->crtc_hdisplay < 256)
DRM_ERROR("hactive is less then 256 pixels\n");
@@ -794,11 +787,6 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
if (hback_porch < 16)
DRM_ERROR("hback porch < 16 pixels\n");
- if (intel_dsi->dual_link) {
- hsync_start /= 2;
- hsync_end /= 2;
- }
-
for_each_dsi_port(port, intel_dsi->ports) {
dsi_trans = dsi_port_to_transcoder(port);
I915_WRITE(HSYNC(dsi_trans),
@@ -844,6 +832,61 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
}
}
+static
+void
+gen11_dsi_calc_transcoder_timings(struct intel_encoder *encoder,
+ struct intel_crtc_state *pipe_config)
+{
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
+ struct drm_display_mode *adjusted_mode =
+ _config->base.adjusted_mode;
+ /* horizontal timings */
+ u16 htotal, hactive, hsync_start, hsync_end, hsync_size;
+ u16 hfront_porch, hback_porch;
+ /* vertical timings */
+ u16 vtotal, vactive, vsync_start, vsync_end, vsync_shift;
+
+ hactive = adjusted_mode->crtc_hdisplay;
+ htotal = adjusted_mode->crtc_htotal;
+ hsync_start = adjusted_mode->crtc_hsync_start;
+ hsync_end = adjusted_mode->crtc_hsync_end;
+ hsync_size = hsync_end - hsync_start;
+ hfront_porch = (adjusted_mode->crtc_hsync_start -
+ adjusted_mode->crtc_hdisplay);
+ hback_porch = (adjusted_mode->crtc_htotal -
+ adjusted_mode->crtc_hsync_end);
+ vactive = adjusted_mode->crtc_vdisplay;
+ vtotal = adjusted_mode->crtc_vtotal;
+ vsync_start = adjusted_mode->crtc_vsync_start;
+ vsync_end = adjusted_mode->crtc_vsync_end;
+ vsync_shift = hsync_start - htotal / 2;
+
+ if (intel_dsi->dual_link) {
+ hactive /= 2;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
+ hactive += intel_dsi->pixel_overlap;
+ htotal /= 2;
+ }
+
+ /* TRANS_HSYNC register to be programmed only for video mode */
+ if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
+
+ if (intel_dsi->dual_link) {
+ hsync_start /= 2;
+ hsync_end /= 2;
+ }
+ }
+
+ adjusted_mode->crtc_hdisplay = hactive;
+ adjusted_mode->crtc_htotal = htotal;
+ adjusted_mode->crtc_hsync_start = hsync_start;
+ adjusted_mode->crtc_hsync_end = hsync_end;
+ adjusted_mode->crtc_vdisplay = vactive;
+ adjusted_mode->crtc_vtotal = vtotal;
+ adjusted_mode->crtc_vsync_start = vsync_start;
+ adjusted_mode->crtc_vsync_end = vsync_end;
+}
+
static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -1169,6 +1212,9 @@ static void gen11_dsi_get_config(struct intel_encoder
*encoder,
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
+ struct drm_display_mode *adjusted_mode =
+ _config->base.adjusted_mode;
+ u32 bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
u32 pll_id;
/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
@@ -1176,6 +1222,19 @@ static void gen11_dsi_get_config(struct intel_encoder
*encoder,
pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
pipe_config->output_types |=
Re: [Intel-gfx] [PATCH v11 00/23] drm/i915/icl: dsi enabling
Hi, I decided to create a simple patch, which fixes those warns: [ 12.773332] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_hdisplay (expected 1440, found 720) [ 12.773425] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_htotal (expected 1586, found 793) [ 12.773510] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_hblank_start (expected 1440, found 1) [ 12.773588] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_hblank_end (expected 1586, found 1) [ 12.773663] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_hsync_start (expected 1540, found 770) [ 12.773735] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_hsync_end (expected 1550, found 775) [ 12.773813] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_vblank_start (expected 2560, found 1) [ 12.773897] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_vblank_end (expected 2582, found 1) [ 12.773976] [drm:pipe_config_err [i915]] *ERROR* mismatch in output_format (expected 0, found 1) [ 12.774039] [drm:pipe_config_err [i915]] *ERROR* mismatch in pixel_rate (expected 245700, found 122850) [ 12.774099] [drm:pipe_config_err [i915]] *ERROR* mismatch in pipe_bpp (expected 24, found 0) [ 12.774157] [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_clock (expected 245700, found 122850) The patch itself is here: https://patchwork.freedesktop.org/series/53727/ After that it complains only at output_format and scaler_id mismatch(which I currently don't know how to fix): [ 13.476929] [drm:pipe_config_err [i915]] *ERROR* mismatch in output_format (expected 0, found 1) [ 13.476994] [drm:pipe_config_err [i915]] *ERROR* mismatch in scaler_state.scaler_id (expected 0, found -1) Best Regards, Lisovskiy Stanislav Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo From: Chauhan, Madhav Sent: Wednesday, December 05, 2018 10:54 AM To: Lisovskiy, Stanislav; Nikula, Jani; intel-gfx@lists.freedesktop.org Cc: Peres, Martin; Saarinen, Jani; Kulkarni, Vandita Subject: RE: [PATCH v11 00/23] drm/i915/icl: dsi enabling However I have not seen fdo bug mentioned below, but root cause for these pipe config error messages are due to the fact that for dual link we are programming half the timings but while comparing We use undivided values. Moreover some timings are not even gets programmed for DSI like HBLANK/VBLANK etc. So while dumping pipe_config errors we need to consider dual link scenario for DSI. Regards, Madhav > -Original Message- > From: Lisovskiy, Stanislav > Sent: Wednesday, December 5, 2018 2:18 PM > To: Nikula, Jani ; Chauhan, Madhav > ; intel-gfx@lists.freedesktop.org > Cc: Peres, Martin ; Saarinen, Jani > ; Kulkarni, Vandita > Subject: RE: [PATCH v11 00/23] drm/i915/icl: dsi enabling > > Ok, I didn't file a bug yet, because I still have suspicion that this could > be a > bios thing. > > Vandita, Madhav, did you happen to see same issue? > > Best Regards, > > Lisovskiy Stanislav > > Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 > Espoo > > > From: Nikula, Jani > Sent: Wednesday, December 05, 2018 10:35 AM > To: Lisovskiy, Stanislav; Chauhan, Madhav; intel-gfx@lists.freedesktop.org > Cc: Peres, Martin; Saarinen, Jani > Subject: RE: [PATCH v11 00/23] drm/i915/icl: dsi enabling > > On Wed, 05 Dec 2018, "Lisovskiy, Stanislav" > wrote: > > I still see this pipe config mismatch(with icl-dsi-2018-12-03(4.20.0-rc5) + > > "fix > transcoder state readout" commit applied): > > > > [ 12.773332] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_hdisplay (expected 1440, found 720) > > [ 12.773425] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_htotal (expected 1586, found 793) > > [ 12.773510] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_hblank_start (expected 1440, found 1) > > [ 12.773588] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_hblank_end (expected 1586, found 1) > > [ 12.773663] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_hsync_start (expected 1540, found 770) > > [ 12.773735] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_hsync_end (expected 1550, found 775) > > [ 12.773813] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_vblank_start (expected 2560, found 1) > > [ 12.773897] [drm:pipe_config_err [i915]] *ERROR* mismatch in > base.adjusted_mode.crtc_vblank_end (expected 2582, found 1) > > [ 12.773976] [drm:pipe_config_err [i915]] *ERROR* mismatch in > output_format (expected 0, found 1) > > [ 12.774039] [drm:pipe_config_err [i915]] *ERROR*
[Intel-gfx] [PATCH 3/3] drm/i915: Pipeline PDP updates for Braswell
Currently we face a severe problem on Braswell that manifests as invalid
ppGTT accesses. The code tries to maintain the PDP (page directory
pointers) inside the context in two ways, direct write into the context
and a pipelined LRI update. The direct write into the context is
fundamentally racy as it is unserialised with any access (read or write)
the GPU is doing. By asserting that Braswell is not used with vGPU
(currently an unsupported platform) we can eliminate the dangerous
direct write into the context image and solely use the pipelined update.
However, the LRI of the PDP fouls up the GPU, causing it to freeze and
take out the machine with "forcewake ack timeouts". This seems possible
to workaround by preventing the GPU from sleeping (via means of
disabling the power-state management interface, i.e. forcing each ring
to remain awake) around the update.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108656
References: https://bugs.freedesktop.org/show_bug.cgi?id=108714
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 -
drivers/gpu/drm/i915/intel_lrc.c| 140 ++--
2 files changed, 69 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index add1fe7aeb93..62bde517d383 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1423,8 +1423,6 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space
*vm,
gen8_initialize_pd(vm, pd);
gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
GEM_BUG_ON(pdp->used_pdpes > i915_pdpes_per_pdp(vm));
-
- mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
}
ret = gen8_ppgtt_alloc_pd(vm, pd, start, length);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b1f5db3442eb..c84bdc21bcce 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -363,31 +363,12 @@ execlists_context_schedule_out(struct i915_request *rq,
unsigned long status)
trace_i915_request_out(rq);
}
-static void
-execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state)
-{
- ASSIGN_CTX_PDP(ppgtt, reg_state, 3);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 2);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 1);
- ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
-}
-
static u64 execlists_update_context(struct i915_request *rq)
{
- struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt;
struct intel_context *ce = rq->hw_context;
- u32 *reg_state = ce->lrc_reg_state;
- reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
-
- /*
-* True 32b PPGTT with dynamic page allocation: update PDP
-* registers and point the unallocated PDPs to scratch page.
-* PML4 is allocated during ppgtt init, so this is not needed
-* in 48-bit mode.
-*/
- if (!i915_vm_is_48bit(>vm))
- execlists_update_context_pdps(ppgtt, reg_state);
+ ce->lrc_reg_state[CTX_RING_TAIL + 1] =
+ intel_ring_set_tail(rq->ring, rq->tail);
/*
* Make sure the context image is complete before we submit it to HW.
@@ -1247,6 +1228,59 @@ execlists_context_pin(struct intel_engine_cs *engine,
return __execlists_context_pin(engine, ctx, ce);
}
+static int emit_pdps(struct i915_request *rq)
+{
+ const struct intel_engine_cs * const engine = rq->engine;
+ struct i915_hw_ppgtt * const ppgtt = rq->gem_context->ppgtt;
+ int err, i;
+ u32 *cs;
+
+ GEM_BUG_ON(intel_vgpu_active(rq->i915));
+
+ /*
+* Beware ye of the dragons, this sequence is magic!
+*
+* Small changes to this sequence can cause anything from
+* GPU hangs to forcewake errors and machine lockups!
+*/
+
+ /* Flush any residual operations from the context load */
+ err = engine->emit_flush(rq, EMIT_FLUSH);
+ if (err)
+ return err;
+
+ /* Magic required to prevent forcewake errors! */
+ err = engine->emit_flush(rq, EMIT_INVALIDATE);
+ if (err)
+ return err;
+
+ cs = intel_ring_begin(rq, 4 * GEN8_3LVL_PDPES + 2);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+
+ /* Ensure the LRI have landed before we invalidate & continue */
+ *cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED;
+ for (i = GEN8_3LVL_PDPES; i--; ) {
+ const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
+
+ *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(engine, i));
+ *cs++ = upper_32_bits(pd_daddr);
+ *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(engine, i));
+ *cs++ = lower_32_bits(pd_daddr);
+ }
+ *cs++ = MI_NOOP;
+
+
[Intel-gfx] [PATCH 2/3] drm/i915/ringbuffer: EMIT_INVALIDATE after switch context
The recommend procedure was to switch contexts (and mm) then invalidate the TLBs. Make it so. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 16084749adf5..74a4d587c312 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1827,12 +1827,12 @@ static int ring_request_alloc(struct i915_request *request) */ request->reserved_space += LEGACY_REQUEST_SIZE; - /* Unconditionally invalidate GPU caches and TLBs. */ - ret = request->engine->emit_flush(request, EMIT_INVALIDATE); + ret = switch_context(request); if (ret) return ret; - ret = switch_context(request); + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = request->engine->emit_flush(request, EMIT_INVALIDATE); if (ret) return ret; -- 2.20.0.rc2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Push EMIT_INVALIDATE at request start to backends
Move the common engine->emit_flush(EMIT_INVALIDATE) back to the backends (where it was once previously) as we seek to specialise it in future patches. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_request.c | 5 - drivers/gpu/drm/i915/intel_lrc.c| 9 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 -- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index ca95ab2f4cfa..8ab8e8e6a086 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -719,11 +719,6 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) */ rq->head = rq->ring->emit; - /* Unconditionally invalidate GPU caches and TLBs. */ - ret = engine->emit_flush(rq, EMIT_INVALIDATE); - if (ret) - goto err_unwind; - ret = engine->request_alloc(rq); if (ret) goto err_unwind; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 27d3a780611a..b1f5db3442eb 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1253,17 +1253,20 @@ static int execlists_request_alloc(struct i915_request *request) GEM_BUG_ON(!request->hw_context->pin_count); - /* Flush enough space to reduce the likelihood of waiting after + /* +* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just * have to repeat work. */ request->reserved_space += EXECLISTS_REQUEST_SIZE; - ret = intel_ring_wait_for_space(request->ring, request->reserved_space); + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = request->engine->emit_flush(request, EMIT_INVALIDATE); if (ret) return ret; - /* Note that after this point, we have committed to using + /* +* Note that after this point, we have committed to using * this request as it is being used to both track the * state of engine initialisation and liveness of the * golden renderstate above. Think twice before you try diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c5eb26a7ee79..16084749adf5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1820,13 +1820,15 @@ static int ring_request_alloc(struct i915_request *request) GEM_BUG_ON(!request->hw_context->pin_count); - /* Flush enough space to reduce the likelihood of waiting after + /* +* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just * have to repeat work. */ request->reserved_space += LEGACY_REQUEST_SIZE; - ret = intel_ring_wait_for_space(request->ring, request->reserved_space); + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = request->engine->emit_flush(request, EMIT_INVALIDATE); if (ret) return ret; -- 2.20.0.rc2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/icl: Fix pipe config mismatch warnings
Fixes hblank, vblank, vsync_start/vsync_end,
hsync_start//hsync_end, pipe_bpp, port clock,
pixel rate mismatches for dsi which
happen during pipe_config comparation in
intel_atomic_check.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/icl_dsi.c | 93 +-
1 file changed, 81 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 4dd793b78996..0fb6b140bc4b 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -760,13 +760,6 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
vsync_end = adjusted_mode->crtc_vsync_end;
vsync_shift = hsync_start - htotal / 2;
- if (intel_dsi->dual_link) {
- hactive /= 2;
- if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
- hactive += intel_dsi->pixel_overlap;
- htotal /= 2;
- }
-
/* minimum hactive as per bspec: 256 pixels */
if (adjusted_mode->crtc_hdisplay < 256)
DRM_ERROR("hactive is less then 256 pixels\n");
@@ -794,11 +787,6 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
if (hback_porch < 16)
DRM_ERROR("hback porch < 16 pixels\n");
- if (intel_dsi->dual_link) {
- hsync_start /= 2;
- hsync_end /= 2;
- }
-
for_each_dsi_port(port, intel_dsi->ports) {
dsi_trans = dsi_port_to_transcoder(port);
I915_WRITE(HSYNC(dsi_trans),
@@ -844,6 +832,64 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
}
}
+static
+void
+gen11_dsi_calc_transcoder_timings(struct intel_encoder *encoder,
+const struct intel_crtc_state *pipe_config)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
+ struct drm_display_mode *adjusted_mode =
+ _config->base.adjusted_mode;
+ enum port port;
+ enum transcoder dsi_trans;
+ /* horizontal timings */
+ u16 htotal, hactive, hsync_start, hsync_end, hsync_size;
+ u16 hfront_porch, hback_porch;
+ /* vertical timings */
+ u16 vtotal, vactive, vsync_start, vsync_end, vsync_shift;
+
+ hactive = adjusted_mode->crtc_hdisplay;
+ htotal = adjusted_mode->crtc_htotal;
+ hsync_start = adjusted_mode->crtc_hsync_start;
+ hsync_end = adjusted_mode->crtc_hsync_end;
+ hsync_size = hsync_end - hsync_start;
+ hfront_porch = (adjusted_mode->crtc_hsync_start -
+ adjusted_mode->crtc_hdisplay);
+ hback_porch = (adjusted_mode->crtc_htotal -
+ adjusted_mode->crtc_hsync_end);
+ vactive = adjusted_mode->crtc_vdisplay;
+ vtotal = adjusted_mode->crtc_vtotal;
+ vsync_start = adjusted_mode->crtc_vsync_start;
+ vsync_end = adjusted_mode->crtc_vsync_end;
+ vsync_shift = hsync_start - htotal / 2;
+
+ if (intel_dsi->dual_link) {
+ hactive /= 2;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
+ hactive += intel_dsi->pixel_overlap;
+ htotal /= 2;
+ }
+
+ /* TRANS_HSYNC register to be programmed only for video mode */
+ if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
+
+ if (intel_dsi->dual_link) {
+ hsync_start /= 2;
+ hsync_end /= 2;
+ }
+ }
+
+ adjusted_mode->crtc_hdisplay = hactive;
+ adjusted_mode->crtc_htotal = htotal;
+ adjusted_mode->crtc_hsync_start = hsync_start;
+ adjusted_mode->crtc_hsync_end = hsync_end;
+ adjusted_mode->crtc_vdisplay = vactive;
+ adjusted_mode->crtc_vtotal = vtotal;
+ adjusted_mode->crtc_vsync_start = vsync_start;
+ adjusted_mode->crtc_vsync_end = vsync_end;
+}
+
static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -1169,6 +1215,9 @@ static void gen11_dsi_get_config(struct intel_encoder
*encoder,
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
+ struct drm_display_mode *adjusted_mode =
+ _config->base.adjusted_mode;
+ u32 bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
u32 pll_id;
/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
@@ -1176,6 +1225,19 @@ static void gen11_dsi_get_config(struct intel_encoder
*encoder,
pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
[Intel-gfx] [RFT i-g-t 1/2] tests/gem_shrink: Background, direct and OOM shrinker plus userptr tests
From: Tvrtko Ursulin
...
Signed-off-by: Tvrtko Ursulin
---
tests/i915/gem_shrink.c | 213
1 file changed, 213 insertions(+)
diff --git a/tests/i915/gem_shrink.c b/tests/i915/gem_shrink.c
index c8e05814ee70..acc12efed15e 100644
--- a/tests/i915/gem_shrink.c
+++ b/tests/i915/gem_shrink.c
@@ -26,6 +26,9 @@
*
* Exercise the shrinker by overallocating GEM objects
*/
+#include
+#include
+#include
#include "igt.h"
#include "igt_gt.h"
@@ -366,6 +369,210 @@ static void reclaim(unsigned engine, int timeout)
close(fd);
}
+static unsigned long get_meminfo(const char *info, const char *tag)
+{
+ const char *str;
+ unsigned long val;
+
+ str = strstr(info, tag);
+ if (str && sscanf(str + strlen(tag), " %lu", ) == 1)
+ return val >> 10;
+
+ igt_warn("Unrecognised /proc/meminfo field: '%s'\n", tag);
+ return 0;
+}
+
+static unsigned long get_avail_ram_mb(void)
+{
+ int fd;
+ int ret;
+ char buf[4096];
+ unsigned long ram;
+
+ fd = open("/proc/meminfo", O_RDONLY);
+ igt_assert_fd(fd);
+
+ ret = read(fd, buf, sizeof(buf));
+ igt_assert(ret >= 0);
+
+ close(fd);
+
+ ram = get_meminfo(buf, "MemAvailable:");
+ ram += get_meminfo(buf, "Buffers:");
+ ram += get_meminfo(buf, "Cached:");
+ ram += get_meminfo(buf, "SwapCached:");
+
+ return ram;
+}
+
+struct test {
+#define TEST_BO(1)
+#define TEST_USERPTR (2)
+ unsigned int flags;
+ int fd;
+};
+
+static uint32_t __get_pages(int fd, unsigned long alloc)
+{
+ uint32_t handle = gem_create(fd, alloc);
+
+ gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, 0);
+ gem_madvise(fd, handle, I915_MADV_DONTNEED);
+
+ return handle;
+}
+
+struct test_obj {
+ void *ptr;
+ uint32_t handle;
+};
+
+static void
+__get_userptr(int fd, struct test_obj *obj, unsigned long sz)
+{
+ struct local_i915_gem_userptr userptr = { };
+ void *ptr;
+
+ igt_assert_eq(sz & 4095, 0);
+
+ ptr = mmap(NULL, sz, PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ assert(ptr != MAP_FAILED);
+
+ userptr.user_size = sz;
+ userptr.user_ptr = to_user_pointer(ptr);
+ do_ioctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, );
+
+ gem_set_domain(fd, userptr.handle, I915_GEM_DOMAIN_GTT, 0);
+
+ obj->ptr = ptr;
+ obj->handle = userptr.handle;
+}
+
+#define PAGE_SIZE 4096
+static void *mempressure(void *arg)
+{
+ struct test_obj *list = NULL;
+ struct test *test = arg;
+ const unsigned int sz_mb = 2;
+ const unsigned int sz = sz_mb << 20;
+ unsigned int n = 0, max = 0;
+ unsigned int blocks;
+
+ while (true) {
+ unsigned long ram_mb = get_avail_ram_mb();
+
+ if (!list) {
+ blocks = ram_mb / sz_mb;
+ list = calloc(blocks, sizeof(*list));
+ igt_assert(list);
+ } else if (ram_mb < 256) {
+ blocks = max + 1;
+ }
+
+ if (list[n].ptr || list[n].handle) {
+ if (test->flags & TEST_BO)
+ gem_close(test->fd, list[n].handle);
+ else
+ munmap(list[n].ptr, sz);
+ }
+
+ if (test->flags & TEST_BO) {
+ list[n].handle = __get_pages(test->fd, sz);
+ } else if (test->flags & TEST_USERPTR) {
+ __get_userptr(test->fd, [n], sz);
+ } else {
+ list[n].ptr = mmap(NULL, sz, PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ assert(list[n].ptr != MAP_FAILED);
+
+ madvise(list[n].ptr, sz, MADV_HUGEPAGE);
+
+ for (size_t page = 0; page < sz; page += PAGE_SIZE)
+ *(volatile uint32_t *)((unsigned char
*)list[n].ptr + page) = 0;
+ }
+
+ if (n > max)
+ max = n;
+
+ n++;
+
+ if (n >= blocks)
+ n = 0;
+ }
+
+ return NULL;
+}
+
+static void oom_adjust(const char *score)
+{
+int fd;
+
+fd = open("/proc/self/oom_score_adj", O_WRONLY);
+igt_assert_fd(fd);
+igt_assert(write(fd, score, sizeof(score)) == sizeof(score));
+close(fd);
+}
+
+static void trigger_oom(void)
+{
+ const char *cmd = "f";
+int fd;
+
+fd = open("/proc/sysrq-trigger", O_WRONLY);
+igt_assert_fd(fd);
+igt_assert(write(fd, cmd, sizeof(cmd)) == sizeof(cmd));
+close(fd);
+}
+
+static void reclaim_oom(unsigned int flags)
+{
+ unsigned int count = 0;
+
+ igt_assert_eq(__builtin_popcount(flags), 1);
+
+ oom_adjust("-1000");
+
[Intel-gfx] [RFT i-g-t 2/2] intel-ci: Unblack list the new tests??
From: Tvrtko Ursulin Signed-off-by: Tvrtko Ursulin --- tests/intel-ci/blacklist.txt | 1 + tests/intel-ci/fast-feedback.testlist | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt index 73d127603d28..8083efc407d4 100644 --- a/tests/intel-ci/blacklist.txt +++ b/tests/intel-ci/blacklist.txt @@ -60,6 +60,7 @@ igt@gem_ring_sync_copy(@.*)? igt@gem_ring_sync_loop(@.*)? igt@gem_seqno_wrap(@.*)? igt@gem_shrink@(?!reclaim$).* +igt@gem_shrink@(?!two-reclaims-and-oom).* igt@gem_softpin@.*(hang|S4).* igt@gem_spin_batch(@.*)? igt@gem_stolen@.*hibernate.* diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist index 6d42792c67f7..77b5ae15a64a 100644 --- a/tests/intel-ci/fast-feedback.testlist +++ b/tests/intel-ci/fast-feedback.testlist @@ -124,6 +124,8 @@ igt@gem_ringfill@basic-default igt@gem_ringfill@basic-default-interruptible igt@gem_ringfill@basic-default-forked igt@gem_ringfill@basic-default-fd +igt@gem_shrink@two-reclaims-and-oom +igt@gem_shrink@two-reclaims-and-oom-userptr igt@gem_sync@basic-all igt@gem_sync@basic-each igt@gem_sync@basic-many-each -- 2.19.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm tip conflict in intel_workarounds.c
On 06/12/2018 19:36, Rodrigo Vivi wrote: On Thu, Dec 06, 2018 at 12:25:48PM +1000, Dave Airlie wrote: I merged the i915 tree into drm-next this morning, but got a major conflict on the drm-tip rebuild in intel_workarounds.c. I'm not sure if I did wrong thing, but there were a couple of places where the code seemed inconsistent and I couldn't track down what was actually wanted. I'd appreciate someone checking my work on what is in current drm-tip, what is currently on drm-tip looks sane to me. Yes I've fixed it up yesterday with Daniel's help. Regards, Tvrtko /* Wa_1604302699:icl */ + wa_write_or(wal, + GEN10_L3_CHICKEN_MODE_REGISTER, + GEN11_I2M_WRITE_DISABLE); and - - /* Wa_1406609255:icl (pre-prod) */ - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0)) - wa_write_or(wal, - GEN7_SARCHKMD, - GEN7_DISABLE_DEMAND_PREFETCH | - GEN7_DISABLE_SAMPLER_PREFETCH); were the two problems I was seeing. Dave. ___ 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 5/7] drm/i915: Return immediately if trylock fails for direct-reclaim
Quoting Chris Wilson (2018-12-06 21:30:25)
> Quoting Tvrtko Ursulin (2018-12-06 15:18:13)
> >
> > On 04/12/2018 14:15, Chris Wilson wrote:
> > > Ignore trying to shrink from i915 if we fail to acquire the struct_mutex
> > > in the shrinker while performing direct-reclaim. The trade-off being
> > > (much) lower latency for non-i915 clients at an increased risk of being
> > > unable to obtain a page from direct-reclaim without hitting the
> > > oom-notifier. The proviso being that we still keep trying to hard
> > > obtain the lock for oom so that we can reap under heavy memory pressure.
> > >
> > > Signed-off-by: Chris Wilson
> > > Cc: Tvrtko Ursulin
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 4 ++--
> > > drivers/gpu/drm/i915/i915_gem_shrinker.c | 24 +++-
> > > 2 files changed, 13 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index c5f01964f0fb..1cad218b71d3 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2916,9 +2916,9 @@ i915_gem_object_unpin_pages(struct
> > > drm_i915_gem_object *obj)
> > > __i915_gem_object_unpin_pages(obj);
> > > }
> > >
> > > -enum i915_mm_subclass { /* lockdep subclass for obj->mm.lock */
> > > +enum i915_mm_subclass { /* lockdep subclass for
> > > obj->mm.lock/struct_mutex */
> > > I915_MM_NORMAL = 0,
> > > - I915_MM_SHRINKER
> > > + I915_MM_SHRINKER /* called "recursively" from direct-reclaim-esque
> > > */
> > > };
> > >
> > > void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > > b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > > index ea90d3a0d511..d461f458f4af 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > > @@ -36,7 +36,9 @@
> > > #include "i915_drv.h"
> > > #include "i915_trace.h"
> > >
> > > -static bool shrinker_lock(struct drm_i915_private *i915, bool *unlock)
> > > +static bool shrinker_lock(struct drm_i915_private *i915,
> > > + unsigned int flags,
> > > + bool *unlock)
> > > {
> > > switch (mutex_trylock_recursive(>drm.struct_mutex)) {
> > > case MUTEX_TRYLOCK_RECURSIVE:
> > > @@ -45,15 +47,11 @@ static bool shrinker_lock(struct drm_i915_private
> > > *i915, bool *unlock)
> > >
> > > case MUTEX_TRYLOCK_FAILED:
> > > *unlock = false;
> > > - preempt_disable();
> > > - do {
> > > - cpu_relax();
> > > - if (mutex_trylock(>drm.struct_mutex)) {
> > > - *unlock = true;
> > > - break;
> > > - }
> > > - } while (!need_resched());
> > > - preempt_enable();
> > > + if (flags & I915_SHRINK_ACTIVE) {
> > > + mutex_lock_nested(>drm.struct_mutex,
> > > + I915_MM_SHRINKER);
> > > + *unlock = true;
> > > + }
> >
> > I just realized once oddity in the shrinker code which escaped me
> > before. It is the fact the call paths will call the shrinker_lock twice.
> > For instance i915_gem_shrinker_vmap and i915_gem_shrinker_scan. They
> > both first take lock with flags of zero, and then they call
> > i915_gem_shrink which takes the lock again, which obviously always
> > results in the recursive path to be taken.
> >
> > I think we need to clean this up so it is easier to understand the code
> > before further tweaking, even if in this patch. For instance adding
> > I915_SHRINK_LOCKED would solve it.
> >
> > shrinker_lock_uninterruptible is also funky in that it doesn't respect
> > the timeout in the waiting for idle phase.
> >
> > Sounds reasonable?
>
> My alternate code for this avoids struct_mutex here, but the compromise
> is that we can't process active requests here, and can't reap pages from
> zombie objects (objects that are still waiting for the RCU release).
As far as what the current patch is describing, I still like it. It
basically says if we get to this point and we need to wait and freeze the
batch queue but haven't actually committed ourselves to that, don't.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 2/3] drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
The parameter 'void *gvt' is not used and required for hypervisor's
exit call. Even for non-merged Xen hypervisor support. So just remove it.
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/gvt/gvt.c | 2 +-
drivers/gpu/drm/i915/gvt/hypercall.h | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
drivers/gpu/drm/i915/gvt/mpt.h | 5 ++---
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 733a2a0d0c30..a5b760b7bc10 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -316,7 +316,7 @@ void intel_gvt_clean_device(struct drm_i915_private
*dev_priv)
return;
intel_gvt_destroy_idle_vgpu(gvt->idle_vgpu);
- intel_gvt_hypervisor_host_exit(_priv->drm.pdev->dev, gvt);
+ intel_gvt_hypervisor_host_exit(_priv->drm.pdev->dev);
intel_gvt_cleanup_vgpu_type_groups(gvt);
intel_gvt_clean_vgpu_types(gvt);
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h
b/drivers/gpu/drm/i915/gvt/hypercall.h
index 5af11cf1b482..e49a9247ed78 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -39,7 +39,7 @@
*/
struct intel_gvt_mpt {
int (*host_init)(struct device *dev, void *gvt, const void *ops);
- void (*host_exit)(struct device *dev, void *gvt);
+ void (*host_exit)(struct device *dev);
int (*attach_vgpu)(void *vgpu, unsigned long *handle);
void (*detach_vgpu)(unsigned long handle);
int (*inject_msi)(unsigned long handle, u32 addr, u16 data);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index c1072143da1d..1bbd04d30c42 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1467,7 +1467,7 @@ static int kvmgt_host_init(struct device *dev, void *gvt,
const void *ops)
return mdev_register_device(dev, _vgpu_ops);
}
-static void kvmgt_host_exit(struct device *dev, void *gvt)
+static void kvmgt_host_exit(struct device *dev)
{
mdev_unregister_device(dev);
}
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index ce721099a020..c95ef77da62c 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -61,14 +61,13 @@ static inline int intel_gvt_hypervisor_host_init(struct
device *dev,
/**
* intel_gvt_hypervisor_host_exit - exit GVT-g host side
*/
-static inline void intel_gvt_hypervisor_host_exit(struct device *dev,
- void *gvt)
+static inline void intel_gvt_hypervisor_host_exit(struct device *dev)
{
/* optional to provide */
if (!intel_gvt_host.mpt->host_exit)
return;
- intel_gvt_host.mpt->host_exit(dev, gvt);
+ intel_gvt_host.mpt->host_exit(dev);
}
/**
--
2.19.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 1/3] drm/i915/gvt: mandatory require hypervisor's host_init
Don't mark hypervisor module's host_init as optional,
but mandatory required.
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/gvt/mpt.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index 67f19992b226..ce721099a020 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -50,11 +50,10 @@
* Zero on success, negative error code if failed
*/
static inline int intel_gvt_hypervisor_host_init(struct device *dev,
- void *gvt, const void *ops)
+void *gvt, const void *ops)
{
- /* optional to provide */
if (!intel_gvt_host.mpt->host_init)
- return 0;
+ return -ENODEV;
return intel_gvt_host.mpt->host_init(dev, gvt, ops);
}
--
2.19.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 3/3] drm/i915/gvt: Change KVMGT as self load module
This trys to make 'kvmgt' module as self loadable instead of loading
by i915/gvt device model. So hypervisor specific module could be
stand-alone, e.g only after loading hypervisor specific module, GVT
feature could be enabled via specific hypervisor interface, e.g VFIO/mdev.
So this trys to use hypervisor module register/unregister interface
for that. Hypervisor module needs to take care of module reference
itself when working for hypervisor interface, e.g for VFIO/mdev,
hypervisor module would reference counting mdev when open and release.
This makes 'kvmgt' module really split from GVT device model. User
needs to load 'kvmgt' to enable VFIO/mdev interface.
v6:
- remove unused variable
v5:
- put module reference in register error path
v4:
- fix checkpatch warning
v3:
- Fix module reference handling for device open and release. Unused
mdev devices would be cleaned up in device unregister when module unload.
v2:
- Fix kvmgt order after i915 for built-in case
Cc: "Yuan, Hang"
Cc: Alex Williamson
Cc: "He, Min"
Reviewed-by: Yuan, Hang
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/gvt/Makefile| 1 -
drivers/gpu/drm/i915/gvt/gvt.c | 108 +++
drivers/gpu/drm/i915/gvt/gvt.h | 6 +-
drivers/gpu/drm/i915/gvt/hypercall.h | 7 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 16 +++-
drivers/gpu/drm/i915/gvt/mpt.h | 3 +
drivers/gpu/drm/i915/intel_gvt.c | 9 ---
8 files changed, 69 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0ff878c994e2..ae0d975a6f34 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -195,3 +195,4 @@ endif
i915-y += intel_lpe_audio.o
obj-$(CONFIG_DRM_I915) += i915.o
+obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
diff --git a/drivers/gpu/drm/i915/gvt/Makefile
b/drivers/gpu/drm/i915/gvt/Makefile
index b016dc753db9..271fb46d4dd0 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -7,4 +7,3 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o
trace_points.o firmware.o \
ccflags-y += -I$(src) -I$(src)/$(GVT_DIR)
i915-y += $(addprefix $(GVT_DIR)/,
$(GVT_SOURCE))
-obj-$(CONFIG_DRM_I915_GVT_KVMGT) += $(GVT_DIR)/kvmgt.o
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index a5b760b7bc10..4e8947f33bd0 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -187,52 +187,6 @@ static const struct intel_gvt_ops intel_gvt_ops = {
.write_protect_handler = intel_vgpu_page_track_handler,
};
-/**
- * intel_gvt_init_host - Load MPT modules and detect if we're running in host
- *
- * This function is called at the driver loading stage. If failed to find a
- * loadable MPT module or detect currently we're running in a VM, then GVT-g
- * will be disabled
- *
- * Returns:
- * Zero on success, negative error code if failed.
- *
- */
-int intel_gvt_init_host(void)
-{
- if (intel_gvt_host.initialized)
- return 0;
-
- /* Xen DOM U */
- if (xen_domain() && !xen_initial_domain())
- return -ENODEV;
-
- /* Try to load MPT modules for hypervisors */
- if (xen_initial_domain()) {
- /* In Xen dom0 */
- intel_gvt_host.mpt = try_then_request_module(
- symbol_get(xengt_mpt), "xengt");
- intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
- } else {
-#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
- /* not in Xen. Try KVMGT */
- intel_gvt_host.mpt = try_then_request_module(
- symbol_get(kvmgt_mpt), "kvmgt");
- intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
-#endif
- }
-
- /* Fail to load MPT modules - bail out */
- if (!intel_gvt_host.mpt)
- return -EINVAL;
-
- gvt_dbg_core("Running with hypervisor %s in host mode\n",
- supported_hypervisors[intel_gvt_host.hypervisor_type]);
-
- intel_gvt_host.initialized = true;
- return 0;
-}
-
static void init_device_info(struct intel_gvt *gvt)
{
struct intel_gvt_device_info *info = >device_info;
@@ -316,7 +270,6 @@ void intel_gvt_clean_device(struct drm_i915_private
*dev_priv)
return;
intel_gvt_destroy_idle_vgpu(gvt->idle_vgpu);
- intel_gvt_hypervisor_host_exit(_priv->drm.pdev->dev);
intel_gvt_cleanup_vgpu_type_groups(gvt);
intel_gvt_clean_vgpu_types(gvt);
@@ -352,13 +305,6 @@ int intel_gvt_init_device(struct drm_i915_private
*dev_priv)
struct intel_vgpu *vgpu;
int ret;
- /*
-* Cannot initialize GVT device without intel_gvt_host gets
-* initialized first.
-*/
- if
[Intel-gfx] [PATCH v6 0/3] Change KVMGT into self loadable module
This series try to change 'kvmgt' as self loadable module to enable GVT feature for VFIO/mdev interface when user loads 'kvmgt' module, instead of current method to load hypervisor module when i915/gvt device model initializes. Resend whole series for QA's jekins job. v6: - remove unused variable Zhenyu Wang (3): drm/i915/gvt: mandatory require hypervisor's host_init drm/i915/gvt: remove unused parameter for hypervisor's host_exit call drm/i915/gvt: Change KVMGT as self load module drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/gvt/Makefile| 1 - drivers/gpu/drm/i915/gvt/gvt.c | 108 +++ drivers/gpu/drm/i915/gvt/gvt.h | 6 +- drivers/gpu/drm/i915/gvt/hypercall.h | 9 ++- drivers/gpu/drm/i915/gvt/kvmgt.c | 18 - drivers/gpu/drm/i915/gvt/mpt.h | 13 ++-- drivers/gpu/drm/i915/intel_gvt.c | 9 --- 8 files changed, 75 insertions(+), 90 deletions(-) -- 2.19.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v2,1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
== Series Details ==
Series: series starting with [v2,1/3] ACPI / PMIC: Add support for executing
PMIC MIPI sequence elements
URL : https://patchwork.freedesktop.org/series/53662/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5275_full -> Patchwork_11035_full
Summary
---
**WARNING**
Minor unknown changes coming with Patchwork_11035_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11035_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_11035_full:
### IGT changes ###
Warnings
* igt@tools_test@tools_test:
- shard-glk: SKIP -> PASS
Known issues
Here are the changes found in Patchwork_11035_full that come from known
issues:
### IGT changes ###
Issues hit
* igt@gem_exec_schedule@pi-ringfull-vebox:
- shard-skl: NOTRUN -> FAIL [fdo#103158]
* igt@i915_suspend@fence-restore-tiled2untiled:
- {shard-iclb}: PASS -> INCOMPLETE [fdo#107713]
* igt@kms_available_modes_crc@available_mode_test_crc:
- shard-skl: NOTRUN -> FAIL [fdo#106641]
* igt@kms_color@pipe-b-degamma:
- shard-skl: PASS -> FAIL [fdo#104782]
* igt@kms_cursor_crc@cursor-256x256-dpms:
- shard-glk: PASS -> FAIL [fdo#103232]
* igt@kms_cursor_crc@cursor-256x85-random:
- {shard-iclb}: NOTRUN -> FAIL [fdo#103232]
* igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
- shard-hsw: PASS -> DMESG-WARN [fdo#102614]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
- shard-apl: PASS -> FAIL [fdo#103167]
- shard-glk: PASS -> FAIL [fdo#103167] +1
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
- {shard-iclb}: PASS -> FAIL [fdo#103167] +2
* igt@kms_panel_fitting@legacy:
- shard-skl: NOTRUN -> FAIL [fdo#105456]
* {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
- shard-apl: PASS -> FAIL [fdo#108948]
* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- {shard-iclb}: PASS -> FAIL [fdo#103166]
* igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
- shard-apl: PASS -> FAIL [fdo#103166]
* igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
- shard-glk: PASS -> FAIL [fdo#103166] +1
* igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
- {shard-iclb}: NOTRUN -> DMESG-WARN [fdo#107724]
* igt@kms_setmode@basic:
- shard-skl: NOTRUN -> FAIL [fdo#99912]
* igt@pm_rpm@gem-mmap-cpu:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#107807]
* igt@pm_rpm@gem-mmap-gtt:
- shard-skl: PASS -> INCOMPLETE [fdo#107807]
* igt@pm_rpm@legacy-planes:
- shard-skl: PASS -> INCOMPLETE [fdo#105959] / [fdo#107807]
Possible fixes
* igt@kms_busy@extended-modeset-hang-newfb-render-c:
- {shard-iclb}: DMESG-WARN [fdo#107724] / [fdo#107956] -> PASS
* igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
- shard-glk: FAIL [fdo#108145] -> PASS
* igt@kms_color@pipe-c-legacy-gamma:
- shard-apl: FAIL [fdo#104782] -> PASS
* igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
- shard-skl: FAIL [fdo#103184] -> PASS
* igt@kms_draw_crc@draw-method-xrgb-pwrite-untiled:
- shard-skl: FAIL [fdo#108472] -> PASS
* igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-render:
- shard-glk: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-pwrite:
- shard-skl: FAIL [fdo#103167] / [fdo#105682] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-indfb-fliptrack:
- {shard-iclb}: DMESG-FAIL [fdo#107724] -> PASS +2
* igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-render:
- shard-skl: FAIL [fdo#103167] -> PASS +2
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-gtt:
- {shard-iclb}: DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +4
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
- {shard-iclb}: FAIL [fdo#103167] -> PASS +4
* igt@kms_frontbuffer_tracking@psr-suspend:
- shard-skl: INCOMPLETE [fdo#104108] / [fdo#106978] -> PASS
* igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
- shard-apl: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
- shard-skl: FAIL [fdo#107815] / [fdo#108145] -> PASS
* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- shard-apl: FAIL [fdo#103166] -> PASS +1
* igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
-
Re: [Intel-gfx] [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
On 2018.12.07 07:47:59 +, He, Min wrote:
> Zhenyu,
> Overall I think the impact to AcrnGT is not big, we can modify our code to
> adapt
> to the new interfaces.
> But I have some comments embedded.
>
Good!
> > > @@ -467,6 +408,44 @@ int intel_gvt_init_device(struct drm_i915_private
> > *dev_priv)
> > > return ret;
> > > }
> > >
> > > -#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
> > > -MODULE_SOFTDEP("pre: kvmgt");
> > > -#endif
> > > +int
> > > +intel_gvt_register_hypervisor(struct intel_gvt_mpt *m)
> > > +{
> > > + int ret;
> > > + void *gvt;
> > > +
> > > + if (!intel_gvt_host.initialized)
> > > + return -ENODEV;
> > > +
> > > + if (m->type != INTEL_GVT_HYPERVISOR_KVM &&
> > > + m->type != INTEL_GVT_HYPERVISOR_XEN)
> > > + return -EINVAL;
> > > +
> > > + /* Get a reference for device model module */
> > > + if (!try_module_get(THIS_MODULE))
> > > + return -ENODEV;
> > > +
> > > + intel_gvt_host.mpt = m;
> > > + intel_gvt_host.hypervisor_type = m->type;
> > > + gvt = (void *)kdev_to_i915(intel_gvt_host.dev)->gvt;
> > > +
> > > + ret = intel_gvt_hypervisor_host_init(intel_gvt_host.dev, gvt,
> > > + _gvt_ops);
> I think we can remove the host_init and host_exit interfaces, since now
> it's mpt modules who proactively call the GVT-g to initialize and
> un-initialize,
> Thus we can return the intel_gvt_ops in intel_gvt_register_hypervisor() and
> moduels initialize its rest part.
> Current kvmgt_init-> intel_gvt_register_hypervisor->host_init seems a little
> bit
> redundant.
> But it's up to you to make the call to remove it in this patch or other
> furture
> optimization patches.
>
I'd like to keep as in current approach, as it can keep hypervisor init
function in one place instead of passing gvt host info to hypervisor module.
And calling submodule's init function is fine process I think.
> > > @@ -67,6 +73,5 @@ struct intel_gvt_mpt {
> > > };
> > >
> > > extern struct intel_gvt_mpt xengt_mpt;
> We can remove this definition, too.
>
yeah, but maybe in another xengt cleanup patch.
> > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > index 1bbd04d30c42..ef248d577e49 100644
> > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > @@ -50,6 +50,7 @@
> > > #include "gvt.h"
> > >
> > > static const struct intel_gvt_ops *intel_gvt_ops;
> > > +static struct intel_gvt *intel_gvt;
> This variable intel_gvt seems useless.
>
Thanks for pointing this out, was trying to use for exit path,
but looks I missed to remove it finally.
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
