v1: Based on review comments from Daniel following changes are done.
- More focus is given for better synchronization.
- Replaced async firmware loading by using request_firmawre() call.
- Prevented entering in dc5/dc6 while firmware loading in process.
Now register programming for dc5/dc6 always will happen followed
by firmware loading.
- Removed the csr-lock and csr-state which was used before.
- Added a async work which is responsible for both loading the
firmware and register programming for dc5/dc6.
- Added flush_work() to explicitly synchronize the async work
during suspend and driver unloading.
Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_dma.c | 1 -
drivers/gpu/drm/i915/i915_drv.c | 10 +--
drivers/gpu/drm/i915/i915_drv.h | 13 ++-
drivers/gpu/drm/i915/intel_csr.c | 141 +++++++++++---------------------
drivers/gpu/drm/i915/intel_drv.h | 5 +-
drivers/gpu/drm/i915/intel_runtime_pm.c | 63 +++++++-------
6 files changed, 92 insertions(+), 141 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index c5349fa..1ebf0e1 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -820,7 +820,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
spin_lock_init(&dev_priv->mmio_flip_lock);
mutex_init(&dev_priv->sb_lock);
mutex_init(&dev_priv->modeset_restore_lock);
- mutex_init(&dev_priv->csr_lock);
intel_pm_setup(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e44dc0d..6049ce3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1019,11 +1019,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
{
/* Enabling DC6 is not a hard requirement to enter runtime D3 */
- /*
- * This is to ensure that CSR isn't identified as loaded before
- * CSR-loading program is called during runtime-resume.
- */
- intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
+ dev_priv->csr.dmc_present = false;
+ flush_work(&dev_priv->csr.csr_work);
skl_uninit_cdclk(dev_priv);
@@ -1071,10 +1068,7 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
static int skl_resume_prepare(struct drm_i915_private *dev_priv)
{
- struct drm_device *dev = dev_priv->dev;
-
skl_init_cdclk(dev_priv);
- intel_csr_load_program(dev);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1dbd957..73509f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -737,20 +737,16 @@ struct intel_uncore {
#define for_each_fw_domain(domain__, dev_priv__, i__) \
for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
-enum csr_state {
- FW_UNINITIALIZED = 0,
- FW_LOADED,
- FW_FAILED
-};
-
struct intel_csr {
+ bool dc_state_req;
+ struct work_struct csr_work;
const char *fw_path;
__be32 *dmc_payload;
uint32_t dmc_fw_size;
uint32_t mmio_count;
uint32_t mmioaddr[8];
uint32_t mmiodata[8];
- enum csr_state state;
+ bool dmc_present;
};
#define DEV_INFO_FOR_EACH_FLAG(func, sep) \
@@ -2631,6 +2627,9 @@ extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
void i915_firmware_load_error_print(const char *fw_path, int err);
+/* intel_csr.c */
+void intel_csr_setdc_work_fn(struct work_struct *__work);
+
/* intel_hotplug.c */
void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
void intel_hpd_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 6d8a7bf..44f05a8 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -32,13 +32,6 @@
* onwards to drive newly added DMC (Display microcontroller) in display
* engine to save and restore the state of display engine when it enter into
* low-power state and comes back to normal.
- *
- * Firmware loading status will be one of the below states: FW_UNINITIALIZED,
- * FW_LOADED, FW_FAILED.
- *
- * Once the firmware is written into the registers status will be moved from
- * FW_UNINITIALIZED to FW_LOADED and for any erroneous condition status will
- * be moved to FW_FAILED.
*/
#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
@@ -199,48 +192,6 @@ static char intel_get_substepping(struct drm_device *dev)
return -ENODATA;
}
-/**
- * intel_csr_load_status_get() - to get firmware loading status.
- * @dev_priv: i915 device.
- *
- * This function helps to get the firmware loading status.
- *
- * Return: Firmware loading status.
- */
-enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv)
-{
- enum csr_state state;
-
- mutex_lock(&dev_priv->csr_lock);
- state = dev_priv->csr.state;
- mutex_unlock(&dev_priv->csr_lock);
-
- return state;
-}
-
-/**
- * intel_csr_load_status_set() - help to set firmware loading status.
- * @dev_priv: i915 device.
- * @state: enumeration of firmware loading status.
- *
- * Set the firmware loading status.
- */
-void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
- enum csr_state state)
-{
- mutex_lock(&dev_priv->csr_lock);
- dev_priv->csr.state = state;
- mutex_unlock(&dev_priv->csr_lock);
-}
-
-/**
- * intel_csr_load_program() - write the firmware from memory to register.
- * @dev: drm device.
- *
- * CSR firmware is read from a .bin file and kept in internal memory one time.
- * Everytime display comes back from low power state this function is called to
- * copy the firmware from internal memory to registers.
- */
void intel_csr_load_program(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -252,7 +203,6 @@ void intel_csr_load_program(struct drm_device *dev)
return;
}
- mutex_lock(&dev_priv->csr_lock);
fw_size = dev_priv->csr.dmc_fw_size;
for (i = 0; i < fw_size; i++)
I915_WRITE(CSR_PROGRAM_BASE + i * 4,
@@ -262,14 +212,11 @@ void intel_csr_load_program(struct drm_device *dev)
I915_WRITE(dev_priv->csr.mmioaddr[i],
dev_priv->csr.mmiodata[i]);
}
-
- dev_priv->csr.state = FW_LOADED;
- mutex_unlock(&dev_priv->csr_lock);
}
-static void finish_csr_load(const struct firmware *fw, void *context)
+static void finish_csr_load(const struct firmware *fw,
+ struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = context;
struct drm_device *dev = dev_priv->dev;
struct intel_css_header *css_header;
struct intel_package_header *package_header;
@@ -280,16 +227,15 @@ static void finish_csr_load(const struct firmware *fw, void *context)
uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
uint32_t i;
__be32 *dmc_payload;
- bool fw_loaded = false;
if (!fw) {
i915_firmware_load_error_print(csr->fw_path, 0);
- goto out;
+ return;
}
if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
DRM_ERROR("Unknown stepping info, firmware loading failed\n");
- goto out;
+ return;
}
/* Extract CSS Header information*/
@@ -298,7 +244,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
(css_header->header_len * 4)) {
DRM_ERROR("Firmware has wrong CSS header length %u bytes\n",
(css_header->header_len * 4));
- goto out;
+ return;
}
readcount += sizeof(struct intel_css_header);
@@ -309,7 +255,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
(package_header->header_len * 4)) {
DRM_ERROR("Firmware has wrong package header length %u bytes\n",
(package_header->header_len * 4));
- goto out;
+ return;
}
readcount += sizeof(struct intel_package_header);
@@ -329,7 +275,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
}
if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
DRM_ERROR("Firmware not supported for %c stepping\n", stepping);
- goto out;
+ return;
}
readcount += dmc_offset;
@@ -338,7 +284,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
DRM_ERROR("Firmware has wrong dmc header length %u bytes\n",
(dmc_header->header_len));
- goto out;
+ return;
}
readcount += sizeof(struct intel_dmc_header);
@@ -346,7 +292,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
DRM_ERROR("Firmware has wrong mmio count %u\n",
dmc_header->mmio_count);
- goto out;
+ return;
}
csr->mmio_count = dmc_header->mmio_count;
for (i = 0; i < dmc_header->mmio_count; i++) {
@@ -354,7 +300,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
DRM_ERROR(" Firmware has wrong mmio address 0x%x\n",
dmc_header->mmioaddr[i]);
- goto out;
+ return;
}
csr->mmioaddr[i] = dmc_header->mmioaddr[i];
csr->mmiodata[i] = dmc_header->mmiodata[i];
@@ -364,14 +310,14 @@ static void finish_csr_load(const struct firmware *fw, void *context)
nbytes = dmc_header->fw_size * 4;
if (nbytes > CSR_MAX_FW_SIZE) {
DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes);
- goto out;
+ return;
}
csr->dmc_fw_size = dmc_header->fw_size;
csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL);
if (!csr->dmc_payload) {
DRM_ERROR("Memory allocation failed for dmc payload\n");
- goto out;
+ return;
}
dmc_payload = csr->dmc_payload;
@@ -387,16 +333,42 @@ static void finish_csr_load(const struct firmware *fw, void *context)
/* load csr program during system boot, as needed for DC states */
intel_csr_load_program(dev);
- fw_loaded = true;
+ dev_priv->csr.dmc_present = true;
DRM_DEBUG_KMS("Finished loading %s\n", dev_priv->csr.fw_path);
-out:
- if (fw_loaded)
- intel_runtime_pm_put(dev_priv);
- else
- intel_csr_load_status_set(dev_priv, FW_FAILED);
+}
- release_firmware(fw);
+/**
+ * intel_display_load_csr() - write the firmware from memory to register.
+ * @dev: drm device.
+ *
+ * CSR firmware is read from a .bin file and kept in internal memory one time.
+ * Everytime display comes back from low power state this function is called to
+ * copy the firmware to registers.
+ */
+
+void intel_display_load_csr(struct drm_i915_private *dev_priv)
+{
+ struct intel_csr *csr = &dev_priv->csr;
+ const struct firmware *fw;
+ int ret;
+
+ if (dev_priv->csr.dmc_present)
+ intel_csr_load_program(dev_priv->dev);
+ else {
+ /* CSR supported for platform, load firmware */
+ ret = request_firmware(&fw, csr->fw_path,
+ &dev_priv->dev->pdev->dev);
+
+ DRM_DEBUG_KMS("Loading %d\n", ret);
+
+ if (ret) {
+ i915_firmware_load_error_print(csr->fw_path, ret);
+ return;
+ }
+ finish_csr_load(fw, dev_priv);
+ release_firmware(fw);
+ }
}
/**
@@ -410,7 +382,6 @@ void intel_csr_ucode_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_csr *csr = &dev_priv->csr;
- int ret;
if (!HAS_CSR(dev))
return;
@@ -419,27 +390,12 @@ void intel_csr_ucode_init(struct drm_device *dev)
csr->fw_path = I915_CSR_SKL;
else {
DRM_ERROR("Unexpected: no known CSR firmware for platform\n");
- intel_csr_load_status_set(dev_priv, FW_FAILED);
return;
}
DRM_DEBUG_KMS("Loading %s\n", csr->fw_path);
- /*
- * Obtain a runtime pm reference, until CSR is loaded,
- * to avoid entering runtime-suspend.
- */
- intel_runtime_pm_get(dev_priv);
-
- /* CSR supported for platform, load firmware */
- ret = request_firmware_nowait(THIS_MODULE, true, csr->fw_path,
- &dev_priv->dev->pdev->dev,
- GFP_KERNEL, dev_priv,
- finish_csr_load);
- if (ret) {
- i915_firmware_load_error_print(csr->fw_path, ret);
- intel_csr_load_status_set(dev_priv, FW_FAILED);
- }
+ INIT_WORK(&csr->csr_work, intel_csr_setdc_work_fn);
}
/**
@@ -456,14 +412,13 @@ void intel_csr_ucode_fini(struct drm_device *dev)
if (!HAS_CSR(dev))
return;
- intel_csr_load_status_set(dev_priv, FW_FAILED);
+ flush_work(&dev_priv->csr.csr_work);
+
kfree(dev_priv->csr.dmc_payload);
}
void assert_csr_loaded(struct drm_i915_private *dev_priv)
{
- WARN(intel_csr_load_status_get(dev_priv) != FW_LOADED,
- "CSR is not loaded.\n");
WARN(!I915_READ(CSR_PROGRAM_BASE),
"CSR program storage start is NULL\n");
WARN(!I915_READ(CSR_SSP_BASE), "CSR SSP Base Not fine\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3f0a890..b427407 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1163,10 +1163,7 @@ u32 skl_plane_ctl_rotation(unsigned int rotation);
/* intel_csr.c */
void intel_csr_ucode_init(struct drm_device *dev);
-enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv);
-void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
- enum csr_state state);
-void intel_csr_load_program(struct drm_device *dev);
+void intel_display_load_csr(struct drm_i915_private *dev_priv);
void intel_csr_ucode_fini(struct drm_device *dev);
void assert_csr_loaded(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1a45385..61c018d 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -527,7 +527,6 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
if (dev_priv->power_domains.initializing)
return;
- assert_csr_loaded(dev_priv);
WARN(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6),
"DC6 already programmed to be disabled.\n");
}
@@ -563,6 +562,37 @@ static void skl_disable_dc6(struct drm_i915_private *dev_priv)
POSTING_READ(DC_STATE_EN);
}
+void intel_csr_setdc_work_fn(struct work_struct *__work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(__work, struct drm_i915_private, csr.csr_work);
+ struct intel_csr *csr = &dev_priv->csr;
+
+ if (csr->dc_state_req) {
+ intel_display_load_csr(dev_priv);
+
+ if (IS_SKYLAKE(dev_priv->dev))
+ skl_enable_dc6(dev_priv);
+ else
+ gen9_enable_dc5(dev_priv);
+ } else {
+ if (IS_SKYLAKE(dev_priv->dev)) {
+ skl_disable_dc6(dev_priv);
+ /*
+ * DDI buffer programming unnecessary during
+ * driver-load/resume as it's already done during
+ * modeset initialization then. It's also invalid
+ * here as encoder list is still uninitialized.
+ */
+ if (!dev_priv->power_domains.initializing)
+ intel_prepare_ddi(dev_priv->dev);
+ } else {
+ gen9_disable_dc5(dev_priv);
+ }
+ }
+}
+
+
static void skl_set_power_well(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well, bool enable)
{
@@ -612,18 +642,8 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
when request is to disable!\n");
if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
power_well->data == SKL_DISP_PW_2) {
- if (SKL_ENABLE_DC6(dev)) {
- skl_disable_dc6(dev_priv);
- /*
- * DDI buffer programming unnecessary during driver-load/resume
- * as it's already done during modeset initialization then.
- * It's also invalid here as encoder list is still uninitialized.
- */
- if (!dev_priv->power_domains.initializing)
- intel_prepare_ddi(dev);
- } else {
- gen9_disable_dc5(dev_priv);
- }
+ dev_priv->csr.dc_state_req = false;
+ schedule_work(&dev_priv->csr.csr_work);
}
I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
}
@@ -644,21 +664,8 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
power_well->data == SKL_DISP_PW_2) {
- enum csr_state state;
- /* TODO: wait for a completion event or
- * similar here instead of busy
- * waiting using wait_for function.
- */
- wait_for((state = intel_csr_load_status_get(dev_priv)) !=
- FW_UNINITIALIZED, 1000);
- if (state != FW_LOADED)
- DRM_ERROR("CSR firmware not ready (%d)\n",
- state);
- else
- if (SKL_ENABLE_DC6(dev))
- skl_enable_dc6(dev_priv);
- else
- gen9_enable_dc5(dev_priv);
+ dev_priv->csr.dc_state_req = true;
+ schedule_work(&dev_priv->csr.csr_work);
}
}
}
--
2.0.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
