* [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues
@ 2023-05-11 8:20 Luca Coelho
2023-05-11 8:20 ` [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private Luca Coelho
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Luca Coelho @ 2023-05-11 8:20 UTC (permalink / raw)
To: intel-gfx
Hi,
This series implements internal workqueues in the i915 driver in order
to avoid using the system queue. We add one generic workqueue in the
drm_i915_private structure, one specific for wake references and one
in a self-test.
This is based on Tetsuo's work[1] and is required to get rid of the
flush_scheduled_work() usage.
Please review.
[1] https://patchwork.freedesktop.org/series/114608/
Cheers,
Luca.
Luca Coelho (3):
drm/i915: add a dedicated workqueue inside drm_i915_private
drm/i915/gt: create workqueue dedicated to wake references
drm/i915/selftests: add local workqueue for SW fence selftest
drivers/gpu/drm/i915/display/intel_display.c | 5 ++--
.../drm/i915/display/intel_display_driver.c | 2 +-
drivers/gpu/drm/i915/display/intel_dmc.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
.../drm/i915/display/intel_dp_link_training.c | 5 ++--
drivers/gpu/drm/i915/display/intel_drrs.c | 4 ++-
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbdev.c | 3 ++-
drivers/gpu/drm/i915/display/intel_hdcp.c | 23 ++++++++++-------
drivers/gpu/drm/i915/display/intel_hotplug.c | 18 ++++++++-----
drivers/gpu/drm/i915/display/intel_opregion.c | 3 ++-
drivers/gpu/drm/i915/display/intel_pps.c | 4 ++-
drivers/gpu/drm/i915/display/intel_psr.c | 8 +++---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 +++++-
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 +++++++++--
drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
.../drm/i915/gt/intel_execlists_submission.c | 5 ++--
.../gpu/drm/i915/gt/intel_gt_buffer_pool.c | 10 +++++---
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_requests.c | 10 ++++----
drivers/gpu/drm/i915/gt/intel_reset.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c | 20 ++++++++-------
drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++-
drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 2 +-
drivers/gpu/drm/i915/i915_driver.c | 7 ++++++
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/i915_request.c | 2 +-
drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++----
drivers/gpu/drm/i915/intel_wakeref.h | 25 ++++++++++++-------
.../gpu/drm/i915/selftests/i915_sw_fence.c | 16 +++++++++---
30 files changed, 162 insertions(+), 77 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
@ 2023-05-11 8:20 ` Luca Coelho
2023-05-12 12:34 ` Tvrtko Ursulin
2023-05-11 8:20 ` [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references Luca Coelho
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Luca Coelho @ 2023-05-11 8:20 UTC (permalink / raw)
To: intel-gfx
In order to avoid flush_scheduled_work() usage, add a dedicated
workqueue in the drm_i915_private structure. In this way, we don't
need to use the system queue anymore.
This change is mostly mechanical and based on Tetsuo's original
patch[1].
Link: https://patchwork.freedesktop.org/series/114608/ [1]
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 5 ++--
.../drm/i915/display/intel_display_driver.c | 2 +-
drivers/gpu/drm/i915/display/intel_dmc.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
.../drm/i915/display/intel_dp_link_training.c | 5 ++--
drivers/gpu/drm/i915/display/intel_drrs.c | 4 +++-
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbdev.c | 3 ++-
drivers/gpu/drm/i915/display/intel_hdcp.c | 23 +++++++++++--------
drivers/gpu/drm/i915/display/intel_hotplug.c | 18 ++++++++++-----
drivers/gpu/drm/i915/display/intel_opregion.c | 3 ++-
drivers/gpu/drm/i915/display/intel_pps.c | 4 +++-
drivers/gpu/drm/i915/display/intel_psr.c | 8 ++++---
.../drm/i915/gt/intel_execlists_submission.c | 5 ++--
.../gpu/drm/i915/gt/intel_gt_buffer_pool.c | 10 ++++----
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_requests.c | 10 ++++----
drivers/gpu/drm/i915/gt/intel_reset.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c | 20 ++++++++--------
drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 2 +-
drivers/gpu/drm/i915/i915_driver.c | 7 ++++++
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/i915_request.c | 2 +-
23 files changed, 89 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1d5d42a40803..155b8e378f7e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7127,11 +7127,12 @@ intel_atomic_commit_ready(struct i915_sw_fence *fence,
break;
case FENCE_FREE:
{
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
struct intel_atomic_helper *helper =
- &to_i915(state->base.dev)->display.atomic_helper;
+ &i915->display.atomic_helper;
if (llist_add(&state->freed, &helper->free_list))
- schedule_work(&helper->free_work);
+ queue_work(i915->i915_wq, &helper->free_work);
break;
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 60ce10fc7205..bd61418fef15 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -435,7 +435,7 @@ void intel_display_driver_remove_noirq(struct drm_i915_private *i915)
intel_unregister_dsm_handler();
/* flush any delayed tasks or pending work */
- flush_scheduled_work();
+ flush_workqueue(i915->i915_wq);
intel_hdcp_component_fini(i915);
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 8a88de67ff0a..743c5b3e610d 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -1057,7 +1057,7 @@ void intel_dmc_init(struct drm_i915_private *i915)
i915->display.dmc.dmc = dmc;
drm_dbg_kms(&i915->drm, "Loading %s\n", dmc->fw_path);
- schedule_work(&dmc->work);
+ queue_work(i915->i915_wq, &dmc->work);
return;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0cc57681dc4d..02ce619ecd70 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5250,7 +5250,7 @@ static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
spin_lock_irq(&i915->irq_lock);
i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin);
spin_unlock_irq(&i915->irq_lock);
- queue_delayed_work(system_wq, &i915->display.hotplug.hotplug_work, 0);
+ queue_delayed_work(i915->i915_wq, &i915->display.hotplug.hotplug_work, 0);
}
static const struct drm_connector_funcs intel_dp_connector_funcs = {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index e92c62bcc9b8..f7eb431f03fa 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1130,9 +1130,10 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
{
struct intel_connector *intel_connector = intel_dp->attached_connector;
struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
if (intel_dp->hobl_active) {
- drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
+ drm_dbg_kms(&i915->drm,
"[ENCODER:%d:%s] Link Training failed with HOBL active, "
"not enabling it from now on",
encoder->base.base.id, encoder->base.name);
@@ -1144,7 +1145,7 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
}
/* Schedule a Hotplug Uevent to userspace to start modeset */
- schedule_work(&intel_connector->modeset_retry_work);
+ queue_work(i915->i915_wq, &intel_connector->modeset_retry_work);
}
/* Perform the link training on all LTTPRs and the DPRX on a link. */
diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
index 760e63cdc0c8..0939dbe453e0 100644
--- a/drivers/gpu/drm/i915/display/intel_drrs.c
+++ b/drivers/gpu/drm/i915/display/intel_drrs.c
@@ -111,7 +111,9 @@ static void intel_drrs_set_state(struct intel_crtc *crtc,
static void intel_drrs_schedule_work(struct intel_crtc *crtc)
{
- mod_delayed_work(system_wq, &crtc->drrs.work, msecs_to_jiffies(1000));
+ struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+
+ mod_delayed_work(i915->i915_wq, &crtc->drrs.work, msecs_to_jiffies(1000));
}
static unsigned int intel_drrs_frontbuffer_bits(const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 11bb8cf9c9d0..177d6e6bcad0 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1600,7 +1600,7 @@ static void __intel_fbc_handle_fifo_underrun_irq(struct intel_fbc *fbc)
if (READ_ONCE(fbc->underrun_detected))
return;
- schedule_work(&fbc->underrun_work);
+ queue_work(fbc->i915->i915_wq, &fbc->underrun_work);
}
/**
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index aab1ae74a8f7..af0df232b806 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -689,7 +689,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
/* Don't block our own workqueue as this can
* be run in parallel with other i915.ko tasks.
*/
- schedule_work(&dev_priv->display.fbdev.suspend_work);
+ queue_work(dev_priv->i915_wq,
+ &dev_priv->display.fbdev.suspend_work);
return;
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 650232c4892b..85a71609563f 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1019,6 +1019,7 @@ static void intel_hdcp_update_value(struct intel_connector *connector,
struct drm_device *dev = connector->base.dev;
struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
struct intel_hdcp *hdcp = &connector->hdcp;
+ struct drm_i915_private *i915 = to_i915(connector->base.dev);
drm_WARN_ON(connector->base.dev, !mutex_is_locked(&hdcp->mutex));
@@ -1037,7 +1038,7 @@ static void intel_hdcp_update_value(struct intel_connector *connector,
hdcp->value = value;
if (update_property) {
drm_connector_get(&connector->base);
- schedule_work(&hdcp->prop_work);
+ queue_work(i915->i915_wq, &hdcp->prop_work);
}
}
@@ -2132,16 +2133,17 @@ static void intel_hdcp_check_work(struct work_struct *work)
struct intel_hdcp,
check_work);
struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
+ struct drm_i915_private *i915 = to_i915(connector->base.dev);
if (drm_connector_is_unregistered(&connector->base))
return;
if (!intel_hdcp2_check_link(connector))
- schedule_delayed_work(&hdcp->check_work,
- DRM_HDCP2_CHECK_PERIOD_MS);
+ queue_delayed_work(i915->i915_wq, &hdcp->check_work,
+ DRM_HDCP2_CHECK_PERIOD_MS);
else if (!intel_hdcp_check_link(connector))
- schedule_delayed_work(&hdcp->check_work,
- DRM_HDCP_CHECK_PERIOD_MS);
+ queue_delayed_work(i915->i915_wq, &hdcp->check_work,
+ DRM_HDCP_CHECK_PERIOD_MS);
}
static int i915_hdcp_component_bind(struct device *i915_kdev,
@@ -2386,7 +2388,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
}
if (!ret) {
- schedule_delayed_work(&hdcp->check_work, check_link_interval);
+ queue_delayed_work(dev_priv->i915_wq, &hdcp->check_work,
+ check_link_interval);
intel_hdcp_update_value(connector,
DRM_MODE_CONTENT_PROTECTION_ENABLED,
true);
@@ -2435,6 +2438,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
to_intel_connector(conn_state->connector);
struct intel_hdcp *hdcp = &connector->hdcp;
bool content_protection_type_changed, desired_and_not_enabled = false;
+ struct drm_i915_private *i915 = to_i915(connector->base.dev);
if (!connector->hdcp.shim)
return;
@@ -2461,7 +2465,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
mutex_lock(&hdcp->mutex);
hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
drm_connector_get(&connector->base);
- schedule_work(&hdcp->prop_work);
+ queue_work(i915->i915_wq, &hdcp->prop_work);
mutex_unlock(&hdcp->mutex);
}
@@ -2478,7 +2482,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
*/
if (!desired_and_not_enabled && !content_protection_type_changed) {
drm_connector_get(&connector->base);
- schedule_work(&hdcp->prop_work);
+ queue_work(i915->i915_wq, &hdcp->prop_work);
}
}
@@ -2592,6 +2596,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
void intel_hdcp_handle_cp_irq(struct intel_connector *connector)
{
struct intel_hdcp *hdcp = &connector->hdcp;
+ struct drm_i915_private *i915 = to_i915(connector->base.dev);
if (!hdcp->shim)
return;
@@ -2599,5 +2604,5 @@ void intel_hdcp_handle_cp_irq(struct intel_connector *connector)
atomic_inc(&connector->hdcp.cp_irq_count);
wake_up_all(&connector->hdcp.cp_irq_queue);
- schedule_delayed_work(&hdcp->check_work, 0);
+ queue_delayed_work(i915->i915_wq, &hdcp->check_work, 0);
}
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index b12900446828..7a50c392e8b1 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -211,7 +211,8 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv)
/* Enable polling and queue hotplug re-enabling. */
if (hpd_disabled) {
drm_kms_helper_poll_enable(&dev_priv->drm);
- mod_delayed_work(system_wq, &dev_priv->display.hotplug.reenable_work,
+ mod_delayed_work(dev_priv->i915_wq,
+ &dev_priv->display.hotplug.reenable_work,
msecs_to_jiffies(HPD_STORM_REENABLE_DELAY));
}
}
@@ -338,7 +339,8 @@ static void i915_digport_work_func(struct work_struct *work)
spin_lock_irq(&dev_priv->irq_lock);
dev_priv->display.hotplug.event_bits |= old_bits;
spin_unlock_irq(&dev_priv->irq_lock);
- queue_delayed_work(system_wq, &dev_priv->display.hotplug.hotplug_work, 0);
+ queue_delayed_work(dev_priv->i915_wq,
+ &dev_priv->display.hotplug.hotplug_work, 0);
}
}
@@ -445,7 +447,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
dev_priv->display.hotplug.retry_bits |= retry;
spin_unlock_irq(&dev_priv->irq_lock);
- mod_delayed_work(system_wq, &dev_priv->display.hotplug.hotplug_work,
+ mod_delayed_work(dev_priv->i915_wq,
+ &dev_priv->display.hotplug.hotplug_work,
msecs_to_jiffies(HPD_RETRY_DELAY));
}
}
@@ -576,7 +579,8 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
if (queue_dig)
queue_work(dev_priv->display.hotplug.dp_wq, &dev_priv->display.hotplug.dig_port_work);
if (queue_hp)
- queue_delayed_work(system_wq, &dev_priv->display.hotplug.hotplug_work, 0);
+ queue_delayed_work(dev_priv->i915_wq,
+ &dev_priv->display.hotplug.hotplug_work, 0);
}
/**
@@ -686,7 +690,8 @@ void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
* As well, there's no issue if we race here since we always reschedule
* this worker anyway
*/
- schedule_work(&dev_priv->display.hotplug.poll_init_work);
+ queue_work(dev_priv->i915_wq,
+ &dev_priv->display.hotplug.poll_init_work);
}
/**
@@ -714,7 +719,8 @@ void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
return;
WRITE_ONCE(dev_priv->display.hotplug.poll_enabled, false);
- schedule_work(&dev_priv->display.hotplug.poll_init_work);
+ queue_work(dev_priv->i915_wq,
+ &dev_priv->display.hotplug.poll_init_work);
}
void intel_hpd_init_early(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
index b7973a05d022..e7af1935b4eb 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -635,7 +635,8 @@ static void asle_work(struct work_struct *work)
void intel_opregion_asle_intr(struct drm_i915_private *dev_priv)
{
if (dev_priv->display.opregion.asle)
- schedule_work(&dev_priv->display.opregion.asle_work);
+ queue_work(dev_priv->i915_wq,
+ &dev_priv->display.opregion.asle_work);
}
#define ACPI_EV_DISPLAY_SWITCH (1<<0)
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index 4f0b0cca03cc..63d1d879bd69 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -867,6 +867,7 @@ static void edp_panel_vdd_work(struct work_struct *__work)
static void edp_panel_vdd_schedule_off(struct intel_dp *intel_dp)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
unsigned long delay;
/*
@@ -882,7 +883,8 @@ static void edp_panel_vdd_schedule_off(struct intel_dp *intel_dp)
* operations.
*/
delay = msecs_to_jiffies(intel_dp->pps.panel_power_cycle_delay * 5);
- schedule_delayed_work(&intel_dp->pps.panel_vdd_work, delay);
+ queue_delayed_work(i915->i915_wq,
+ &intel_dp->pps.panel_vdd_work, delay);
}
/*
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index ea0389c5f656..ca2337e84159 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -341,7 +341,7 @@ void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir)
*/
intel_de_rmw(dev_priv, imr_reg, 0, psr_irq_psr_error_bit_get(intel_dp));
- schedule_work(&intel_dp->psr.work);
+ queue_work(dev_priv->i915_wq, &intel_dp->psr.work);
}
}
@@ -2440,6 +2440,8 @@ static void
tgl_dc3co_flush_locked(struct intel_dp *intel_dp, unsigned int frontbuffer_bits,
enum fb_op_origin origin)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
if (!intel_dp->psr.dc3co_exitline || !intel_dp->psr.psr2_enabled ||
!intel_dp->psr.active)
return;
@@ -2453,7 +2455,7 @@ tgl_dc3co_flush_locked(struct intel_dp *intel_dp, unsigned int frontbuffer_bits,
return;
tgl_psr2_enable_dc3co(intel_dp);
- mod_delayed_work(system_wq, &intel_dp->psr.dc3co_work,
+ mod_delayed_work(i915->i915_wq, &intel_dp->psr.dc3co_work,
intel_dp->psr.dc3co_exit_delay);
}
@@ -2493,7 +2495,7 @@ static void _psr_flush_handle(struct intel_dp *intel_dp)
psr_force_hw_tracking_exit(intel_dp);
if (!intel_dp->psr.active && !intel_dp->psr.busy_frontbuffer_bits)
- schedule_work(&intel_dp->psr.work);
+ queue_work(dev_priv->i915_wq, &intel_dp->psr.work);
}
}
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 750326434677..cf313296efd5 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -2327,6 +2327,7 @@ static u32 active_ccid(struct intel_engine_cs *engine)
static void execlists_capture(struct intel_engine_cs *engine)
{
+ struct drm_i915_private *i915 = engine->i915;
struct execlists_capture *cap;
if (!IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR))
@@ -2375,7 +2376,7 @@ static void execlists_capture(struct intel_engine_cs *engine)
goto err_rq;
INIT_WORK(&cap->work, execlists_capture_work);
- schedule_work(&cap->work);
+ queue_work(i915->i915_wq, &cap->work);
return;
err_rq:
@@ -3680,7 +3681,7 @@ static void virtual_context_destroy(struct kref *kref)
* lock, we can delegate the free of the engine to an RCU worker.
*/
INIT_RCU_WORK(&ve->rcu, rcu_virtual_context_destroy);
- queue_rcu_work(system_wq, &ve->rcu);
+ queue_rcu_work(ve->context.engine->i915->i915_wq, &ve->rcu);
}
static void virtual_engine_initial_hint(struct virtual_engine *ve)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c
index cadfd85785b1..a4ead9ccd223 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c
@@ -88,10 +88,11 @@ static void pool_free_work(struct work_struct *wrk)
{
struct intel_gt_buffer_pool *pool =
container_of(wrk, typeof(*pool), work.work);
+ struct intel_gt *gt = container_of(pool, struct intel_gt, buffer_pool);
if (pool_free_older_than(pool, HZ))
- schedule_delayed_work(&pool->work,
- round_jiffies_up_relative(HZ));
+ queue_delayed_work(gt->i915->i915_wq, &pool->work,
+ round_jiffies_up_relative(HZ));
}
static void pool_retire(struct i915_active *ref)
@@ -99,6 +100,7 @@ static void pool_retire(struct i915_active *ref)
struct intel_gt_buffer_pool_node *node =
container_of(ref, typeof(*node), active);
struct intel_gt_buffer_pool *pool = node->pool;
+ struct intel_gt *gt = container_of(pool, struct intel_gt, buffer_pool);
struct list_head *list = bucket_for_size(pool, node->obj->base.size);
unsigned long flags;
@@ -116,8 +118,8 @@ static void pool_retire(struct i915_active *ref)
WRITE_ONCE(node->age, jiffies ?: 1); /* 0 reserved for active nodes */
spin_unlock_irqrestore(&pool->lock, flags);
- schedule_delayed_work(&pool->work,
- round_jiffies_up_relative(HZ));
+ queue_delayed_work(gt->i915->i915_wq, &pool->work,
+ round_jiffies_up_relative(HZ));
}
void intel_gt_buffer_pool_mark_used(struct intel_gt_buffer_pool_node *node)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 95e59ed6651d..6916c86acbc1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -375,7 +375,7 @@ static void gen7_parity_error_irq_handler(struct intel_gt *gt, u32 iir)
if (iir & GT_RENDER_L3_PARITY_ERROR_INTERRUPT)
gt->i915->l3_parity.which_slice |= 1 << 0;
- schedule_work(>->i915->l3_parity.error_work);
+ queue_work(gt->i915->i915_wq, >->i915->l3_parity.error_work);
}
void gen6_gt_irq_handler(struct intel_gt *gt, u32 gt_iir)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index 1dfd01668c79..13f372143cc9 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -116,7 +116,7 @@ void intel_engine_add_retire(struct intel_engine_cs *engine,
GEM_BUG_ON(intel_engine_is_virtual(engine));
if (add_retire(engine, tl))
- schedule_work(&engine->retire_work);
+ queue_work(engine->i915->i915_wq, &engine->retire_work);
}
void intel_engine_init_retire(struct intel_engine_cs *engine)
@@ -207,8 +207,8 @@ static void retire_work_handler(struct work_struct *work)
struct intel_gt *gt =
container_of(work, typeof(*gt), requests.retire_work.work);
- schedule_delayed_work(>->requests.retire_work,
- round_jiffies_up_relative(HZ));
+ queue_delayed_work(gt->i915->i915_wq, >->requests.retire_work,
+ round_jiffies_up_relative(HZ));
intel_gt_retire_requests(gt);
}
@@ -224,8 +224,8 @@ void intel_gt_park_requests(struct intel_gt *gt)
void intel_gt_unpark_requests(struct intel_gt *gt)
{
- schedule_delayed_work(>->requests.retire_work,
- round_jiffies_up_relative(HZ));
+ queue_delayed_work(gt->i915->i915_wq, >->requests.retire_work,
+ round_jiffies_up_relative(HZ));
}
void intel_gt_fini_requests(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 195ff72d7a14..26ccb7ed1f91 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1625,7 +1625,7 @@ void __intel_init_wedge(struct intel_wedge_me *w,
w->name = name;
INIT_DELAYED_WORK_ONSTACK(&w->work, intel_wedge_me);
- schedule_delayed_work(&w->work, timeout);
+ queue_delayed_work(gt->i915->i915_wq, &w->work, timeout);
}
void __intel_fini_wedge(struct intel_wedge_me *w)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 80968e49e2c3..2dfbc43a71d0 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -72,13 +72,14 @@ static void set(struct intel_uncore *uncore, i915_reg_t reg, u32 val)
static void rps_timer(struct timer_list *t)
{
struct intel_rps *rps = from_timer(rps, t, timer);
+ struct intel_gt *gt = rps_to_gt(rps);
struct intel_engine_cs *engine;
ktime_t dt, last, timestamp;
enum intel_engine_id id;
s64 max_busy[3] = {};
timestamp = 0;
- for_each_engine(engine, rps_to_gt(rps), id) {
+ for_each_engine(engine, gt, id) {
s64 busy;
int i;
@@ -122,7 +123,7 @@ static void rps_timer(struct timer_list *t)
busy += div_u64(max_busy[i], 1 << i);
}
- GT_TRACE(rps_to_gt(rps),
+ GT_TRACE(gt,
"busy:%lld [%d%%], max:[%lld, %lld, %lld], interval:%d\n",
busy, (int)div64_u64(100 * busy, dt),
max_busy[0], max_busy[1], max_busy[2],
@@ -132,12 +133,12 @@ static void rps_timer(struct timer_list *t)
rps->cur_freq < rps->max_freq_softlimit) {
rps->pm_iir |= GEN6_PM_RP_UP_THRESHOLD;
rps->pm_interval = 1;
- schedule_work(&rps->work);
+ queue_work(gt->i915->i915_wq, &rps->work);
} else if (100 * busy < rps->power.down_threshold * dt &&
rps->cur_freq > rps->min_freq_softlimit) {
rps->pm_iir |= GEN6_PM_RP_DOWN_THRESHOLD;
rps->pm_interval = 1;
- schedule_work(&rps->work);
+ queue_work(gt->i915->i915_wq, &rps->work);
} else {
rps->last_adj = 0;
}
@@ -972,7 +973,7 @@ static int rps_set_boost_freq(struct intel_rps *rps, u32 val)
}
mutex_unlock(&rps->lock);
if (boost)
- schedule_work(&rps->work);
+ queue_work(rps_to_gt(rps)->i915->i915_wq, &rps->work);
return 0;
}
@@ -1024,7 +1025,8 @@ void intel_rps_boost(struct i915_request *rq)
if (!atomic_fetch_inc(&slpc->num_waiters)) {
GT_TRACE(rps_to_gt(rps), "boost fence:%llx:%llx\n",
rq->fence.context, rq->fence.seqno);
- schedule_work(&slpc->boost_work);
+ queue_work(rps_to_gt(rps)->i915->i915_wq,
+ &slpc->boost_work);
}
return;
@@ -1040,7 +1042,7 @@ void intel_rps_boost(struct i915_request *rq)
rq->fence.context, rq->fence.seqno);
if (READ_ONCE(rps->cur_freq) < rps->boost_freq)
- schedule_work(&rps->work);
+ queue_work(rps_to_gt(rps)->i915->i915_wq, &rps->work);
WRITE_ONCE(rps->boosts, rps->boosts + 1); /* debug only */
}
@@ -1899,7 +1901,7 @@ void gen11_rps_irq_handler(struct intel_rps *rps, u32 pm_iir)
gen6_gt_pm_mask_irq(gt, events);
rps->pm_iir |= events;
- schedule_work(&rps->work);
+ queue_work(gt->i915->i915_wq, &rps->work);
}
void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir)
@@ -1916,7 +1918,7 @@ void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir)
gen6_gt_pm_mask_irq(gt, events);
rps->pm_iir |= events;
- schedule_work(&rps->work);
+ queue_work(gt->i915->i915_wq, &rps->work);
spin_unlock(gt->irq_lock);
}
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
index 542ce6d2de19..a7627ccf13f1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
@@ -27,7 +27,7 @@ static void perf_begin(struct intel_gt *gt)
/* Boost gpufreq to max [waitboost] and keep it fixed */
atomic_inc(>->rps.num_waiters);
- schedule_work(>->rps.work);
+ queue_work(gt->i915->i915_wq, >->rps.work);
flush_work(>->rps.work);
}
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index fd198700272b..6ab5db790490 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -132,8 +132,14 @@ static int i915_workqueues_init(struct drm_i915_private *dev_priv)
if (dev_priv->display.hotplug.dp_wq == NULL)
goto out_free_wq;
+ dev_priv->i915_wq = alloc_workqueue("i915-generic", 0, 0);
+ if (dev_priv->i915_wq == NULL)
+ goto out_free_dp_wq;
+
return 0;
+out_free_dp_wq:
+ destroy_workqueue(dev_priv->i915_wq);
out_free_wq:
destroy_workqueue(dev_priv->wq);
out_err:
@@ -144,6 +150,7 @@ static int i915_workqueues_init(struct drm_i915_private *dev_priv)
static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
{
+ destroy_workqueue(dev_priv->i915_wq);
destroy_workqueue(dev_priv->display.hotplug.dp_wq);
destroy_workqueue(dev_priv->wq);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 14c5338c96a6..06cd956b03ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -259,6 +259,8 @@ struct drm_i915_private {
*/
struct workqueue_struct *wq;
+ struct workqueue_struct *i915_wq;
+
/* pm private clock gating functions */
const struct drm_i915_clock_gating_funcs *clock_gating_funcs;
@@ -930,5 +932,4 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_LMEMBAR_SMEM_STOLEN(i915) (!HAS_LMEM(i915) && \
GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
-
#endif
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 630a732aaecc..0f77c46cacab 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -290,7 +290,7 @@ static enum hrtimer_restart __rq_watchdog_expired(struct hrtimer *hrtimer)
if (!i915_request_completed(rq)) {
if (llist_add(&rq->watchdog.link, >->watchdog.list))
- schedule_work(>->watchdog.work);
+ queue_work(gt->i915->i915_wq, >->watchdog.work);
} else {
i915_request_put(rq);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
2023-05-11 8:20 ` [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private Luca Coelho
@ 2023-05-11 8:20 ` Luca Coelho
2023-05-12 9:04 ` Tvrtko Ursulin
2023-05-11 8:20 ` [Intel-gfx] [PATCH 3/3] drm/i915/selftests: add local workqueue for SW fence selftest Luca Coelho
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Luca Coelho @ 2023-05-11 8:20 UTC (permalink / raw)
To: intel-gfx
Add a work queue in the intel_wakeref structure to be used exclusively
by the wake reference mechanism. This is needed in order to avoid
using the system workqueue and relying on flush_scheduled_work().
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
6 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 0aff5bb13c53..6505bfa70cd0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
goto err_cmd_parser;
intel_engine_init_execlists(engine);
- intel_engine_init__pm(engine);
+
+ err = intel_engine_init__pm(engine);
+ if (err)
+ goto err_cmd_parser;
+
intel_engine_init_retire(engine);
/* Use the whole device by default */
@@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
{
GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
+ intel_engine_destroy__pm(engine);
i915_sched_engine_put(engine->sched_engine);
intel_breadcrumbs_put(engine->breadcrumbs);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index ee531a5c142c..859b62cf660f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
.put = __engine_park,
};
-void intel_engine_init__pm(struct intel_engine_cs *engine)
+int intel_engine_init__pm(struct intel_engine_cs *engine)
{
struct intel_runtime_pm *rpm = engine->uncore->rpm;
+ int err;
+
+ err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
+ if (err)
+ return err;
- intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
intel_engine_init_heartbeat(engine);
intel_gsc_idle_msg_enable(engine);
+
+ return 0;
+}
+
+void intel_engine_destroy__pm(struct intel_engine_cs *engine)
+{
+ intel_wakeref_destroy(&engine->wakeref);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
index d68675925b79..e8568f7d10c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
@@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
return rq;
}
-void intel_engine_init__pm(struct intel_engine_cs *engine);
+int intel_engine_init__pm(struct intel_engine_cs *engine);
+void intel_engine_destroy__pm(struct intel_engine_cs *engine);
void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index c0637bf799a3..0a3c702c21e2 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
intel_context_put(engine->kernel_context);
intel_engine_fini_retire(engine);
+ intel_engine_destroy__pm(engine);
}
struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
@@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
int mock_engine_init(struct intel_engine_cs *engine)
{
struct intel_context *ce;
+ int err;
INIT_LIST_HEAD(&engine->pinned_contexts_list);
@@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
engine->sched_engine->private_data = engine;
intel_engine_init_execlists(engine);
- intel_engine_init__pm(engine);
+
+ err = intel_engine_init__pm(engine);
+ if (err)
+ return err;
+
intel_engine_init_retire(engine);
engine->breadcrumbs = intel_breadcrumbs_create(NULL);
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
index dfd87d082218..6bae609e1312 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
/* Assume we are not in process context and so cannot sleep. */
if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
- mod_delayed_work(system_wq, &wf->work,
+ mod_delayed_work(wf->wq, &wf->work,
FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
return;
}
@@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
____intel_wakeref_put_last(wf);
}
-void __intel_wakeref_init(struct intel_wakeref *wf,
- struct intel_runtime_pm *rpm,
- const struct intel_wakeref_ops *ops,
- struct intel_wakeref_lockclass *key)
+int __intel_wakeref_init(struct intel_wakeref *wf,
+ struct intel_runtime_pm *rpm,
+ const struct intel_wakeref_ops *ops,
+ struct intel_wakeref_lockclass *key)
{
wf->rpm = rpm;
wf->ops = ops;
@@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
atomic_set(&wf->count, 0);
wf->wakeref = 0;
+ wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
+ if (wf->wq == NULL)
+ return -ENOMEM;
+
INIT_DELAYED_WORK(&wf->work, __intel_wakeref_put_work);
lockdep_init_map(&wf->work.work.lockdep_map,
"wakeref.work", &key->work, 0);
+
+ return 0;
+}
+
+void intel_wakeref_destroy(struct intel_wakeref *wf)
+{
+ destroy_workqueue(wf->wq);
}
int intel_wakeref_wait_for_idle(struct intel_wakeref *wf)
diff --git a/drivers/gpu/drm/i915/intel_wakeref.h b/drivers/gpu/drm/i915/intel_wakeref.h
index 0b6b4852ab23..86f99c044444 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.h
+++ b/drivers/gpu/drm/i915/intel_wakeref.h
@@ -42,6 +42,7 @@ struct intel_wakeref {
struct intel_runtime_pm *rpm;
const struct intel_wakeref_ops *ops;
+ struct workqueue_struct *wq;
struct delayed_work work;
};
@@ -50,15 +51,21 @@ struct intel_wakeref_lockclass {
struct lock_class_key work;
};
-void __intel_wakeref_init(struct intel_wakeref *wf,
- struct intel_runtime_pm *rpm,
- const struct intel_wakeref_ops *ops,
- struct intel_wakeref_lockclass *key);
-#define intel_wakeref_init(wf, rpm, ops) do { \
- static struct intel_wakeref_lockclass __key; \
- \
- __intel_wakeref_init((wf), (rpm), (ops), &__key); \
-} while (0)
+int __intel_wakeref_init(struct intel_wakeref *wf,
+ struct intel_runtime_pm *rpm,
+ const struct intel_wakeref_ops *ops,
+ struct intel_wakeref_lockclass *key);
+static inline int
+intel_wakeref_init(struct intel_wakeref *wf,
+ struct intel_runtime_pm *rpm,
+ const struct intel_wakeref_ops *ops)
+{
+ static struct intel_wakeref_lockclass __key;
+
+ return __intel_wakeref_init((wf), (rpm), (ops), &__key);
+}
+
+void intel_wakeref_destroy(struct intel_wakeref *wf);
int __intel_wakeref_get_first(struct intel_wakeref *wf);
void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags);
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 3/3] drm/i915/selftests: add local workqueue for SW fence selftest
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
2023-05-11 8:20 ` [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private Luca Coelho
2023-05-11 8:20 ` [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references Luca Coelho
@ 2023-05-11 8:20 ` Luca Coelho
2023-05-11 9:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement internal workqueues Patchwork
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Luca Coelho @ 2023-05-11 8:20 UTC (permalink / raw)
To: intel-gfx
Instead of using a global workqueue for the SW fence selftest,
allocate a separate one temporarily only while running the test.
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index daa985e5a19b..8f5ce71fa453 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -523,12 +523,19 @@ static void task_ipc(struct work_struct *work)
static int test_ipc(void *arg)
{
struct task_ipc ipc;
+ struct workqueue_struct *wq;
int ret = 0;
+ wq = alloc_workqueue("i1915-selftest", 0, 0);
+ if (wq == NULL)
+ return -ENOMEM;
+
/* Test use of i915_sw_fence as an interprocess signaling mechanism */
ipc.in = alloc_fence();
- if (!ipc.in)
- return -ENOMEM;
+ if (!ipc.in) {
+ ret = -ENOMEM;
+ goto err_work;
+ }
ipc.out = alloc_fence();
if (!ipc.out) {
ret = -ENOMEM;
@@ -540,7 +547,7 @@ static int test_ipc(void *arg)
ipc.value = 0;
INIT_WORK_ONSTACK(&ipc.work, task_ipc);
- schedule_work(&ipc.work);
+ queue_work(wq, &ipc.work);
wait_for_completion(&ipc.started);
@@ -563,6 +570,9 @@ static int test_ipc(void *arg)
free_fence(ipc.out);
err_in:
free_fence(ipc.in);
+err_work:
+ destroy_workqueue(wq);
+
return ret;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement internal workqueues
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
` (2 preceding siblings ...)
2023-05-11 8:20 ` [Intel-gfx] [PATCH 3/3] drm/i915/selftests: add local workqueue for SW fence selftest Luca Coelho
@ 2023-05-11 9:05 ` Patchwork
2023-05-11 9:05 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2023-05-11 9:05 UTC (permalink / raw)
To: Luca Coelho; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: implement internal workqueues
URL : https://patchwork.freedesktop.org/series/117618/
State : warning
== Summary ==
Error: dim checkpatch failed
501007ed8e45 drm/i915: add a dedicated workqueue inside drm_i915_private
-:622: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!dev_priv->i915_wq"
#622: FILE: drivers/gpu/drm/i915/i915_driver.c:136:
+ if (dev_priv->i915_wq == NULL)
total: 0 errors, 0 warnings, 1 checks, 501 lines checked
c4a59f545f7a drm/i915/gt: create workqueue dedicated to wake references
-:156: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!wf->wq"
#156: FILE: drivers/gpu/drm/i915/intel_wakeref.c:109:
+ if (wf->wq == NULL)
total: 0 errors, 0 warnings, 1 checks, 160 lines checked
0e6e672b91b6 drm/i915/selftests: add local workqueue for SW fence selftest
-:30: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!wq"
#30: FILE: drivers/gpu/drm/i915/selftests/i915_sw_fence.c:530:
+ if (wq == NULL)
total: 0 errors, 0 warnings, 1 checks, 38 lines checked
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: implement internal workqueues
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
` (3 preceding siblings ...)
2023-05-11 9:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement internal workqueues Patchwork
@ 2023-05-11 9:05 ` Patchwork
2023-05-11 9:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-05-11 10:55 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2023-05-11 9:05 UTC (permalink / raw)
To: Luca Coelho; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: implement internal workqueues
URL : https://patchwork.freedesktop.org/series/117618/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: implement internal workqueues
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
` (4 preceding siblings ...)
2023-05-11 9:05 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-05-11 9:19 ` Patchwork
2023-05-11 10:55 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2023-05-11 9:19 UTC (permalink / raw)
To: Luca Coelho; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 5026 bytes --]
== Series Details ==
Series: drm/i915: implement internal workqueues
URL : https://patchwork.freedesktop.org/series/117618/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13132 -> Patchwork_117618v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/index.html
Participating hosts (41 -> 40)
------------------------------
Missing (1): fi-snb-2520m
Known issues
------------
Here are the changes found in Patchwork_117618v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@reset:
- bat-rpls-1: [PASS][1] -> [ABORT][2] ([i915#4983] / [i915#7461] / [i915#7953] / [i915#8347] / [i915#8384])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/bat-rpls-1/igt@i915_selftest@live@reset.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/bat-rpls-1/igt@i915_selftest@live@reset.html
* igt@i915_selftest@live@slpc:
- bat-rpls-2: NOTRUN -> [DMESG-WARN][3] ([i915#6367])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/bat-rpls-2/igt@i915_selftest@live@slpc.html
* igt@i915_suspend@basic-s2idle-without-i915:
- bat-rpls-2: NOTRUN -> [ABORT][4] ([i915#6687])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html
#### Possible fixes ####
* igt@i915_module_load@reload:
- fi-kbl-soraka: [DMESG-WARN][5] ([i915#1982]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/fi-kbl-soraka/igt@i915_module_load@reload.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/fi-kbl-soraka/igt@i915_module_load@reload.html
* igt@i915_selftest@live@execlists:
- fi-kbl-soraka: [INCOMPLETE][7] ([i915#7156] / [i915#7913]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/fi-kbl-soraka/igt@i915_selftest@live@execlists.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/fi-kbl-soraka/igt@i915_selftest@live@execlists.html
* igt@i915_selftest@live@reset:
- bat-rpls-2: [ABORT][9] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#8347]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/bat-rpls-2/igt@i915_selftest@live@reset.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/bat-rpls-2/igt@i915_selftest@live@reset.html
* igt@i915_selftest@live@slpc:
- {bat-mtlp-6}: [DMESG-WARN][11] ([i915#6367] / [i915#7953]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/bat-mtlp-6/igt@i915_selftest@live@slpc.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/bat-mtlp-6/igt@i915_selftest@live@slpc.html
* igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1:
- bat-dg2-8: [FAIL][13] ([i915#7932]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
[i915#7156]: https://gitlab.freedesktop.org/drm/intel/issues/7156
[i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#7953]: https://gitlab.freedesktop.org/drm/intel/issues/7953
[i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
[i915#8384]: https://gitlab.freedesktop.org/drm/intel/issues/8384
Build changes
-------------
* Linux: CI_DRM_13132 -> Patchwork_117618v1
CI-20190529: 20190529
CI_DRM_13132: 4760cbc0f5aa85cd9a0ff94ecc58ea3f5e087ea7 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7285: d1cbf2bad9c2664ab8bd3bd0946510a52800912f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_117618v1: 4760cbc0f5aa85cd9a0ff94ecc58ea3f5e087ea7 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
d1f4b0a226dc drm/i915/selftests: add local workqueue for SW fence selftest
98688cdecd49 drm/i915/gt: create workqueue dedicated to wake references
52a7006c7e77 drm/i915: add a dedicated workqueue inside drm_i915_private
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/index.html
[-- Attachment #2: Type: text/html, Size: 6162 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: implement internal workqueues
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
` (5 preceding siblings ...)
2023-05-11 9:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-05-11 10:55 ` Patchwork
6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2023-05-11 10:55 UTC (permalink / raw)
To: Luca Coelho; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 14962 bytes --]
== Series Details ==
Series: drm/i915: implement internal workqueues
URL : https://patchwork.freedesktop.org/series/117618/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13132_full -> Patchwork_117618v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Known issues
------------
Here are the changes found in Patchwork_117618v1_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk: [PASS][1] -> [FAIL][2] ([i915#2842])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-glk3/igt@gem_exec_fair@basic-pace-share@rcs0.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-glk4/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_mmap_gtt@fault-concurrent-x:
- shard-snb: [PASS][3] -> [ABORT][4] ([i915#5161])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-snb1/igt@gem_mmap_gtt@fault-concurrent-x.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-snb7/igt@gem_mmap_gtt@fault-concurrent-x.html
* igt@gem_ppgtt@blt-vs-render-ctx0:
- shard-snb: [PASS][5] -> [FAIL][6] ([i915#8295])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-snb4/igt@gem_ppgtt@blt-vs-render-ctx0.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-snb1/igt@gem_ppgtt@blt-vs-render-ctx0.html
* igt@kms_color@ctm-0-25@pipe-b-hdmi-a-1:
- shard-snb: NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4579]) +8 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-snb1/igt@kms_color@ctm-0-25@pipe-b-hdmi-a-1.html
* igt@kms_color@ctm-green-to-red@pipe-a-hdmi-a-1:
- shard-snb: NOTRUN -> [SKIP][8] ([fdo#109271]) +11 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-snb1/igt@kms_color@ctm-green-to-red@pipe-a-hdmi-a-1.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-apl: [PASS][9] -> [FAIL][10] ([i915#2346])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@kms_flip@2x-plain-flip-ts-check-interruptible@ab-hdmi-a1-hdmi-a2:
- shard-glk: [PASS][11] -> [FAIL][12] ([i915#2122]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-glk5/igt@kms_flip@2x-plain-flip-ts-check-interruptible@ab-hdmi-a1-hdmi-a2.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-glk2/igt@kms_flip@2x-plain-flip-ts-check-interruptible@ab-hdmi-a1-hdmi-a2.html
* igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1:
- shard-apl: [PASS][13] -> [FAIL][14] ([i915#1188])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-apl1/igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-apl4/igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1.html
* igt@kms_properties@crtc-properties-legacy:
- shard-snb: [PASS][15] -> [SKIP][16] ([fdo#109271]) +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-snb4/igt@kms_properties@crtc-properties-legacy.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-snb1/igt@kms_properties@crtc-properties-legacy.html
#### Possible fixes ####
* igt@gem_barrier_race@remote-request@rcs0:
- {shard-tglu}: [ABORT][17] ([i915#7953] / [i915#8211] / [i915#8234]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-tglu-9/igt@gem_barrier_race@remote-request@rcs0.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-tglu-9/igt@gem_barrier_race@remote-request@rcs0.html
* igt@gem_exec_endless@dispatch@rcs0:
- {shard-rkl}: [TIMEOUT][19] ([i915#3778]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-4/igt@gem_exec_endless@dispatch@rcs0.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-3/igt@gem_exec_endless@dispatch@rcs0.html
* igt@gem_exec_fair@basic-none@vcs0:
- {shard-rkl}: [FAIL][21] ([i915#2842]) -> [PASS][22] +1 similar issue
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-4/igt@gem_exec_fair@basic-none@vcs0.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-7/igt@gem_exec_fair@basic-none@vcs0.html
* igt@gem_mmap_offset@clear@smem0:
- {shard-dg1}: [FAIL][23] ([i915#7962]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-dg1-14/igt@gem_mmap_offset@clear@smem0.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-dg1-18/igt@gem_mmap_offset@clear@smem0.html
* igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
- {shard-rkl}: [SKIP][25] ([i915#1937] / [i915#4579]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-4/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-7/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
* igt@i915_pm_rpm@dpms-non-lpsp:
- {shard-rkl}: [SKIP][27] ([i915#1397]) -> [PASS][28] +1 similar issue
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-7/igt@i915_pm_rpm@dpms-non-lpsp.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-3/igt@i915_pm_rpm@dpms-non-lpsp.html
* igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1:
- shard-glk: [FAIL][29] ([i915#2521]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-glk8/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-glk9/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-glk: [FAIL][31] ([i915#2346]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-glk2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@kms_cursor_legacy@single-bo@pipe-b:
- {shard-rkl}: [INCOMPLETE][33] ([i915#8011]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-7/igt@kms_cursor_legacy@single-bo@pipe-b.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-3/igt@kms_cursor_legacy@single-bo@pipe-b.html
* igt@kms_rotation_crc@bad-pixel-format:
- {shard-rkl}: [ABORT][35] ([i915#8311]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-3/igt@kms_rotation_crc@bad-pixel-format.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-4/igt@kms_rotation_crc@bad-pixel-format.html
* igt@kms_rotation_crc@primary-y-tiled-reflect-x-270:
- {shard-rkl}: [ABORT][37] ([i915#7461] / [i915#8178] / [i915#8311]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-rkl-4/igt@kms_rotation_crc@primary-y-tiled-reflect-x-270.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-rkl-3/igt@kms_rotation_crc@primary-y-tiled-reflect-x-270.html
* igt@kms_vblank@pipe-d-ts-continuation-modeset:
- {shard-dg1}: [DMESG-FAIL][39] -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-dg1-12/igt@kms_vblank@pipe-d-ts-continuation-modeset.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-dg1-15/igt@kms_vblank@pipe-d-ts-continuation-modeset.html
* igt@perf_pmu@idle@rcs0:
- {shard-dg1}: [FAIL][41] ([i915#4349]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13132/shard-dg1-15/igt@perf_pmu@idle@rcs0.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/shard-dg1-17/igt@perf_pmu@idle@rcs0.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
[i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
[i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
[i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
[i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#5099]: https://gitlab.freedesktop.org/drm/intel/issues/5099
[i915#5161]: https://gitlab.freedesktop.org/drm/intel/issues/5161
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
[i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
[i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
[i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7953]: https://gitlab.freedesktop.org/drm/intel/issues/7953
[i915#7962]: https://gitlab.freedesktop.org/drm/intel/issues/7962
[i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8178]: https://gitlab.freedesktop.org/drm/intel/issues/8178
[i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
[i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
[i915#8234]: https://gitlab.freedesktop.org/drm/intel/issues/8234
[i915#8295]: https://gitlab.freedesktop.org/drm/intel/issues/8295
[i915#8311]: https://gitlab.freedesktop.org/drm/intel/issues/8311
[i915#8411]: https://gitlab.freedesktop.org/drm/intel/issues/8411
Build changes
-------------
* Linux: CI_DRM_13132 -> Patchwork_117618v1
CI-20190529: 20190529
CI_DRM_13132: 4760cbc0f5aa85cd9a0ff94ecc58ea3f5e087ea7 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7285: d1cbf2bad9c2664ab8bd3bd0946510a52800912f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_117618v1: 4760cbc0f5aa85cd9a0ff94ecc58ea3f5e087ea7 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117618v1/index.html
[-- Attachment #2: Type: text/html, Size: 12290 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-11 8:20 ` [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references Luca Coelho
@ 2023-05-12 9:04 ` Tvrtko Ursulin
2023-05-12 9:10 ` Coelho, Luciano
0 siblings, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2023-05-12 9:04 UTC (permalink / raw)
To: Luca Coelho, intel-gfx
On 11/05/2023 09:20, Luca Coelho wrote:
> Add a work queue in the intel_wakeref structure to be used exclusively
> by the wake reference mechanism. This is needed in order to avoid
> using the system workqueue and relying on flush_scheduled_work().
>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
> drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
> drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
> drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
> drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
> 6 files changed, 60 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 0aff5bb13c53..6505bfa70cd0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
> goto err_cmd_parser;
>
> intel_engine_init_execlists(engine);
> - intel_engine_init__pm(engine);
> +
> + err = intel_engine_init__pm(engine);
> + if (err)
> + goto err_cmd_parser;
> +
> intel_engine_init_retire(engine);
>
> /* Use the whole device by default */
> @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
> {
> GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
>
> + intel_engine_destroy__pm(engine);
> i915_sched_engine_put(engine->sched_engine);
> intel_breadcrumbs_put(engine->breadcrumbs);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index ee531a5c142c..859b62cf660f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
> .put = __engine_park,
> };
>
> -void intel_engine_init__pm(struct intel_engine_cs *engine)
> +int intel_engine_init__pm(struct intel_engine_cs *engine)
> {
> struct intel_runtime_pm *rpm = engine->uncore->rpm;
> + int err;
> +
> + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> + if (err)
> + return err;
>
> - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> intel_engine_init_heartbeat(engine);
>
> intel_gsc_idle_msg_enable(engine);
> +
> + return 0;
> +}
> +
> +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
> +{
> + intel_wakeref_destroy(&engine->wakeref);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> index d68675925b79..e8568f7d10c6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> return rq;
> }
>
> -void intel_engine_init__pm(struct intel_engine_cs *engine);
> +int intel_engine_init__pm(struct intel_engine_cs *engine);
> +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
>
> void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
>
> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> index c0637bf799a3..0a3c702c21e2 100644
> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
> intel_context_put(engine->kernel_context);
>
> intel_engine_fini_retire(engine);
> + intel_engine_destroy__pm(engine);
> }
>
> struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> int mock_engine_init(struct intel_engine_cs *engine)
> {
> struct intel_context *ce;
> + int err;
>
> INIT_LIST_HEAD(&engine->pinned_contexts_list);
>
> @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
> engine->sched_engine->private_data = engine;
>
> intel_engine_init_execlists(engine);
> - intel_engine_init__pm(engine);
> +
> + err = intel_engine_init__pm(engine);
> + if (err)
> + return err;
> +
> intel_engine_init_retire(engine);
>
> engine->breadcrumbs = intel_breadcrumbs_create(NULL);
> diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
> index dfd87d082218..6bae609e1312 100644
> --- a/drivers/gpu/drm/i915/intel_wakeref.c
> +++ b/drivers/gpu/drm/i915/intel_wakeref.c
> @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
>
> /* Assume we are not in process context and so cannot sleep. */
> if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
> - mod_delayed_work(system_wq, &wf->work,
> + mod_delayed_work(wf->wq, &wf->work,
> FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
> return;
> }
> @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
> ____intel_wakeref_put_last(wf);
> }
>
> -void __intel_wakeref_init(struct intel_wakeref *wf,
> - struct intel_runtime_pm *rpm,
> - const struct intel_wakeref_ops *ops,
> - struct intel_wakeref_lockclass *key)
> +int __intel_wakeref_init(struct intel_wakeref *wf,
> + struct intel_runtime_pm *rpm,
> + const struct intel_wakeref_ops *ops,
> + struct intel_wakeref_lockclass *key)
> {
> wf->rpm = rpm;
> wf->ops = ops;
> @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
> atomic_set(&wf->count, 0);
> wf->wakeref = 0;
>
> + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
Typo here - I wanted to ask however - why does this particular wq
"deserves" to be dedicated and can't just use one of the
drm_i915_private ones?
Regards,
Tvrtko
> + if (wf->wq == NULL)
> + return -ENOMEM;
> +
> INIT_DELAYED_WORK(&wf->work, __intel_wakeref_put_work);
> lockdep_init_map(&wf->work.work.lockdep_map,
> "wakeref.work", &key->work, 0);
> +
> + return 0;
> +}
> +
> +void intel_wakeref_destroy(struct intel_wakeref *wf)
> +{
> + destroy_workqueue(wf->wq);
> }
>
> int intel_wakeref_wait_for_idle(struct intel_wakeref *wf)
> diff --git a/drivers/gpu/drm/i915/intel_wakeref.h b/drivers/gpu/drm/i915/intel_wakeref.h
> index 0b6b4852ab23..86f99c044444 100644
> --- a/drivers/gpu/drm/i915/intel_wakeref.h
> +++ b/drivers/gpu/drm/i915/intel_wakeref.h
> @@ -42,6 +42,7 @@ struct intel_wakeref {
> struct intel_runtime_pm *rpm;
> const struct intel_wakeref_ops *ops;
>
> + struct workqueue_struct *wq;
> struct delayed_work work;
> };
>
> @@ -50,15 +51,21 @@ struct intel_wakeref_lockclass {
> struct lock_class_key work;
> };
>
> -void __intel_wakeref_init(struct intel_wakeref *wf,
> - struct intel_runtime_pm *rpm,
> - const struct intel_wakeref_ops *ops,
> - struct intel_wakeref_lockclass *key);
> -#define intel_wakeref_init(wf, rpm, ops) do { \
> - static struct intel_wakeref_lockclass __key; \
> - \
> - __intel_wakeref_init((wf), (rpm), (ops), &__key); \
> -} while (0)
> +int __intel_wakeref_init(struct intel_wakeref *wf,
> + struct intel_runtime_pm *rpm,
> + const struct intel_wakeref_ops *ops,
> + struct intel_wakeref_lockclass *key);
> +static inline int
> +intel_wakeref_init(struct intel_wakeref *wf,
> + struct intel_runtime_pm *rpm,
> + const struct intel_wakeref_ops *ops)
> +{
> + static struct intel_wakeref_lockclass __key;
> +
> + return __intel_wakeref_init((wf), (rpm), (ops), &__key);
> +}
> +
> +void intel_wakeref_destroy(struct intel_wakeref *wf);
>
> int __intel_wakeref_get_first(struct intel_wakeref *wf);
> void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags);
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-12 9:04 ` Tvrtko Ursulin
@ 2023-05-12 9:10 ` Coelho, Luciano
2023-05-12 9:32 ` Tvrtko Ursulin
0 siblings, 1 reply; 16+ messages in thread
From: Coelho, Luciano @ 2023-05-12 9:10 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx
On Fri, 2023-05-12 at 10:04 +0100, Tvrtko Ursulin wrote:
> On 11/05/2023 09:20, Luca Coelho wrote:
> > Add a work queue in the intel_wakeref structure to be used exclusively
> > by the wake reference mechanism. This is needed in order to avoid
> > using the system workqueue and relying on flush_scheduled_work().
> >
> > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
> > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
> > drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
> > drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
> > drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
> > drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
> > 6 files changed, 60 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index 0aff5bb13c53..6505bfa70cd0 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
> > goto err_cmd_parser;
> >
> > intel_engine_init_execlists(engine);
> > - intel_engine_init__pm(engine);
> > +
> > + err = intel_engine_init__pm(engine);
> > + if (err)
> > + goto err_cmd_parser;
> > +
> > intel_engine_init_retire(engine);
> >
> > /* Use the whole device by default */
> > @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
> > {
> > GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
> >
> > + intel_engine_destroy__pm(engine);
> > i915_sched_engine_put(engine->sched_engine);
> > intel_breadcrumbs_put(engine->breadcrumbs);
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > index ee531a5c142c..859b62cf660f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
> > .put = __engine_park,
> > };
> >
> > -void intel_engine_init__pm(struct intel_engine_cs *engine)
> > +int intel_engine_init__pm(struct intel_engine_cs *engine)
> > {
> > struct intel_runtime_pm *rpm = engine->uncore->rpm;
> > + int err;
> > +
> > + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > + if (err)
> > + return err;
> >
> > - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > intel_engine_init_heartbeat(engine);
> >
> > intel_gsc_idle_msg_enable(engine);
> > +
> > + return 0;
> > +}
> > +
> > +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
> > +{
> > + intel_wakeref_destroy(&engine->wakeref);
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > index d68675925b79..e8568f7d10c6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> > return rq;
> > }
> >
> > -void intel_engine_init__pm(struct intel_engine_cs *engine);
> > +int intel_engine_init__pm(struct intel_engine_cs *engine);
> > +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
> >
> > void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
> >
> > diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> > index c0637bf799a3..0a3c702c21e2 100644
> > --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> > +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> > @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
> > intel_context_put(engine->kernel_context);
> >
> > intel_engine_fini_retire(engine);
> > + intel_engine_destroy__pm(engine);
> > }
> >
> > struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> > @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> > int mock_engine_init(struct intel_engine_cs *engine)
> > {
> > struct intel_context *ce;
> > + int err;
> >
> > INIT_LIST_HEAD(&engine->pinned_contexts_list);
> >
> > @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
> > engine->sched_engine->private_data = engine;
> >
> > intel_engine_init_execlists(engine);
> > - intel_engine_init__pm(engine);
> > +
> > + err = intel_engine_init__pm(engine);
> > + if (err)
> > + return err;
> > +
> > intel_engine_init_retire(engine);
> >
> > engine->breadcrumbs = intel_breadcrumbs_create(NULL);
> > diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
> > index dfd87d082218..6bae609e1312 100644
> > --- a/drivers/gpu/drm/i915/intel_wakeref.c
> > +++ b/drivers/gpu/drm/i915/intel_wakeref.c
> > @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
> >
> > /* Assume we are not in process context and so cannot sleep. */
> > if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
> > - mod_delayed_work(system_wq, &wf->work,
> > + mod_delayed_work(wf->wq, &wf->work,
> > FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
> > return;
> > }
> > @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
> > ____intel_wakeref_put_last(wf);
> > }
> >
> > -void __intel_wakeref_init(struct intel_wakeref *wf,
> > - struct intel_runtime_pm *rpm,
> > - const struct intel_wakeref_ops *ops,
> > - struct intel_wakeref_lockclass *key)
> > +int __intel_wakeref_init(struct intel_wakeref *wf,
> > + struct intel_runtime_pm *rpm,
> > + const struct intel_wakeref_ops *ops,
> > + struct intel_wakeref_lockclass *key)
> > {
> > wf->rpm = rpm;
> > wf->ops = ops;
> > @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
> > atomic_set(&wf->count, 0);
> > wf->wakeref = 0;
> >
> > + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
>
> Typo here -
Oh, good catch! This is one of my "favorite" typos, for some reason.
> I wanted to ask however - why does this particular wq
> "deserves" to be dedicated and can't just use one of the
> drm_i915_private ones?
It's because there's no easy way to get access to the drm_i915_private
structure from here. And I don't think this work needs to be in sync
with the rest of the works in i915.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-12 9:10 ` Coelho, Luciano
@ 2023-05-12 9:32 ` Tvrtko Ursulin
2023-05-12 9:54 ` Coelho, Luciano
0 siblings, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2023-05-12 9:32 UTC (permalink / raw)
To: Coelho, Luciano, intel-gfx
On 12/05/2023 10:10, Coelho, Luciano wrote:
> On Fri, 2023-05-12 at 10:04 +0100, Tvrtko Ursulin wrote:
>> On 11/05/2023 09:20, Luca Coelho wrote:
>>> Add a work queue in the intel_wakeref structure to be used exclusively
>>> by the wake reference mechanism. This is needed in order to avoid
>>> using the system workqueue and relying on flush_scheduled_work().
>>>
>>> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
>>> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
>>> drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
>>> drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
>>> drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
>>> drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
>>> 6 files changed, 60 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 0aff5bb13c53..6505bfa70cd0 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>>> goto err_cmd_parser;
>>>
>>> intel_engine_init_execlists(engine);
>>> - intel_engine_init__pm(engine);
>>> +
>>> + err = intel_engine_init__pm(engine);
>>> + if (err)
>>> + goto err_cmd_parser;
>>> +
>>> intel_engine_init_retire(engine);
>>>
>>> /* Use the whole device by default */
>>> @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>>> {
>>> GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
>>>
>>> + intel_engine_destroy__pm(engine);
>>> i915_sched_engine_put(engine->sched_engine);
>>> intel_breadcrumbs_put(engine->breadcrumbs);
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>> index ee531a5c142c..859b62cf660f 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>> @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
>>> .put = __engine_park,
>>> };
>>>
>>> -void intel_engine_init__pm(struct intel_engine_cs *engine)
>>> +int intel_engine_init__pm(struct intel_engine_cs *engine)
>>> {
>>> struct intel_runtime_pm *rpm = engine->uncore->rpm;
>>> + int err;
>>> +
>>> + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>> + if (err)
>>> + return err;
>>>
>>> - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>> intel_engine_init_heartbeat(engine);
>>>
>>> intel_gsc_idle_msg_enable(engine);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
>>> +{
>>> + intel_wakeref_destroy(&engine->wakeref);
>>> }
>>>
>>> /**
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>> index d68675925b79..e8568f7d10c6 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>> @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
>>> return rq;
>>> }
>>>
>>> -void intel_engine_init__pm(struct intel_engine_cs *engine);
>>> +int intel_engine_init__pm(struct intel_engine_cs *engine);
>>> +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
>>>
>>> void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
>>> index c0637bf799a3..0a3c702c21e2 100644
>>> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
>>> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
>>> @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
>>> intel_context_put(engine->kernel_context);
>>>
>>> intel_engine_fini_retire(engine);
>>> + intel_engine_destroy__pm(engine);
>>> }
>>>
>>> struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>>> @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>>> int mock_engine_init(struct intel_engine_cs *engine)
>>> {
>>> struct intel_context *ce;
>>> + int err;
>>>
>>> INIT_LIST_HEAD(&engine->pinned_contexts_list);
>>>
>>> @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
>>> engine->sched_engine->private_data = engine;
>>>
>>> intel_engine_init_execlists(engine);
>>> - intel_engine_init__pm(engine);
>>> +
>>> + err = intel_engine_init__pm(engine);
>>> + if (err)
>>> + return err;
>>> +
>>> intel_engine_init_retire(engine);
>>>
>>> engine->breadcrumbs = intel_breadcrumbs_create(NULL);
>>> diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
>>> index dfd87d082218..6bae609e1312 100644
>>> --- a/drivers/gpu/drm/i915/intel_wakeref.c
>>> +++ b/drivers/gpu/drm/i915/intel_wakeref.c
>>> @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
>>>
>>> /* Assume we are not in process context and so cannot sleep. */
>>> if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
>>> - mod_delayed_work(system_wq, &wf->work,
>>> + mod_delayed_work(wf->wq, &wf->work,
>>> FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
>>> return;
>>> }
>>> @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
>>> ____intel_wakeref_put_last(wf);
>>> }
>>>
>>> -void __intel_wakeref_init(struct intel_wakeref *wf,
>>> - struct intel_runtime_pm *rpm,
>>> - const struct intel_wakeref_ops *ops,
>>> - struct intel_wakeref_lockclass *key)
>>> +int __intel_wakeref_init(struct intel_wakeref *wf,
>>> + struct intel_runtime_pm *rpm,
>>> + const struct intel_wakeref_ops *ops,
>>> + struct intel_wakeref_lockclass *key)
>>> {
>>> wf->rpm = rpm;
>>> wf->ops = ops;
>>> @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
>>> atomic_set(&wf->count, 0);
>>> wf->wakeref = 0;
>>>
>>> + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
>>
>> Typo here -
>
> Oh, good catch! This is one of my "favorite" typos, for some reason.
Yes, I had the same one. :) Patch 3/3 too.
>> I wanted to ask however - why does this particular wq
>> "deserves" to be dedicated and can't just use one of the
>> drm_i915_private ones?
>
> It's because there's no easy way to get access to the drm_i915_private
> structure from here. And I don't think this work needs to be in sync
> with the rest of the works in i915.
Yeah I don't think it needs to be synchronised either. Was just thinking
if we really need to be creating a bunch of separate workqueues (one per
engine) for not much use, or instead could just add a backpointer to
either intel_wakeref or intel_runtime_pm. Latter already has rpm->kdev
so could plausably be replaced with rpm->i915.
Actually, looking at intel_runtime_pm_init_early, you could get to i915
via wf->rpm and container_of.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-12 9:32 ` Tvrtko Ursulin
@ 2023-05-12 9:54 ` Coelho, Luciano
2023-05-12 12:16 ` Tvrtko Ursulin
0 siblings, 1 reply; 16+ messages in thread
From: Coelho, Luciano @ 2023-05-12 9:54 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx
On Fri, 2023-05-12 at 10:32 +0100, Tvrtko Ursulin wrote:
> On 12/05/2023 10:10, Coelho, Luciano wrote:
> > On Fri, 2023-05-12 at 10:04 +0100, Tvrtko Ursulin wrote:
> > > On 11/05/2023 09:20, Luca Coelho wrote:
> > > > Add a work queue in the intel_wakeref structure to be used exclusively
> > > > by the wake reference mechanism. This is needed in order to avoid
> > > > using the system workqueue and relying on flush_scheduled_work().
> > > >
> > > > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
> > > > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
> > > > drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
> > > > drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
> > > > drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
> > > > drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
> > > > 6 files changed, 60 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > index 0aff5bb13c53..6505bfa70cd0 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
> > > > goto err_cmd_parser;
> > > >
> > > > intel_engine_init_execlists(engine);
> > > > - intel_engine_init__pm(engine);
> > > > +
> > > > + err = intel_engine_init__pm(engine);
> > > > + if (err)
> > > > + goto err_cmd_parser;
> > > > +
> > > > intel_engine_init_retire(engine);
> > > >
> > > > /* Use the whole device by default */
> > > > @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
> > > > {
> > > > GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
> > > >
> > > > + intel_engine_destroy__pm(engine);
> > > > i915_sched_engine_put(engine->sched_engine);
> > > > intel_breadcrumbs_put(engine->breadcrumbs);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > index ee531a5c142c..859b62cf660f 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
> > > > .put = __engine_park,
> > > > };
> > > >
> > > > -void intel_engine_init__pm(struct intel_engine_cs *engine)
> > > > +int intel_engine_init__pm(struct intel_engine_cs *engine)
> > > > {
> > > > struct intel_runtime_pm *rpm = engine->uncore->rpm;
> > > > + int err;
> > > > +
> > > > + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > > > + if (err)
> > > > + return err;
> > > >
> > > > - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > > > intel_engine_init_heartbeat(engine);
> > > >
> > > > intel_gsc_idle_msg_enable(engine);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
> > > > +{
> > > > + intel_wakeref_destroy(&engine->wakeref);
> > > > }
> > > >
> > > > /**
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > > > index d68675925b79..e8568f7d10c6 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > > > @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> > > > return rq;
> > > > }
> > > >
> > > > -void intel_engine_init__pm(struct intel_engine_cs *engine);
> > > > +int intel_engine_init__pm(struct intel_engine_cs *engine);
> > > > +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
> > > >
> > > > void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> > > > index c0637bf799a3..0a3c702c21e2 100644
> > > > --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> > > > +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> > > > @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
> > > > intel_context_put(engine->kernel_context);
> > > >
> > > > intel_engine_fini_retire(engine);
> > > > + intel_engine_destroy__pm(engine);
> > > > }
> > > >
> > > > struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> > > > @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> > > > int mock_engine_init(struct intel_engine_cs *engine)
> > > > {
> > > > struct intel_context *ce;
> > > > + int err;
> > > >
> > > > INIT_LIST_HEAD(&engine->pinned_contexts_list);
> > > >
> > > > @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
> > > > engine->sched_engine->private_data = engine;
> > > >
> > > > intel_engine_init_execlists(engine);
> > > > - intel_engine_init__pm(engine);
> > > > +
> > > > + err = intel_engine_init__pm(engine);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > intel_engine_init_retire(engine);
> > > >
> > > > engine->breadcrumbs = intel_breadcrumbs_create(NULL);
> > > > diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
> > > > index dfd87d082218..6bae609e1312 100644
> > > > --- a/drivers/gpu/drm/i915/intel_wakeref.c
> > > > +++ b/drivers/gpu/drm/i915/intel_wakeref.c
> > > > @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
> > > >
> > > > /* Assume we are not in process context and so cannot sleep. */
> > > > if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
> > > > - mod_delayed_work(system_wq, &wf->work,
> > > > + mod_delayed_work(wf->wq, &wf->work,
> > > > FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
> > > > return;
> > > > }
> > > > @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
> > > > ____intel_wakeref_put_last(wf);
> > > > }
> > > >
> > > > -void __intel_wakeref_init(struct intel_wakeref *wf,
> > > > - struct intel_runtime_pm *rpm,
> > > > - const struct intel_wakeref_ops *ops,
> > > > - struct intel_wakeref_lockclass *key)
> > > > +int __intel_wakeref_init(struct intel_wakeref *wf,
> > > > + struct intel_runtime_pm *rpm,
> > > > + const struct intel_wakeref_ops *ops,
> > > > + struct intel_wakeref_lockclass *key)
> > > > {
> > > > wf->rpm = rpm;
> > > > wf->ops = ops;
> > > > @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
> > > > atomic_set(&wf->count, 0);
> > > > wf->wakeref = 0;
> > > >
> > > > + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
> > >
> > > Typo here -
> >
> > Oh, good catch! This is one of my "favorite" typos, for some reason.
>
> Yes, I had the same one. :) Patch 3/3 too.
>
> > > I wanted to ask however - why does this particular wq
> > > "deserves" to be dedicated and can't just use one of the
> > > drm_i915_private ones?
> >
> > It's because there's no easy way to get access to the drm_i915_private
> > structure from here. And I don't think this work needs to be in sync
> > with the rest of the works in i915.
>
> Yeah I don't think it needs to be synchronised either. Was just thinking
> if we really need to be creating a bunch of separate workqueues (one per
> engine) for not much use, or instead could just add a backpointer to
> either intel_wakeref or intel_runtime_pm. Latter already has rpm->kdev
> so could plausably be replaced with rpm->i915.
>
> Actually, looking at intel_runtime_pm_init_early, you could get to i915
> via wf->rpm and container_of.
Yeah, I considered that, but using container_of() can be problematic
when we're not sure where exactly the element is coming from. My worry
was this:
int intel_engine_init__pm(struct intel_engine_cs *engine)
{
struct intel_runtime_pm *rpm = engine->uncore->rpm;
int err;
err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
[...]
}
In this case, we're getting to __intel_wakeref_init() with an *rpm that
is coming from an intel_uncore structure and not from
drm_i915_private...
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-12 9:54 ` Coelho, Luciano
@ 2023-05-12 12:16 ` Tvrtko Ursulin
2023-05-17 11:18 ` Coelho, Luciano
0 siblings, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2023-05-12 12:16 UTC (permalink / raw)
To: Coelho, Luciano, intel-gfx
On 12/05/2023 10:54, Coelho, Luciano wrote:
> On Fri, 2023-05-12 at 10:32 +0100, Tvrtko Ursulin wrote:
>> On 12/05/2023 10:10, Coelho, Luciano wrote:
>>> On Fri, 2023-05-12 at 10:04 +0100, Tvrtko Ursulin wrote:
>>>> On 11/05/2023 09:20, Luca Coelho wrote:
>>>>> Add a work queue in the intel_wakeref structure to be used exclusively
>>>>> by the wake reference mechanism. This is needed in order to avoid
>>>>> using the system workqueue and relying on flush_scheduled_work().
>>>>>
>>>>> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
>>>>> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
>>>>> drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
>>>>> drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
>>>>> drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
>>>>> drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
>>>>> 6 files changed, 60 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>>> index 0aff5bb13c53..6505bfa70cd0 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>>> @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>>>>> goto err_cmd_parser;
>>>>>
>>>>> intel_engine_init_execlists(engine);
>>>>> - intel_engine_init__pm(engine);
>>>>> +
>>>>> + err = intel_engine_init__pm(engine);
>>>>> + if (err)
>>>>> + goto err_cmd_parser;
>>>>> +
>>>>> intel_engine_init_retire(engine);
>>>>>
>>>>> /* Use the whole device by default */
>>>>> @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>>>>> {
>>>>> GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
>>>>>
>>>>> + intel_engine_destroy__pm(engine);
>>>>> i915_sched_engine_put(engine->sched_engine);
>>>>> intel_breadcrumbs_put(engine->breadcrumbs);
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>>>> index ee531a5c142c..859b62cf660f 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>>>> @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
>>>>> .put = __engine_park,
>>>>> };
>>>>>
>>>>> -void intel_engine_init__pm(struct intel_engine_cs *engine)
>>>>> +int intel_engine_init__pm(struct intel_engine_cs *engine)
>>>>> {
>>>>> struct intel_runtime_pm *rpm = engine->uncore->rpm;
>>>>> + int err;
>>>>> +
>>>>> + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>>>> + if (err)
>>>>> + return err;
>>>>>
>>>>> - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>>>> intel_engine_init_heartbeat(engine);
>>>>>
>>>>> intel_gsc_idle_msg_enable(engine);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
>>>>> +{
>>>>> + intel_wakeref_destroy(&engine->wakeref);
>>>>> }
>>>>>
>>>>> /**
>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>>>> index d68675925b79..e8568f7d10c6 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>>>> @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
>>>>> return rq;
>>>>> }
>>>>>
>>>>> -void intel_engine_init__pm(struct intel_engine_cs *engine);
>>>>> +int intel_engine_init__pm(struct intel_engine_cs *engine);
>>>>> +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
>>>>>
>>>>> void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
>>>>> index c0637bf799a3..0a3c702c21e2 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
>>>>> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
>>>>> @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
>>>>> intel_context_put(engine->kernel_context);
>>>>>
>>>>> intel_engine_fini_retire(engine);
>>>>> + intel_engine_destroy__pm(engine);
>>>>> }
>>>>>
>>>>> struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>>>>> @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>>>>> int mock_engine_init(struct intel_engine_cs *engine)
>>>>> {
>>>>> struct intel_context *ce;
>>>>> + int err;
>>>>>
>>>>> INIT_LIST_HEAD(&engine->pinned_contexts_list);
>>>>>
>>>>> @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
>>>>> engine->sched_engine->private_data = engine;
>>>>>
>>>>> intel_engine_init_execlists(engine);
>>>>> - intel_engine_init__pm(engine);
>>>>> +
>>>>> + err = intel_engine_init__pm(engine);
>>>>> + if (err)
>>>>> + return err;
>>>>> +
>>>>> intel_engine_init_retire(engine);
>>>>>
>>>>> engine->breadcrumbs = intel_breadcrumbs_create(NULL);
>>>>> diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
>>>>> index dfd87d082218..6bae609e1312 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_wakeref.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_wakeref.c
>>>>> @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
>>>>>
>>>>> /* Assume we are not in process context and so cannot sleep. */
>>>>> if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
>>>>> - mod_delayed_work(system_wq, &wf->work,
>>>>> + mod_delayed_work(wf->wq, &wf->work,
>>>>> FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
>>>>> return;
>>>>> }
>>>>> @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
>>>>> ____intel_wakeref_put_last(wf);
>>>>> }
>>>>>
>>>>> -void __intel_wakeref_init(struct intel_wakeref *wf,
>>>>> - struct intel_runtime_pm *rpm,
>>>>> - const struct intel_wakeref_ops *ops,
>>>>> - struct intel_wakeref_lockclass *key)
>>>>> +int __intel_wakeref_init(struct intel_wakeref *wf,
>>>>> + struct intel_runtime_pm *rpm,
>>>>> + const struct intel_wakeref_ops *ops,
>>>>> + struct intel_wakeref_lockclass *key)
>>>>> {
>>>>> wf->rpm = rpm;
>>>>> wf->ops = ops;
>>>>> @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
>>>>> atomic_set(&wf->count, 0);
>>>>> wf->wakeref = 0;
>>>>>
>>>>> + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
>>>>
>>>> Typo here -
>>>
>>> Oh, good catch! This is one of my "favorite" typos, for some reason.
>>
>> Yes, I had the same one. :) Patch 3/3 too.
>>
>>>> I wanted to ask however - why does this particular wq
>>>> "deserves" to be dedicated and can't just use one of the
>>>> drm_i915_private ones?
>>>
>>> It's because there's no easy way to get access to the drm_i915_private
>>> structure from here. And I don't think this work needs to be in sync
>>> with the rest of the works in i915.
>>
>> Yeah I don't think it needs to be synchronised either. Was just thinking
>> if we really need to be creating a bunch of separate workqueues (one per
>> engine) for not much use, or instead could just add a backpointer to
>> either intel_wakeref or intel_runtime_pm. Latter already has rpm->kdev
>> so could plausably be replaced with rpm->i915.
>>
>> Actually, looking at intel_runtime_pm_init_early, you could get to i915
>> via wf->rpm and container_of.
>
> Yeah, I considered that, but using container_of() can be problematic
> when we're not sure where exactly the element is coming from. My worry
> was this:
>
> int intel_engine_init__pm(struct intel_engine_cs *engine)
> {
> struct intel_runtime_pm *rpm = engine->uncore->rpm;
> int err;
>
> err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> [...]
> }
>
> In this case, we're getting to __intel_wakeref_init() with an *rpm that
> is coming from an intel_uncore structure and not from
> drm_i915_private...
Right. Yes I agree that would be a flaky/questionable design, even if it
worked in practice. I'd just replace rpm->dev with rpm->i915 then. Not
feeling *that* strongly about it, but it just feels a waste to create a
bunch of workqueues for this.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private
2023-05-11 8:20 ` [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private Luca Coelho
@ 2023-05-12 12:34 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2023-05-12 12:34 UTC (permalink / raw)
To: Luca Coelho, intel-gfx
On 11/05/2023 09:20, Luca Coelho wrote:
> In order to avoid flush_scheduled_work() usage, add a dedicated
> workqueue in the drm_i915_private structure. In this way, we don't
> need to use the system queue anymore.
>
> This change is mostly mechanical and based on Tetsuo's original
> patch[1].
>
> Link: https://patchwork.freedesktop.org/series/114608/ [1]
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
[snip]
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 14c5338c96a6..06cd956b03ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -259,6 +259,8 @@ struct drm_i915_private {
> */
> struct workqueue_struct *wq;
>
> + struct workqueue_struct *i915_wq;
Two things.
First, the i915->wq has a nice big comments accompanying both the
declaration and initialization. Lets come up with at least one solid
comment for the new one too. No one will otherwise know when to use
i915->wq and when i915->i915_wq.
Which leads me to the second point. Lets try find a nicer name for it.
i915->i915_wq reads a bit bad when we both have i915->wq and also i915
mentioned twice. Easy cheat option could be to signify the property in
the name - like i915->unordered_wq. Or something. It may depend on what
that "solid comment" from the first point will be. I mean what we decide
the prescribed use cases for the new wq will be.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-12 12:16 ` Tvrtko Ursulin
@ 2023-05-17 11:18 ` Coelho, Luciano
2023-05-19 7:56 ` Tvrtko Ursulin
0 siblings, 1 reply; 16+ messages in thread
From: Coelho, Luciano @ 2023-05-17 11:18 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx
On Fri, 2023-05-12 at 13:16 +0100, Tvrtko Ursulin wrote:
> On 12/05/2023 10:54, Coelho, Luciano wrote:
> > On Fri, 2023-05-12 at 10:32 +0100, Tvrtko Ursulin wrote:
> > > On 12/05/2023 10:10, Coelho, Luciano wrote:
> > > > On Fri, 2023-05-12 at 10:04 +0100, Tvrtko Ursulin wrote:
> > > > > On 11/05/2023 09:20, Luca Coelho wrote:
> > > > > > Add a work queue in the intel_wakeref structure to be used exclusively
> > > > > > by the wake reference mechanism. This is needed in order to avoid
> > > > > > using the system workqueue and relying on flush_scheduled_work().
> > > > > >
> > > > > > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
> > > > > > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
> > > > > > drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
> > > > > > drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
> > > > > > drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
> > > > > > drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
> > > > > > 6 files changed, 60 insertions(+), 19 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > > > index 0aff5bb13c53..6505bfa70cd0 100644
> > > > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > > > @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
> > > > > > goto err_cmd_parser;
> > > > > >
> > > > > > intel_engine_init_execlists(engine);
> > > > > > - intel_engine_init__pm(engine);
> > > > > > +
> > > > > > + err = intel_engine_init__pm(engine);
> > > > > > + if (err)
> > > > > > + goto err_cmd_parser;
> > > > > > +
> > > > > > intel_engine_init_retire(engine);
> > > > > >
> > > > > > /* Use the whole device by default */
> > > > > > @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
> > > > > > {
> > > > > > GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
> > > > > >
> > > > > > + intel_engine_destroy__pm(engine);
> > > > > > i915_sched_engine_put(engine->sched_engine);
> > > > > > intel_breadcrumbs_put(engine->breadcrumbs);
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > > > index ee531a5c142c..859b62cf660f 100644
> > > > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > > > > @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
> > > > > > .put = __engine_park,
> > > > > > };
> > > > > >
> > > > > > -void intel_engine_init__pm(struct intel_engine_cs *engine)
> > > > > > +int intel_engine_init__pm(struct intel_engine_cs *engine)
> > > > > > {
> > > > > > struct intel_runtime_pm *rpm = engine->uncore->rpm;
> > > > > > + int err;
> > > > > > +
> > > > > > + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > > > > > + if (err)
> > > > > > + return err;
> > > > > >
> > > > > > - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > > > > > intel_engine_init_heartbeat(engine);
> > > > > >
> > > > > > intel_gsc_idle_msg_enable(engine);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
> > > > > > +{
> > > > > > + intel_wakeref_destroy(&engine->wakeref);
> > > > > > }
> > > > > >
> > > > > > /**
> > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > > > > > index d68675925b79..e8568f7d10c6 100644
> > > > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > > > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> > > > > > @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> > > > > > return rq;
> > > > > > }
> > > > > >
> > > > > > -void intel_engine_init__pm(struct intel_engine_cs *engine);
> > > > > > +int intel_engine_init__pm(struct intel_engine_cs *engine);
> > > > > > +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
> > > > > >
> > > > > > void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> > > > > > index c0637bf799a3..0a3c702c21e2 100644
> > > > > > --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> > > > > > +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> > > > > > @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
> > > > > > intel_context_put(engine->kernel_context);
> > > > > >
> > > > > > intel_engine_fini_retire(engine);
> > > > > > + intel_engine_destroy__pm(engine);
> > > > > > }
> > > > > >
> > > > > > struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> > > > > > @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
> > > > > > int mock_engine_init(struct intel_engine_cs *engine)
> > > > > > {
> > > > > > struct intel_context *ce;
> > > > > > + int err;
> > > > > >
> > > > > > INIT_LIST_HEAD(&engine->pinned_contexts_list);
> > > > > >
> > > > > > @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
> > > > > > engine->sched_engine->private_data = engine;
> > > > > >
> > > > > > intel_engine_init_execlists(engine);
> > > > > > - intel_engine_init__pm(engine);
> > > > > > +
> > > > > > + err = intel_engine_init__pm(engine);
> > > > > > + if (err)
> > > > > > + return err;
> > > > > > +
> > > > > > intel_engine_init_retire(engine);
> > > > > >
> > > > > > engine->breadcrumbs = intel_breadcrumbs_create(NULL);
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
> > > > > > index dfd87d082218..6bae609e1312 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_wakeref.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_wakeref.c
> > > > > > @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
> > > > > >
> > > > > > /* Assume we are not in process context and so cannot sleep. */
> > > > > > if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
> > > > > > - mod_delayed_work(system_wq, &wf->work,
> > > > > > + mod_delayed_work(wf->wq, &wf->work,
> > > > > > FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
> > > > > > return;
> > > > > > }
> > > > > > @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
> > > > > > ____intel_wakeref_put_last(wf);
> > > > > > }
> > > > > >
> > > > > > -void __intel_wakeref_init(struct intel_wakeref *wf,
> > > > > > - struct intel_runtime_pm *rpm,
> > > > > > - const struct intel_wakeref_ops *ops,
> > > > > > - struct intel_wakeref_lockclass *key)
> > > > > > +int __intel_wakeref_init(struct intel_wakeref *wf,
> > > > > > + struct intel_runtime_pm *rpm,
> > > > > > + const struct intel_wakeref_ops *ops,
> > > > > > + struct intel_wakeref_lockclass *key)
> > > > > > {
> > > > > > wf->rpm = rpm;
> > > > > > wf->ops = ops;
> > > > > > @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
> > > > > > atomic_set(&wf->count, 0);
> > > > > > wf->wakeref = 0;
> > > > > >
> > > > > > + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
> > > > >
> > > > > Typo here -
> > > >
> > > > Oh, good catch! This is one of my "favorite" typos, for some reason.
> > >
> > > Yes, I had the same one. :) Patch 3/3 too.
> > >
> > > > > I wanted to ask however - why does this particular wq
> > > > > "deserves" to be dedicated and can't just use one of the
> > > > > drm_i915_private ones?
> > > >
> > > > It's because there's no easy way to get access to the drm_i915_private
> > > > structure from here. And I don't think this work needs to be in sync
> > > > with the rest of the works in i915.
> > >
> > > Yeah I don't think it needs to be synchronised either. Was just thinking
> > > if we really need to be creating a bunch of separate workqueues (one per
> > > engine) for not much use, or instead could just add a backpointer to
> > > either intel_wakeref or intel_runtime_pm. Latter already has rpm->kdev
> > > so could plausably be replaced with rpm->i915.
> > >
> > > Actually, looking at intel_runtime_pm_init_early, you could get to i915
> > > via wf->rpm and container_of.
> >
> > Yeah, I considered that, but using container_of() can be problematic
> > when we're not sure where exactly the element is coming from. My worry
> > was this:
> >
> > int intel_engine_init__pm(struct intel_engine_cs *engine)
> > {
> > struct intel_runtime_pm *rpm = engine->uncore->rpm;
> > int err;
> >
> > err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> > [...]
> > }
> >
> > In this case, we're getting to __intel_wakeref_init() with an *rpm that
> > is coming from an intel_uncore structure and not from
> > drm_i915_private...
>
> Right. Yes I agree that would be a flaky/questionable design, even if it
> worked in practice. I'd just replace rpm->dev with rpm->i915 then. Not
> feeling *that* strongly about it, but it just feels a waste to create a
> bunch of workqueues for this.
How many engines do we typically have at runtime?
I was looking into getting the i915 struct via rpm->kdev and
container_of(), but it's not trivial either. This is how we initialize
rpm->kdev:
void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm)
{
struct drm_i915_private *i915 =
container_of(rpm, struct drm_i915_private, runtime_pm);
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
struct device *kdev = &pdev->dev;
rpm->kdev = kdev;
[...]
}
So, rpm->kdev actually points to the device structure from inside a
pci_dev. i915->drm.dev points to the same device structure, but we
lose the pointer to the element inside i915, so there's no way to trace
it back. We keep only the pointer to the element inside pci_dev.
Or did you mean that we should change the kdev pointer to an i915
pointer in intel_runtime_pm and derive kdev when needed? This would
cause some code shuffle. And, currently, the wakeref code doesn't have
any references to drm_i915_private at all (or even to drm for that
matter)...
I don't know. We could go either way. I think it depends mostly on
how many engines instances we typically have, so we can weigh runtime
resources vs code complexity...
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
2023-05-17 11:18 ` Coelho, Luciano
@ 2023-05-19 7:56 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2023-05-19 7:56 UTC (permalink / raw)
To: Coelho, Luciano, intel-gfx
On 17/05/2023 12:18, Coelho, Luciano wrote:
> On Fri, 2023-05-12 at 13:16 +0100, Tvrtko Ursulin wrote:
>> On 12/05/2023 10:54, Coelho, Luciano wrote:
>>> On Fri, 2023-05-12 at 10:32 +0100, Tvrtko Ursulin wrote:
>>>> On 12/05/2023 10:10, Coelho, Luciano wrote:
>>>>> On Fri, 2023-05-12 at 10:04 +0100, Tvrtko Ursulin wrote:
>>>>>> On 11/05/2023 09:20, Luca Coelho wrote:
>>>>>>> Add a work queue in the intel_wakeref structure to be used exclusively
>>>>>>> by the wake reference mechanism. This is needed in order to avoid
>>>>>>> using the system workqueue and relying on flush_scheduled_work().
>>>>>>>
>>>>>>> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
>>>>>>> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
>>>>>>> drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
>>>>>>> drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
>>>>>>> drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
>>>>>>> drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
>>>>>>> 6 files changed, 60 insertions(+), 19 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>>>>> index 0aff5bb13c53..6505bfa70cd0 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>>>>>> @@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>>>>>>> goto err_cmd_parser;
>>>>>>>
>>>>>>> intel_engine_init_execlists(engine);
>>>>>>> - intel_engine_init__pm(engine);
>>>>>>> +
>>>>>>> + err = intel_engine_init__pm(engine);
>>>>>>> + if (err)
>>>>>>> + goto err_cmd_parser;
>>>>>>> +
>>>>>>> intel_engine_init_retire(engine);
>>>>>>>
>>>>>>> /* Use the whole device by default */
>>>>>>> @@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>>>>>>> {
>>>>>>> GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
>>>>>>>
>>>>>>> + intel_engine_destroy__pm(engine);
>>>>>>> i915_sched_engine_put(engine->sched_engine);
>>>>>>> intel_breadcrumbs_put(engine->breadcrumbs);
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>>>>>> index ee531a5c142c..859b62cf660f 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
>>>>>>> @@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
>>>>>>> .put = __engine_park,
>>>>>>> };
>>>>>>>
>>>>>>> -void intel_engine_init__pm(struct intel_engine_cs *engine)
>>>>>>> +int intel_engine_init__pm(struct intel_engine_cs *engine)
>>>>>>> {
>>>>>>> struct intel_runtime_pm *rpm = engine->uncore->rpm;
>>>>>>> + int err;
>>>>>>> +
>>>>>>> + err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>>>>>> + if (err)
>>>>>>> + return err;
>>>>>>>
>>>>>>> - intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>>>>>> intel_engine_init_heartbeat(engine);
>>>>>>>
>>>>>>> intel_gsc_idle_msg_enable(engine);
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +void intel_engine_destroy__pm(struct intel_engine_cs *engine)
>>>>>>> +{
>>>>>>> + intel_wakeref_destroy(&engine->wakeref);
>>>>>>> }
>>>>>>>
>>>>>>> /**
>>>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>>>>>> index d68675925b79..e8568f7d10c6 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
>>>>>>> @@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs *engine)
>>>>>>> return rq;
>>>>>>> }
>>>>>>>
>>>>>>> -void intel_engine_init__pm(struct intel_engine_cs *engine);
>>>>>>> +int intel_engine_init__pm(struct intel_engine_cs *engine);
>>>>>>> +void intel_engine_destroy__pm(struct intel_engine_cs *engine);
>>>>>>>
>>>>>>> void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
>>>>>>> index c0637bf799a3..0a3c702c21e2 100644
>>>>>>> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
>>>>>>> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
>>>>>>> @@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)
>>>>>>> intel_context_put(engine->kernel_context);
>>>>>>>
>>>>>>> intel_engine_fini_retire(engine);
>>>>>>> + intel_engine_destroy__pm(engine);
>>>>>>> }
>>>>>>>
>>>>>>> struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>>>>>>> @@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>>>>>>> int mock_engine_init(struct intel_engine_cs *engine)
>>>>>>> {
>>>>>>> struct intel_context *ce;
>>>>>>> + int err;
>>>>>>>
>>>>>>> INIT_LIST_HEAD(&engine->pinned_contexts_list);
>>>>>>>
>>>>>>> @@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
>>>>>>> engine->sched_engine->private_data = engine;
>>>>>>>
>>>>>>> intel_engine_init_execlists(engine);
>>>>>>> - intel_engine_init__pm(engine);
>>>>>>> +
>>>>>>> + err = intel_engine_init__pm(engine);
>>>>>>> + if (err)
>>>>>>> + return err;
>>>>>>> +
>>>>>>> intel_engine_init_retire(engine);
>>>>>>>
>>>>>>> engine->breadcrumbs = intel_breadcrumbs_create(NULL);
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
>>>>>>> index dfd87d082218..6bae609e1312 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_wakeref.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_wakeref.c
>>>>>>> @@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags)
>>>>>>>
>>>>>>> /* Assume we are not in process context and so cannot sleep. */
>>>>>>> if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
>>>>>>> - mod_delayed_work(system_wq, &wf->work,
>>>>>>> + mod_delayed_work(wf->wq, &wf->work,
>>>>>>> FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
>>>>>>> return;
>>>>>>> }
>>>>>>> @@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct *wrk)
>>>>>>> ____intel_wakeref_put_last(wf);
>>>>>>> }
>>>>>>>
>>>>>>> -void __intel_wakeref_init(struct intel_wakeref *wf,
>>>>>>> - struct intel_runtime_pm *rpm,
>>>>>>> - const struct intel_wakeref_ops *ops,
>>>>>>> - struct intel_wakeref_lockclass *key)
>>>>>>> +int __intel_wakeref_init(struct intel_wakeref *wf,
>>>>>>> + struct intel_runtime_pm *rpm,
>>>>>>> + const struct intel_wakeref_ops *ops,
>>>>>>> + struct intel_wakeref_lockclass *key)
>>>>>>> {
>>>>>>> wf->rpm = rpm;
>>>>>>> wf->ops = ops;
>>>>>>> @@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
>>>>>>> atomic_set(&wf->count, 0);
>>>>>>> wf->wakeref = 0;
>>>>>>>
>>>>>>> + wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
>>>>>>
>>>>>> Typo here -
>>>>>
>>>>> Oh, good catch! This is one of my "favorite" typos, for some reason.
>>>>
>>>> Yes, I had the same one. :) Patch 3/3 too.
>>>>
>>>>>> I wanted to ask however - why does this particular wq
>>>>>> "deserves" to be dedicated and can't just use one of the
>>>>>> drm_i915_private ones?
>>>>>
>>>>> It's because there's no easy way to get access to the drm_i915_private
>>>>> structure from here. And I don't think this work needs to be in sync
>>>>> with the rest of the works in i915.
>>>>
>>>> Yeah I don't think it needs to be synchronised either. Was just thinking
>>>> if we really need to be creating a bunch of separate workqueues (one per
>>>> engine) for not much use, or instead could just add a backpointer to
>>>> either intel_wakeref or intel_runtime_pm. Latter already has rpm->kdev
>>>> so could plausably be replaced with rpm->i915.
>>>>
>>>> Actually, looking at intel_runtime_pm_init_early, you could get to i915
>>>> via wf->rpm and container_of.
>>>
>>> Yeah, I considered that, but using container_of() can be problematic
>>> when we're not sure where exactly the element is coming from. My worry
>>> was this:
>>>
>>> int intel_engine_init__pm(struct intel_engine_cs *engine)
>>> {
>>> struct intel_runtime_pm *rpm = engine->uncore->rpm;
>>> int err;
>>>
>>> err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
>>> [...]
>>> }
>>>
>>> In this case, we're getting to __intel_wakeref_init() with an *rpm that
>>> is coming from an intel_uncore structure and not from
>>> drm_i915_private...
>>
>> Right. Yes I agree that would be a flaky/questionable design, even if it
>> worked in practice. I'd just replace rpm->dev with rpm->i915 then. Not
>> feeling *that* strongly about it, but it just feels a waste to create a
>> bunch of workqueues for this.
>
> How many engines do we typically have at runtime?
Currently not that many I think. There were plans for like 18 per tile,
but I don't think it will be happening.
> I was looking into getting the i915 struct via rpm->kdev and
> container_of(), but it's not trivial either. This is how we initialize
> rpm->kdev:
>
> void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm)
> {
> struct drm_i915_private *i915 =
> container_of(rpm, struct drm_i915_private, runtime_pm);
> struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> struct device *kdev = &pdev->dev;
>
> rpm->kdev = kdev;
> [...]
> }
>
> So, rpm->kdev actually points to the device structure from inside a
> pci_dev. i915->drm.dev points to the same device structure, but we
> lose the pointer to the element inside i915, so there's no way to trace
> it back. We keep only the pointer to the element inside pci_dev.
>
> Or did you mean that we should change the kdev pointer to an i915
> pointer in intel_runtime_pm and derive kdev when needed? This would
> cause some code shuffle. And, currently, the wakeref code doesn't have
> any references to drm_i915_private at all (or even to drm for that
> matter)...
Yes, that's what I meant. Struct intel_runtime_pm is only intimately
tied to i915 via lmem_userfault_list so I think conceptually it is fine.
But I haven't look at the level of churn it would require.
> I don't know. We could go either way. I think it depends mostly on
> how many engines instances we typically have, so we can weigh runtime
> resources vs code complexity...
Or we just go with Tetsuo's approach of a global i915_wq. For me that
one is completely fine.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-05-19 7:56 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-11 8:20 [Intel-gfx] [PATCH 0/3] drm/i915: implement internal workqueues Luca Coelho
2023-05-11 8:20 ` [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private Luca Coelho
2023-05-12 12:34 ` Tvrtko Ursulin
2023-05-11 8:20 ` [Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references Luca Coelho
2023-05-12 9:04 ` Tvrtko Ursulin
2023-05-12 9:10 ` Coelho, Luciano
2023-05-12 9:32 ` Tvrtko Ursulin
2023-05-12 9:54 ` Coelho, Luciano
2023-05-12 12:16 ` Tvrtko Ursulin
2023-05-17 11:18 ` Coelho, Luciano
2023-05-19 7:56 ` Tvrtko Ursulin
2023-05-11 8:20 ` [Intel-gfx] [PATCH 3/3] drm/i915/selftests: add local workqueue for SW fence selftest Luca Coelho
2023-05-11 9:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement internal workqueues Patchwork
2023-05-11 9:05 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-05-11 9:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-05-11 10:55 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).