Re: [Intel-gfx] [PATCH 3/8] drm/i915/skl: Add DC5 Trigger Sequence.
On Wed, 2015-04-01 at 16:22 +0530, Animesh Manna wrote: From: Suketu Shah suketu.j.s...@intel.com Add triggers as per expectations mentioned in gen9_enable_dc5 and gen9_disable_dc5 patch. Also call POSTING_READ for every write to a register to ensure that its written immediately. v1: Remove POSTING_READ calls as they've already been added in previous patches. v2: Rebase to move all runtime pm specific changes to intel_runtime_pm.c file. Modified as per review comments from Imre: 1] Change variable name 'dc5_allowed' to 'dc5_enabled' to correspond to relevant functions. 2] Move the check dc5_enabled in skl_set_power_well() to disable DC5 into gen9_disable_DC5 which is a more appropriate place. 3] Convert checks for 'pm.dc5_enabled' and 'pm.suspended' in skl_set_power_well() to warnings. However, removing them for now as they'll be included in a future patch asserting DC-state entry/exit criteria. 4] Enable DC5, only when CSR firmware is verified to be loaded. Create new structure to track 'enabled' and 'deferred' status of DC5. 5] Ensure runtime PM reference is obtained, if CSR is not loaded, to avoid entering runtime-suspend and release it when it's loaded. 6] Protect necessary CSR-related code with locks. 7] Move CSR-loading call to runtime PM initialization, as power domains needed to be accessed during deferred DC5-enabling, are not initialized earlier. v3: Rebase to latest. Modified as per review comments from Imre: 1] Use blocking wait for CSR-loading to finish to enable DC5 for simplicity, instead of deferring enabling DC5 until CSR is loaded. 2] Obtain runtime PM reference during CSR-loading initialization itself as deferred DC5- enabling is removed and release it at the end of CSR-loading functionality. 3] Revert calling CSR-loading functionality to the beginning of i915 driver-load functionality to avoid any delay in loading. 4] Define another variable to track whether CSR-loading failed and use it to avoid enabling DC5 if it's true. 5] Define CSR-load-status accessor functions for use later. v4: 1] Disable DC5 before enabling PG2 instead of after it. 2] DC5 was being mistaken enabled even when CSR-loading timed-out. Fix that. 3] Enable DC5-related functionality using a macro. 4] Remove dc5_enabled tracking variable and its use as it's not needed now. v5: 1] Mark CSR failed to load where necessary in finish_csr_load function. 2] Use mutex-protected accessor function to check if CSR loaded instead of directly accessing the variable. 3] Prefix csr_load_status_get/set function names with intel_. v6: rebase to latest. v7: Rebase on top of nightly (Damien) v8: Squashed the patch from Imre - added csr helper pointers to simplify the code. (Imre) v9: After adding dmc ver 1.0 support rebased on top of nightly. (Animesh) Issue: VIZ-2819 Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com Signed-off-by: Suketu Shah suketu.j.s...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Imre Deak imre.d...@intel.com Signed-off-by: Animesh Manna animesh.ma...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_csr.c| 50 - drivers/gpu/drm/i915/intel_drv.h| 2 ++ drivers/gpu/drm/i915/intel_runtime_pm.c | 31 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index dd572a0..3320fb4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -669,6 +669,8 @@ struct intel_uncore { struct intel_csr { int csr_size; u8 *csr_buf; + bool loaded; + bool failed; Nitpick: just using an enum with loading, loaded, failed states would be clearer. const char *fw_path; }; diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index f44f1cd..87d393a 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -60,6 +60,25 @@ char intel_get_substepping(struct drm_device *dev) else return -ENODATA; } + +bool intel_csr_load_status_get(struct drm_i915_private *dev_priv) +{ + bool val = false; + + mutex_lock(dev_priv-csr_lock); + val = dev_priv-csr.loaded; + mutex_unlock(dev_priv-csr_lock); + + return val; +} + +void intel_csr_load_status_set(struct drm_i915_private *dev_priv, bool val) +{ + mutex_lock(dev_priv-csr_lock); + dev_priv-csr.loaded = val; + mutex_unlock(dev_priv-csr_lock); +} + void intel_csr_load_program(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; @@ -91,6 +110,8 @@ void intel_csr_load_program(struct drm_device *dev) I915_WRITE(dev_priv-dmc_info.mmioaddr[i],
[Intel-gfx] [PATCH 3/8] drm/i915/skl: Add DC5 Trigger Sequence.
From: Suketu Shah suketu.j.s...@intel.com Add triggers as per expectations mentioned in gen9_enable_dc5 and gen9_disable_dc5 patch. Also call POSTING_READ for every write to a register to ensure that its written immediately. v1: Remove POSTING_READ calls as they've already been added in previous patches. v2: Rebase to move all runtime pm specific changes to intel_runtime_pm.c file. Modified as per review comments from Imre: 1] Change variable name 'dc5_allowed' to 'dc5_enabled' to correspond to relevant functions. 2] Move the check dc5_enabled in skl_set_power_well() to disable DC5 into gen9_disable_DC5 which is a more appropriate place. 3] Convert checks for 'pm.dc5_enabled' and 'pm.suspended' in skl_set_power_well() to warnings. However, removing them for now as they'll be included in a future patch asserting DC-state entry/exit criteria. 4] Enable DC5, only when CSR firmware is verified to be loaded. Create new structure to track 'enabled' and 'deferred' status of DC5. 5] Ensure runtime PM reference is obtained, if CSR is not loaded, to avoid entering runtime-suspend and release it when it's loaded. 6] Protect necessary CSR-related code with locks. 7] Move CSR-loading call to runtime PM initialization, as power domains needed to be accessed during deferred DC5-enabling, are not initialized earlier. v3: Rebase to latest. Modified as per review comments from Imre: 1] Use blocking wait for CSR-loading to finish to enable DC5 for simplicity, instead of deferring enabling DC5 until CSR is loaded. 2] Obtain runtime PM reference during CSR-loading initialization itself as deferred DC5- enabling is removed and release it at the end of CSR-loading functionality. 3] Revert calling CSR-loading functionality to the beginning of i915 driver-load functionality to avoid any delay in loading. 4] Define another variable to track whether CSR-loading failed and use it to avoid enabling DC5 if it's true. 5] Define CSR-load-status accessor functions for use later. v4: 1] Disable DC5 before enabling PG2 instead of after it. 2] DC5 was being mistaken enabled even when CSR-loading timed-out. Fix that. 3] Enable DC5-related functionality using a macro. 4] Remove dc5_enabled tracking variable and its use as it's not needed now. v5: 1] Mark CSR failed to load where necessary in finish_csr_load function. 2] Use mutex-protected accessor function to check if CSR loaded instead of directly accessing the variable. 3] Prefix csr_load_status_get/set function names with intel_. v6: rebase to latest. v7: Rebase on top of nightly (Damien) v8: Squashed the patch from Imre - added csr helper pointers to simplify the code. (Imre) v9: After adding dmc ver 1.0 support rebased on top of nightly. (Animesh) Issue: VIZ-2819 Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com Signed-off-by: Suketu Shah suketu.j.s...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Animesh Manna animesh.ma...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_csr.c| 50 - drivers/gpu/drm/i915/intel_drv.h| 2 ++ drivers/gpu/drm/i915/intel_runtime_pm.c | 31 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index dd572a0..3320fb4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -669,6 +669,8 @@ struct intel_uncore { struct intel_csr { int csr_size; u8 *csr_buf; + bool loaded; + bool failed; const char *fw_path; }; diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index f44f1cd..87d393a 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -60,6 +60,25 @@ char intel_get_substepping(struct drm_device *dev) else return -ENODATA; } + +bool intel_csr_load_status_get(struct drm_i915_private *dev_priv) +{ + bool val = false; + + mutex_lock(dev_priv-csr_lock); + val = dev_priv-csr.loaded; + mutex_unlock(dev_priv-csr_lock); + + return val; +} + +void intel_csr_load_status_set(struct drm_i915_private *dev_priv, bool val) +{ + mutex_lock(dev_priv-csr_lock); + dev_priv-csr.loaded = val; + mutex_unlock(dev_priv-csr_lock); +} + void intel_csr_load_program(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; @@ -91,6 +110,8 @@ void intel_csr_load_program(struct drm_device *dev) I915_WRITE(dev_priv-dmc_info.mmioaddr[i], dev_priv-dmc_info.mmiodata[i]); } + + dev_priv-csr.loaded = true; mutex_unlock(dev_priv-csr_lock); } @@ -110,11 +131,13 @@ static void finish_csr_load(const struct firmware *fw, void *context) if (!fw) { i915_firmware_load_error_print(csr-fw_path, 0); +
[Intel-gfx] [PATCH 3/8] drm/i915/skl: Add DC5 Trigger Sequence.
From: Suketu Shah suketu.j.s...@intel.com Add triggers as per expectations mentioned in gen9_enable_dc5 and gen9_disable_dc5 patch. Also call POSTING_READ for every write to a register to ensure that its written immediately. v1: Remove POSTING_READ calls as they've already been added in previous patches. v2: Rebase to move all runtime pm specific changes to intel_runtime_pm.c file. Modified as per review comments from Imre: 1] Change variable name 'dc5_allowed' to 'dc5_enabled' to correspond to relevant functions. 2] Move the check dc5_enabled in skl_set_power_well() to disable DC5 into gen9_disable_DC5 which is a more appropriate place. 3] Convert checks for 'pm.dc5_enabled' and 'pm.suspended' in skl_set_power_well() to warnings. However, removing them for now as they'll be included in a future patch asserting DC-state entry/exit criteria. 4] Enable DC5, only when CSR firmware is verified to be loaded. Create new structure to track 'enabled' and 'deferred' status of DC5. 5] Ensure runtime PM reference is obtained, if CSR is not loaded, to avoid entering runtime-suspend and release it when it's loaded. 6] Protect necessary CSR-related code with locks. 7] Move CSR-loading call to runtime PM initialization, as power domains needed to be accessed during deferred DC5-enabling, are not initialized earlier. v3: Rebase to latest. Modified as per review comments from Imre: 1] Use blocking wait for CSR-loading to finish to enable DC5 for simplicity, instead of deferring enabling DC5 until CSR is loaded. 2] Obtain runtime PM reference during CSR-loading initialization itself as deferred DC5- enabling is removed and release it at the end of CSR-loading functionality. 3] Revert calling CSR-loading functionality to the beginning of i915 driver-load functionality to avoid any delay in loading. 4] Define another variable to track whether CSR-loading failed and use it to avoid enabling DC5 if it's true. 5] Define CSR-load-status accessor functions for use later. v4: 1] Disable DC5 before enabling PG2 instead of after it. 2] DC5 was being mistaken enabled even when CSR-loading timed-out. Fix that. 3] Enable DC5-related functionality using a macro. 4] Remove dc5_enabled tracking variable and its use as it's not needed now. v5: 1] Mark CSR failed to load where necessary in finish_csr_load function. 2] Use mutex-protected accessor function to check if CSR loaded instead of directly accessing the variable. 3] Prefix csr_load_status_get/set function names with intel_. v6: rebase to latest. v7: Rebase on top of nightly (Damien) v8: Squashed the patch from Imre - added csr helper pointers to simplify the code. (Imre) v9: After adding dmc ver 1.0 support rebased on top of nightly. (Animesh) Issue: VIZ-2819 Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com Signed-off-by: Suketu Shah suketu.j.s...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Imre Deak imre.d...@intel.com Signed-off-by: Animesh Manna animesh.ma...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_csr.c| 50 - drivers/gpu/drm/i915/intel_drv.h| 2 ++ drivers/gpu/drm/i915/intel_runtime_pm.c | 31 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index dd572a0..3320fb4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -669,6 +669,8 @@ struct intel_uncore { struct intel_csr { int csr_size; u8 *csr_buf; + bool loaded; + bool failed; const char *fw_path; }; diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index f44f1cd..87d393a 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -60,6 +60,25 @@ char intel_get_substepping(struct drm_device *dev) else return -ENODATA; } + +bool intel_csr_load_status_get(struct drm_i915_private *dev_priv) +{ + bool val = false; + + mutex_lock(dev_priv-csr_lock); + val = dev_priv-csr.loaded; + mutex_unlock(dev_priv-csr_lock); + + return val; +} + +void intel_csr_load_status_set(struct drm_i915_private *dev_priv, bool val) +{ + mutex_lock(dev_priv-csr_lock); + dev_priv-csr.loaded = val; + mutex_unlock(dev_priv-csr_lock); +} + void intel_csr_load_program(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; @@ -91,6 +110,8 @@ void intel_csr_load_program(struct drm_device *dev) I915_WRITE(dev_priv-dmc_info.mmioaddr[i], dev_priv-dmc_info.mmiodata[i]); } + + dev_priv-csr.loaded = true; mutex_unlock(dev_priv-csr_lock); } @@ -110,11 +131,13 @@ static void finish_csr_load(const struct firmware *fw, void *context) if (!fw) {