[Intel-gfx] [PATCH] drm/i915: Use num_scalers instead of SKL_NUM_SCALERS in debugfs

2016-11-20 Thread sunil . kamath 
From: "A.Sunil Kamath" 

Better to use num_scaler itself while printing scaler_info.
This fixes a bug of printing information for the missing
second scaler on pipe C for SKL platform.

Signed-off-by: A.Sunil Kamath 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 1cc971c..6c39881 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3081,7 +3081,7 @@ static void intel_scaler_info(struct seq_file *m, struct 
intel_crtc *intel_crtc)
   pipe_config->scaler_state.scaler_users,
   pipe_config->scaler_state.scaler_id);
 
-   for (i = 0; i < SKL_NUM_SCALERS; i++) {
+   for (i = 0; i < num_scalers; i++) {
struct intel_scaler *sc =
_config->scaler_state.scalers[i];
 
-- 
2.8.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 01/13] drm/i915/gen9: csr_init after runtime pm enable

2015-11-04 Thread Sunil Kamath 

On Thursday 29 October 2015 07:25 PM, Imre Deak wrote:

On to, 2015-10-29 at 15:48 +0530, Sunil Kamath wrote:

On Thursday 29 October 2015 03:28 AM, Imre Deak wrote:

From: Animesh Manna <animesh.ma...@intel.com>

Skl is fully dependent on dmc for going to low power state (dc5/dc6).
This requires a trigger from rpm. To ensure the dmc firmware
is available for runtime pm support rpm-reference-count is used
by not releasing the rpm reference if firmware loading is
not completed.

So moved the intel_csr_ucode_init call after runtime pm enable.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Damien Lespiau <damien.lesp...@intel.com>
Cc: Imre Deak <imre.d...@intel.com>
Cc: Sunil Kamath <sunil.kam...@intel.com>
Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
[imre: moved the call right after power domain init to avoid race with
   the console modesetting]
---
   drivers/gpu/drm/i915/i915_dma.c | 5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2336af9..5d68942 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -398,6 +398,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
   
   	intel_power_domains_init_hw(dev_priv);
   
+	intel_csr_ucode_init(dev);

+

Its really unclear why this call to be done here.
We need to call just after intel_runtime_pm_enable.
With this change it will do much in advance.

In general the earlier we start the firmware loading work the better,
since we can start saving power the earliest this way.


Can you please add some more details about "how adding change after
iniet_runtime_pm_enable() will create race condition for console
modesetting?"

We want to avoid to call into either the power well code or the runtime
s/r handlers. This is why we take a ref on the INIT power domain in
intel_csr_ucode_init() which should prevent both. Now if we do this only
after the point where a power well reference can drop to zero, there is
a chance that we call into the power well code before we took the above
INIT power domain ref.

If you look at i915_load_modeset_init(), it schedules
intel_fbdev_initial_config(), which may end up doing a modeset for the
console, leading up to intel_modeset_setup_hw_state() and
intel_display_set_init_power(false). Depending on the the active outputs
this may drop the last reference on some domains and call into the power
well code.

Due to similar reasons we could drop the last runtime PM reference and
call the runtime s/r handlers before the firmware is loaded if we took
the INIT power domain ref only after calling intel_runtime_pm_enable().

--Imre


- Sunil



Concern that we had was:

In intel_csr_ucode_init() we are calling 
intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); which intern call 
intel_runtime_pm_get. That means rpm will not be enabled before this 
code flow.


Now it’s clear that that's not a problem. As we do RPM gets even 
earlier. It will only drop an RPM reference, that in effect will make it 
possible to enter RPM. There are places taking an RPM ref already 
earlier like in intel_power_domains_init_hw() and RPM references are 
taken even before our driver's probe runtime is called by the pci core. 
So that's completely normal.


Reviewed-by: A.Sunil Kamath <sunil.kam...@intel.com> 
<mailto:sunil.kam...@intel.com>



ret = intel_irq_install(dev_priv);
if (ret)
goto cleanup_gem_stolen;
@@ -942,9 +944,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long 
flags)
   
   	intel_uncore_init(dev);
   
-	/* Load CSR Firmware for SKL */

-   intel_csr_ucode_init(dev);
-
ret = i915_gem_gtt_init(dev);
if (ret)
goto out_freecsr;




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 01/13] drm/i915/gen9: csr_init after runtime pm enable

2015-10-29 Thread Sunil Kamath 

On Thursday 29 October 2015 03:28 AM, Imre Deak wrote:

From: Animesh Manna <animesh.ma...@intel.com>

Skl is fully dependent on dmc for going to low power state (dc5/dc6).
This requires a trigger from rpm. To ensure the dmc firmware
is available for runtime pm support rpm-reference-count is used
by not releasing the rpm reference if firmware loading is
not completed.

So moved the intel_csr_ucode_init call after runtime pm enable.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Damien Lespiau <damien.lesp...@intel.com>
Cc: Imre Deak <imre.d...@intel.com>
Cc: Sunil Kamath <sunil.kam...@intel.com>
Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
[imre: moved the call right after power domain init to avoid race with
  the console modesetting]
---
  drivers/gpu/drm/i915/i915_dma.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2336af9..5d68942 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -398,6 +398,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
  
  	intel_power_domains_init_hw(dev_priv);
  
+	intel_csr_ucode_init(dev);

+


Its really unclear why this call to be done here.
We need to call just after intel_runtime_pm_enable.
With this change it will do much in advance.

Can you please add some more details about "how adding change after 
iniet_runtime_pm_enable() will create race condition for console 
modesetting?"


- Sunil



ret = intel_irq_install(dev_priv);
if (ret)
goto cleanup_gem_stolen;
@@ -942,9 +944,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long 
flags)
  
  	intel_uncore_init(dev);
  
-	/* Load CSR Firmware for SKL */

-   intel_csr_ucode_init(dev);
-
ret = i915_gem_gtt_init(dev);
if (ret)
goto out_freecsr;


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [DMC_BUGFIX_SKL_V2 3/5] drm/i915/skl: Making DC6 entry is the last call in suspend flow.

2015-09-07 Thread Sunil Kamath 

On Wednesday 26 August 2015 01:36 AM, Animesh Manna wrote:

Mmio register access after dc6/dc5 entry is not allowed when
DC6 power states are enabled according to bspec (bspec-id 0527),
so enabling dc6 as the last call in suspend flow.

v1: Initial version.

v2: Based on review comment from Daniel,
- created a seperate patch for csr uninitialization set call.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Damien Lespiau <damien.lesp...@intel.com>
Cc: Imre Deak <imre.d...@intel.com>
Cc: Sunil Kamath <sunil.kam...@intel.com>
Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagar...@intel.com>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
---
  drivers/gpu/drm/i915/i915_drv.c | 13 +
  drivers/gpu/drm/i915/intel_drv.h|  2 ++
  drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++
  3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 478101c..fa66162 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1013,10 +1013,20 @@ static int i915_pm_resume(struct device *dev)
  
  static int skl_suspend_complete(struct drm_i915_private *dev_priv)

  {
+   enum csr_state state;
/* Enabling DC6 is not a hard requirement to enter runtime D3 */
  
  	skl_uninit_cdclk(dev_priv);
  
+	/* 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)
+   skl_enable_dc6(dev_priv);
+
return 0;
  }
  
@@ -1063,6 +1073,9 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)

  {
struct drm_device *dev = dev_priv->dev;
  
+	if (intel_csr_load_status_get(dev_priv) == FW_LOADED)

+   skl_disable_dc6(dev_priv);
+
skl_init_cdclk(dev_priv);
intel_csr_load_program(dev);
  
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h

index 81b7d77..9cb7d4e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1118,6 +1118,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
  void skl_init_cdclk(struct drm_i915_private *dev_priv);
  void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
+void skl_disable_dc6(struct drm_i915_private *dev_priv);
  void intel_dp_get_m_n(struct intel_crtc *crtc,
  struct intel_crtc_state *pipe_config);
  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 821644d..23a3aa3 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -548,7 +548,7 @@ static void assert_can_disable_dc6(struct drm_i915_private 
*dev_priv)
"DC6 already programmed to be disabled.\n");
  }
  
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)

+void skl_enable_dc6(struct drm_i915_private *dev_priv)
  {
uint32_t val;
  
@@ -565,7 +565,7 @@ static void skl_enable_dc6(struct drm_i915_private *dev_priv)

POSTING_READ(DC_STATE_EN);
  }
  
-static void skl_disable_dc6(struct drm_i915_private *dev_priv)

+void skl_disable_dc6(struct drm_i915_private *dev_priv)
  {
uint32_t val;
  
@@ -626,10 +626,10 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,

!I915_READ(HSW_PWR_WELL_BIOS),
"Invalid for power well status to be enabled, 
unless done by the BIOS, \
when request is to disable!\n");
-   if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
-   power_well->data == SKL_DISP_PW_2) {
+   if (power_well->data == SKL_DISP_PW_2) {
+   if (GEN9_ENABLE_DC5(dev))
+   gen9_disable_dc5(dev_priv);
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.
@@ -637,8 +637,6 @@ static void skl_set_power_well(struct drm_i915_private 
*dev_priv,
 */
if 
(!dev_priv->power_domains.i

Re: [Intel-gfx] [DMC_BUGFIX_SKL_V2 4/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present

2015-09-07 Thread Sunil Kamath 

On Wednesday 26 August 2015 01:36 AM, Animesh Manna wrote:

While display engine entering into low power state no need to disable
cdclk pll as CSR firmware of dmc will take care. If pll is already
enabled firmware execution sequence will be blocked. This is one
of the criteria for dmc to work properly.

v1: Initial version.

v2: Based on review comment from Daniel added code commnent.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Damien Lespiau <damien.lesp...@intel.com>
Cc: Imre Deak <imre.d...@intel.com>
Cc: Sunil Kamath <sunil.kam...@intel.com>
Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
Signed-off-bt: Vathsala Nagaraju <vathsala.nagar...@intel.com>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhard...@intel.com>
---
  drivers/gpu/drm/i915/intel_display.c | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f604ce1..b6bef20 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5687,10 +5687,16 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
DRM_ERROR("DBuf power disable timeout\n");
  
-	/* disable DPLL0 */

-   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
-   if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
-   DRM_ERROR("Couldn't disable DPLL0\n");
+   /*
+* DMC assumes ownership of LCPLL and will get confused if we touch it.
+*/
+   if (dev_priv->csr.dmc_payload) {
+   /* disable DPLL0 */
+   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
+   ~LCPLL_PLL_ENABLE);
+   if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
+   DRM_ERROR("Couldn't disable DPLL0\n");
+   }
  
  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);

  }


Valid fix and patch is ready for merge now.

Reviewed-by: A.Sunil Kamath <sunil.kam...@intel.com> 
<mailto:sunil.kam...@intel.com>


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [DMC_BUGFIX_SKL_V2 1/5] drm/i915/skl: Added a check for the hardware status of csr fw before loading.

2015-09-07 Thread Sunil Kamath 

On Wednesday 26 August 2015 01:36 AM, Animesh Manna wrote:

Dmc will restore the csr program except DC9, cold boot,
warm reset, PCI function level reset, and hibernate/suspend.

intel_csr_load_program() function is used to load the firmware
data from kernel memory to csr address space.

All values of csr address space will be zero if it got reset and
the first byte of csr program is always a non-zero if firmware
is loaded successfuly. Based on hardware status will load the
firmware.

Without this condition check if we overwrite the firmware data the
counters exposed for dc5/dc6 (help for debugging) will be nullified.

v1: Initial version.

v2: Based on review comments from Daniel,
- Added a check to know hardware status and load the firmware if not loaded.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Damien Lespiau <damien.lesp...@intel.com>
Cc: Imre Deak <imre.d...@intel.com>
Cc: Sunil Kamath <sunil.kam...@intel.com>
Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagar...@intel.com>
---
  drivers/gpu/drm/i915/intel_csr.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index ba1ae03..682cc26 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -252,6 +252,15 @@ void intel_csr_load_program(struct drm_device *dev)
return;
}
  
+	/*

+* Dmc will restore the csr the program except DC9, cold boot,
+* warm reset, PCI function level reset, and hibernate/suspend.
+* This condition will help to check if csr address space is reset/
+* not loaded.
+*/
+   if (I915_READ(CSR_PROGRAM_BASE))
+   return;
+
mutex_lock(_priv->csr_lock);
fw_size = dev_priv->csr.dmc_fw_size;
for (i = 0; i < fw_size; i++)


Valid fix and patch is ready for merge now.

Reviewed-by: A.Sunil Kamath <sunil.kam...@intel.com> 
<mailto:sunil.kam...@intel.com>


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [DMC_BUGFIX_SKL_V2 2/5] drm/i915/skl Remove the call for csr uninitialization from suspend path

2015-09-07 Thread Sunil Kamath 

On Wednesday 26 August 2015 01:36 AM, Animesh Manna wrote:

This patch remove the function call to set the firmware
loading status as uninitialized during suspend.

Dmc firmware will restore the firmware in normal suspend. In previous
patch added a check to directly read the hardware status and load
the firmware if got reset during resume from suspend-hibernation.

Cc: Daniel Vetter 
Signed-off-by: Animesh Manna 
Signed-off-by: Vathsala Nagaraju 
---
  drivers/gpu/drm/i915/i915_drv.c | 6 --
  1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1d88745..478101c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1015,12 +1015,6 @@ 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);
-
skl_uninit_cdclk(dev_priv);
  
  	return 0;


Valid fix and patch is ready for merge now.

Reviewed-by: A.Sunil Kamath  



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [DMC_BUGFIX_SKL_V2 5/5] drm/i915/skl: Block disable call for pw1 if dmc firmware is present.

2015-09-07 Thread Sunil Kamath 

On Wednesday 26 August 2015 01:36 AM, Animesh Manna wrote:

Another interesting criteria to work dmc as expected is pw1 to be
enabled by driver and dmc will shut it off in its execution
sequence. If already disabled by driver dmc will get confuse and
behave differently than expected found during pc10 entry issue
for skl.

So berfore we disable power-well 1, added check if dmc firmware is
present and driver will not disable power well 1, but for any reason
if firmware is not present of failed to load we can shut off the
power well 1 which will save some power.

As skl is currently fully dependent on dmc to go in lowest possible
power state (dc6) but the same is not applicable for bxt. Display
engine can enter into dc9 without dmc, hence unblocking disable call.

v1: Initial version.

v2: Rebased as per current patch series.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Damien Lespiau <damien.lesp...@intel.com>
Cc: Imre Deak <imre.d...@intel.com>
Cc: Sunil Kamath <sunil.kam...@intel.com>
Signed-off-by: Animesh Manna <animesh.ma...@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagar...@intel.com>
---
  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 23a3aa3..340f386 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -652,9 +652,15 @@ static void skl_set_power_well(struct drm_i915_private 
*dev_priv,
}
} else {
if (enable_requested) {
-   I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & ~req_mask);
-   POSTING_READ(HSW_PWR_WELL_DRIVER);
-   DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
+   if (IS_SKYLAKE(dev) &&
+   (power_well->data == SKL_DISP_PW_1) &&
+   (intel_csr_load_status_get(dev_priv) == 
FW_LOADED))
+   DRM_DEBUG_KMS("Not Disabling PW1, dmc will 
handle\n");
+   else {
+   I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & 
~req_mask);
+   POSTING_READ(HSW_PWR_WELL_DRIVER);
+   DRM_DEBUG_KMS("Disabling %s\n", 
power_well->name);
+   }
  
  			if (GEN9_ENABLE_DC5(dev) &&

power_well->data == SKL_DISP_PW_2) {


Valid fix and patch is ready for merge now.

Reviewed-by: A.Sunil Kamath <sunil.kam...@intel.com> 
<mailto:sunil.kam...@intel.com>


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [SKL-DMC-BUGFIX 5/5] drm/i915/skl: Removed csr firmware load in resume path

2015-08-04 Thread Sunil Kamath 

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e1d0102..02019e9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1066,13 +1066,10 @@ 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;
-
if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
skl_disable_dc6(dev_priv);
  
  	skl_init_cdclk(dev_priv);

-   intel_csr_load_program(dev);


Same comment as before.
The context save and restore program is reset on cold boot, warm reset, 
PCI function level reset, and hibernate/suspend.


Need valid reason to do this change. If reading DC5/DC6 counters is a 
concern, lets use this as just debug patch.


Dont hurry on this patch.
Need to close on the above opens.

- Sunil
  
  	return 0;

  }


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [SKL-DMC-BUGFIX 1/5] drm/i915/gen9: Removed byte swapping for csr firmware

2015-08-04 Thread Sunil Kamath 

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

This patch contains the changes to remove the byte
swapping logic introduced with old dmc firmware.
While debugging PC10 entry issue for skylake found
with latest dmc firmware version 1.18 without byte
swapping dmc is working fine and able to enter PC10.

v1: Initial version.

v2: Corrected firmware size during memcpy(). (Suggested by Sunil)

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.h  |  2 +-
  drivers/gpu/drm/i915/intel_csr.c | 16 
  2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b94ada9..9d0ee58 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -742,7 +742,7 @@ enum csr_state {
  
  struct intel_csr {

const char *fw_path;
-   __be32 *dmc_payload;
+   uint32_t *dmc_payload;
uint32_t dmc_fw_size;
uint32_t mmio_count;
uint32_t mmioaddr[8];
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 6d8a7bf..ba1ae03 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -244,7 +244,7 @@ void intel_csr_load_status_set(struct drm_i915_private 
*dev_priv,
  void intel_csr_load_program(struct drm_device *dev)
  {
struct drm_i915_private *dev_priv = dev-dev_private;
-   __be32 *payload = dev_priv-csr.dmc_payload;
+   u32 *payload = dev_priv-csr.dmc_payload;
uint32_t i, fw_size;
  
  	if (!IS_GEN9(dev)) {

@@ -256,7 +256,7 @@ void intel_csr_load_program(struct drm_device *dev)
fw_size = dev_priv-csr.dmc_fw_size;
for (i = 0; i  fw_size; i++)
I915_WRITE(CSR_PROGRAM_BASE + i * 4,
-   (u32 __force)payload[i]);
+   payload[i]);
  
  	for (i = 0; i  dev_priv-csr.mmio_count; i++) {

I915_WRITE(dev_priv-csr.mmioaddr[i],
@@ -279,7 +279,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
char substepping = intel_get_substepping(dev);
uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
uint32_t i;
-   __be32 *dmc_payload;
+   uint32_t *dmc_payload;
bool fw_loaded = false;
  
  	if (!fw) {

@@ -375,15 +375,7 @@ static void finish_csr_load(const struct firmware *fw, 
void *context)
}
  
  	dmc_payload = csr-dmc_payload;

-   for (i = 0; i  dmc_header-fw_size; i++) {
-   uint32_t *tmp = (u32 *)fw-data[readcount + i * 4];
-   /*
-* The firmware payload is an array of 32 bit words stored in
-* little-endian format in the firmware image and programmed
-* as 32 bit big-endian format to memory.
-*/
-   dmc_payload[i] = cpu_to_be32(*tmp);
-   }
+   memcpy(dmc_payload, fw-data[readcount], nbytes);
  
  	/* load csr program during system boot, as needed for DC states */

intel_csr_load_program(dev);

Thanks for addressing review comments.
Small change can be done in Gerrit message to generalize the issue to fw 
1.0+. Else fine.


Valid bug fix.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [SKL-DMC-BUGFIX 2/5] drm/i915/skl: Making DC6 entry is the last call in suspend flow.

2015-08-04 Thread Sunil Kamath 

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

Mmio register access after dc6/dc5 entry is not allowed when
DC6 power states are enabled according to bspec (bspec-id 0527),
so enabling dc6 as the last call in suspend flow.

v1: Initial version.

v2: commit message updated based on comment from Vathsala.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
Signed-off-by: Rajneesh Bhardwaj rajneesh.bhard...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c | 12 ++--
  drivers/gpu/drm/i915/intel_drv.h|  2 ++
  drivers/gpu/drm/i915/intel_runtime_pm.c | 33 -
  3 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0d6775a..e1d0102 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1017,14 +1017,11 @@ 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);
-
skl_uninit_cdclk(dev_priv);
  
+	if (intel_csr_load_status_get(dev_priv) == FW_LOADED)

+   skl_enable_dc6(dev_priv);
+
return 0;
  }
  
@@ -1071,6 +1068,9 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)

  {
struct drm_device *dev = dev_priv-dev;
  
+	if (intel_csr_load_status_get(dev_priv) == FW_LOADED)

+   skl_disable_dc6(dev_priv);
+
skl_init_cdclk(dev_priv);
intel_csr_load_program(dev);
  
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h

index 47cef0e..06f346f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1117,6 +1117,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
  void skl_init_cdclk(struct drm_i915_private *dev_priv);
  void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
+void skl_disable_dc6(struct drm_i915_private *dev_priv);
  void intel_dp_get_m_n(struct intel_crtc *crtc,
  struct intel_crtc_state *pipe_config);
  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6393b76..c660245 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -532,7 +532,7 @@ static void assert_can_disable_dc6(struct drm_i915_private 
*dev_priv)
DC6 already programmed to be disabled.\n);
  }
  
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)

+void skl_enable_dc6(struct drm_i915_private *dev_priv)
  {
uint32_t val;
  
@@ -549,7 +549,7 @@ static void skl_enable_dc6(struct drm_i915_private *dev_priv)

POSTING_READ(DC_STATE_EN);
  }
  
-static void skl_disable_dc6(struct drm_i915_private *dev_priv)

+void skl_disable_dc6(struct drm_i915_private *dev_priv)
  {
uint32_t val;
  
@@ -610,10 +610,10 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,

!I915_READ(HSW_PWR_WELL_BIOS),
Invalid for power well status to be enabled, 
unless done by the BIOS, \
when request is to disable!\n);
-   if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) 
-   power_well-data == SKL_DISP_PW_2) {
+   if (power_well-data == SKL_DISP_PW_2) {
+   if (GEN9_ENABLE_DC5(dev))
+   gen9_disable_dc5(dev_priv);
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.
@@ -621,8 +621,6 @@ static void skl_set_power_well(struct drm_i915_private 
*dev_priv,
 */
if 
(!dev_priv-power_domains.initializing)
intel_prepare_ddi(dev);
-   } else {
-   gen9_disable_dc5(dev_priv

Re: [Intel-gfx] [SKL-DMC-BUGFIX 4/5] drm/i915/skl: Block disable call for pw1 if dmc firmware is present.

2015-08-04 Thread Sunil Kamath 

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

Another interesting criteria to work dmc as expected is pw1 to be
enabled by driver and dmc will shut it off in its execution
sequence. If already disabled by driver dmc will get confuse and
behave differently than expected found during pc10 entry issue
for skl.

So berfore we disable power-well 1, added check if dmc firmware is
present and driver will not disable power well 1, but for any reason
if firmware is not present of failed to load we can shut off the
power well 1 which will save some power.

As skl is currently fully dependent on dmc to go in lowest possible
power state (dc6) but the same is not applicable for bxt. Display
engine can enter into dc9 without dmc, hence unblocking disable call.

v1: Initial version.

v2: Based on revire commnents from Sunil,
- condition check for pw1 is moved in skl_set_power_well.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index c660245..00cd4ff 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -636,9 +636,15 @@ static void skl_set_power_well(struct drm_i915_private 
*dev_priv,
}
} else {
if (enable_requested) {
-   I915_WRITE(HSW_PWR_WELL_DRIVER, tmp  ~req_mask);
-   POSTING_READ(HSW_PWR_WELL_DRIVER);
-   DRM_DEBUG_KMS(Disabling %s\n, power_well-name);
+   if (IS_SKYLAKE(dev) 
+   (power_well-data == SKL_DISP_PW_1) 
+   (intel_csr_load_status_get(dev_priv) == 
FW_LOADED))
+   DRM_DEBUG_KMS(Not Disabling PW1, dmc will 
handle\n);
+   else {
+   I915_WRITE(HSW_PWR_WELL_DRIVER, tmp  
~req_mask);
+   POSTING_READ(HSW_PWR_WELL_DRIVER);
+   DRM_DEBUG_KMS(Disabling %s\n, 
power_well-name);
+   }
  
  			if (GEN9_ENABLE_DC5(dev) 

power_well-data == SKL_DISP_PW_2)

Right bug fix.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [SKL-DMC-BUGFIX 3/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

2015-08-04 Thread Sunil Kamath 

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

While display engine entering into low power state no need to disable
cdclk pll as CSR firmware of dmc will take care. If pll is already
enabled firmware execution sequence will be blocked. This is one
of the criteria for dmc to work properly.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-bt: Vathsala Nagaraju vathsala.nagar...@intel.com
Signed-off-by: Rajneesh Bhardwaj rajneesh.bhard...@intel.com
---
  drivers/gpu/drm/i915/intel_display.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index af0bcfe..ef2ef4d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
if (I915_READ(DBUF_CTL)  DBUF_POWER_STATE)
DRM_ERROR(DBuf power disable timeout\n);
  
-	/* disable DPLL0 */

-   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL)  ~LCPLL_PLL_ENABLE);
-   if (wait_for(!(I915_READ(LCPLL1_CTL)  LCPLL_PLL_LOCK), 1))
-   DRM_ERROR(Couldn't disable DPLL0\n);
+   if (dev_priv-csr.dmc_payload) {
+   /* disable DPLL0 */
+   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) 
+   ~LCPLL_PLL_ENABLE);
+   if (wait_for(!(I915_READ(LCPLL1_CTL)  LCPLL_PLL_LOCK), 1))
+   DRM_ERROR(Couldn't disable DPLL0\n);
+   }
  
  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);

  }

Right bug fix.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [SKL-DMC-BUGFIX 0/5] SKL PC10 entry fixes.

2015-08-04 Thread Sunil Kamath 

On Tuesday 04 August 2015 12:17 AM, Zanoni, Paulo R wrote:

Em Seg, 2015-08-03 às 21:55 +0530, Animesh Manna escreveu:

The following patches helps to solve PC10 entry issue for SKL.
Detailed description about the changes done to solve the issue
is mentioned in commit message of each patch.

All these patches are send earlier for review as part of Redesign of
dmc firmware loading patch series. Now skl specific bug-fixes are
seperated out and sending as seperate patch series to make it simple.

Hello

I find interesting that even with this series, igt/pm_rpm/rte fails. We
don't seem to be doing runtime PM, yet we allow PC10 somehow. Maybe it
would be nice to have some kind of documentation explaining what's
different on SKL and how things are supposed to work.


Why do you say that rpm is not done for SKL?
Even in SKL its runtime PM that triggers DC6 enable then PKGC10.

Vathsala, Rajneesh,
please add your comments and observations here.

- Sunil



Also, since this feature is not relying on runtime PM, it would be
really nice to elaborate some IGT tests for it. You could even try to
automate PC10 entry checks by reading the appropriate MSR regsiters -
just like we do with PC8 on HSW.

Thanks,
Paulo


Animesh Manna (5):
   drm/i915/gen9: Removed byte swapping for csr firmware
   drm/i915/skl: Making DC6 entry is the last call in suspend flow.
   drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.
   drm/i915/skl: Block disable call for pw1 if dmc firmware is
present.
   drm/i915/skl: Removed csr firmware load in resume path.

  drivers/gpu/drm/i915/i915_drv.c | 13 +-
  drivers/gpu/drm/i915/i915_drv.h |  2 +-
  drivers/gpu/drm/i915/intel_csr.c| 16 +++-
  drivers/gpu/drm/i915/intel_display.c| 11 +---
  drivers/gpu/drm/i915/intel_drv.h|  2 ++
  drivers/gpu/drm/i915/intel_runtime_pm.c | 45 +--
--
  6 files changed, 37 insertions(+), 52 deletions(-)


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 16/18] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

2015-07-30 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

While display engine entering into low power state no need to disable
cdclk pll as CSR firmware of dmc will take care. If pll is already
enabled firmware execution sequence will be blocked. This is one
of the criteria for dmc to work properly.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-bt: Vathsala Nagaraju vathsala.nagar...@intel.com
Signed-off-by: Rajneesh Bhardwaj rajneesh.bhard...@intel.com
---
  drivers/gpu/drm/i915/intel_display.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index af0bcfe..ef2ef4d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
if (I915_READ(DBUF_CTL)  DBUF_POWER_STATE)
DRM_ERROR(DBuf power disable timeout\n);
  
-	/* disable DPLL0 */

-   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL)  ~LCPLL_PLL_ENABLE);
-   if (wait_for(!(I915_READ(LCPLL1_CTL)  LCPLL_PLL_LOCK), 1))
-   DRM_ERROR(Couldn't disable DPLL0\n);
+   if (dev_priv-csr.dmc_payload) {
+   /* disable DPLL0 */
+   I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) 
+   ~LCPLL_PLL_ENABLE);
+   if (wait_for(!(I915_READ(LCPLL1_CTL)  LCPLL_PLL_LOCK), 1))
+   DRM_ERROR(Couldn't disable DPLL0\n);
+   }
  
  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);

  }

This is a right bug fix.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 18/18] drm/i915/gen9: Removed byte swapping for csr firmware.

2015-07-30 Thread Sunil Kamath 

On Tuesday 28 July 2015 04:39 PM, Sunil Kamath wrote:

On Tuesday 28 July 2015 01:38 PM, Nagaraju, Vathsala wrote:

Signed-off-by: Vatsala Nagaraju vatsala.nagr...@intel.com
It's   Vathsala Nagaraju vathsala.nagar...@intel.com

Commit message: Removed byte swapping for csr firmware.

Commit message does not convey as to why the change was made. This 
change is needed as DMC firmware loading failed on skylake due  byte 
swap  done in the driver


yes. agree.

Same review comments as other patches. Bug fixing patches should go 
first in sequence and also commit message should contain relevant info 
on fix/patch.


This byte swapping is not needed at all.

Minimal things to take care.

- Sunil






-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On 
Behalf Of Animesh Manna

Sent: Sunday, July 26, 2015 12:31 AM
To: intel-gfx@lists.freedesktop.org
Cc: Vatsala Nagaraju
Subject: [Intel-gfx] [PATCH 18/18] drm/i915/gen9: Removed byte 
swapping for csr firmware.


Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vatsala Nagaraju vatsala.nagr...@intel.com
---
  drivers/gpu/drm/i915/intel_csr.c | 11 +--
  1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c 
b/drivers/gpu/drm/i915/intel_csr.c

index 1858f02..50779e0 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -328,16 +328,7 @@ static uint32_t *parse_csr_fw(struct 
drm_i915_private *dev_priv,

  return NULL;
  }
  -for (i = 0; i  dmc_header-fw_size; i++) {
-uint32_t *tmp = (u32 *)fw-data[readcount + i * 4];
-/*
- * The firmware payload is an array of 32 bit words stored in
- * little-endian format in the firmware image and programmed
- * as 32 bit big-endian format to memory.
- */
-dmc_payload[i] = (uint32_t __force) cpu_to_be32(*tmp);
-}
-
+memcpy(dmc_payload, fw-data[readcount], dmc_header-fw_size);


*4 missing for byte conversion here.
memcpy(dmc_payload, fw-data[readcount], (dmc_header-fw_size)*4); ?
Still issue to be addressed here to confirm that fw is loaded fine.

- Sunil

  return dmc_payload;
  }




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 17/18] drm/i915/skl: Removed csr firmware load in resume path.

2015-07-30 Thread Sunil Kamath 

On Wednesday 29 July 2015 04:40 PM, Sunil Kamath wrote:

On Tuesday 28 July 2015 04:53 PM, Sunil Kamath wrote:

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c 
b/drivers/gpu/drm/i915/i915_drv.c

index 77b35fd..f5e720b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1048,7 +1048,6 @@ static int skl_resume_prepare(struct 
drm_i915_private *dev_priv)

  skl_disable_dc6(dev_priv);
skl_init_cdclk(dev_priv);
-intel_csr_load_program(dev_priv);


The context save and restore program is reset on cold boot, warm 
reset, PCI function level reset, and hibernate/suspend.


Will it not impact?

- Sunil


If the intention is just to check the DC5/DC6 counters, why cant we 
just add debug prints before f/w reload? Than to just avoiding 
reloading fw itself.


- Sunil


Dont hurry on this patch.
Need to close on the above opens.

- Sunil

return 0;
  }






___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 15/18] drm/i915/skl: Making DC6 entry is the last call in suspend flow.

2015-07-30 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

Mmio register access after dc6/dc5 entry is causing the
system hang, so enabling dc6 as the last call in suspend flow.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
Signed-off-by: Rajneesh Bhardwaj rajneesh.bhard...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c |  6 ++
  drivers/gpu/drm/i915/intel_drv.h|  2 ++
  drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++
  3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ddf8a25..77b35fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -995,6 +995,9 @@ static int i915_pm_resume(struct device *dev)
  
  static int skl_suspend_complete(struct drm_i915_private *dev_priv)

  {
+   if (dev_priv-csr.dmc_payload)
+   skl_enable_dc6(dev_priv);
+
skl_uninit_cdclk(dev_priv);


This is really right change.
With this we will go back to our original design.
Deepest possible display state will be triggered by respective suspend 
complete.
for example in BXT we trigger DC9 from bxt_suspend_complete and works 
fine with rpm integrated.

Same case here for SKL for DC6 which uses DMC.

Right bug fix.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com

  
  	return 0;

@@ -1041,6 +1044,9 @@ static int bxt_resume_prepare(struct drm_i915_private 
*dev_priv)
  
  static int skl_resume_prepare(struct drm_i915_private *dev_priv)

  {
+   if (dev_priv-csr.dmc_payload)
+   skl_disable_dc6(dev_priv);
+
skl_init_cdclk(dev_priv);
intel_csr_load_program(dev_priv);
  
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h

index 644286f..0d13f50 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1117,6 +1117,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
  void skl_init_cdclk(struct drm_i915_private *dev_priv);
  void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
+void skl_disable_dc6(struct drm_i915_private *dev_priv);
  void intel_dp_get_m_n(struct intel_crtc *crtc,
  struct intel_crtc_state *pipe_config);
  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index a5059e8..ddae00e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -540,7 +540,7 @@ static void assert_can_disable_dc6(struct drm_i915_private 
*dev_priv)
DC6 already programmed to be disabled.\n);
  }
  
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)

+void skl_enable_dc6(struct drm_i915_private *dev_priv)
  {
uint32_t val;
  
@@ -557,7 +557,7 @@ static void skl_enable_dc6(struct drm_i915_private *dev_priv)

POSTING_READ(DC_STATE_EN);
  }
  
-static void skl_disable_dc6(struct drm_i915_private *dev_priv)

+void skl_disable_dc6(struct drm_i915_private *dev_priv)
  {
uint32_t val;
  
@@ -618,10 +618,10 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,

!I915_READ(HSW_PWR_WELL_BIOS),
Invalid for power well status to be enabled, 
unless done by the BIOS, \
when request is to disable!\n);
-   if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) 
-   power_well-data == SKL_DISP_PW_2) {
+   if (power_well-data == SKL_DISP_PW_2) {
+   if (GEN9_ENABLE_DC5(dev))
+   gen9_disable_dc5(dev_priv);
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.
@@ -629,8 +629,6 @@ static void skl_set_power_well(struct drm_i915_private 
*dev_priv,
 */
if 
(!dev_priv-power_domains.initializing)
intel_prepare_ddi(dev);
-   } else {
-   gen9_disable_dc5(dev_priv);
}
}
I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
@@ -650,12 +648,9 @@ static void

Re: [Intel-gfx] [PATCH 17/18] drm/i915/skl: Removed csr firmware load in resume path.

2015-07-29 Thread Sunil Kamath 

On Tuesday 28 July 2015 04:53 PM, Sunil Kamath wrote:

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c 
b/drivers/gpu/drm/i915/i915_drv.c

index 77b35fd..f5e720b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1048,7 +1048,6 @@ static int skl_resume_prepare(struct 
drm_i915_private *dev_priv)

  skl_disable_dc6(dev_priv);
skl_init_cdclk(dev_priv);
-intel_csr_load_program(dev_priv);


The context save and restore program is reset on cold boot, warm 
reset, PCI function level reset, and hibernate/suspend.


Will it not impact?

- Sunil


If the concern is about checking DC5/DC6 counters, isn't it good idea to 
add that as debug print before f/w reload?

Than to avoid completely reloading of entire firmware?

- Sunil

return 0;
  }




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 17/18] drm/i915/skl: Removed csr firmware load in resume path.

2015-07-29 Thread Sunil Kamath 

On Tuesday 28 July 2015 04:53 PM, Sunil Kamath wrote:

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c 
b/drivers/gpu/drm/i915/i915_drv.c

index 77b35fd..f5e720b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1048,7 +1048,6 @@ static int skl_resume_prepare(struct 
drm_i915_private *dev_priv)

  skl_disable_dc6(dev_priv);
skl_init_cdclk(dev_priv);
-intel_csr_load_program(dev_priv);


The context save and restore program is reset on cold boot, warm 
reset, PCI function level reset, and hibernate/suspend.


Will it not impact?

- Sunil


If the intention is just to check the DC5/DC6 counters, why cant we just 
add debug prints before f/w reload? Than to just avoiding reloading fw 
itself.


- Sunil

return 0;
  }




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 18/18] drm/i915/gen9: Removed byte swapping for csr firmware.

2015-07-28 Thread Sunil Kamath 

On Tuesday 28 July 2015 01:38 PM, Nagaraju, Vathsala wrote:

Signed-off-by: Vatsala Nagaraju vatsala.nagr...@intel.com
It's   Vathsala Nagaraju vathsala.nagar...@intel.com

Commit message: Removed byte swapping for csr firmware.

Commit message does not convey as to why the change was made.  This change is 
needed as DMC firmware loading failed on skylake due  byte swap  done in the driver


yes. agree.

Same review comments as other patches. Bug fixing patches should go 
first in sequence and also commit message should contain relevant info 
on fix/patch.


This byte swapping is not needed at all.

Minimal things to take care.

- Sunil






-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Animesh Manna
Sent: Sunday, July 26, 2015 12:31 AM
To: intel-gfx@lists.freedesktop.org
Cc: Vatsala Nagaraju
Subject: [Intel-gfx] [PATCH 18/18] drm/i915/gen9: Removed byte swapping for csr 
firmware.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vatsala Nagaraju vatsala.nagr...@intel.com
---
  drivers/gpu/drm/i915/intel_csr.c | 11 +--
  1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1858f02..50779e0 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -328,16 +328,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private 
*dev_priv,
return NULL;
}
  
-	for (i = 0; i  dmc_header-fw_size; i++) {

-   uint32_t *tmp = (u32 *)fw-data[readcount + i * 4];
-   /*
-* The firmware payload is an array of 32 bit words stored in
-* little-endian format in the firmware image and programmed
-* as 32 bit big-endian format to memory.
-*/
-   dmc_payload[i] = (uint32_t __force) cpu_to_be32(*tmp);
-   }
-
+   memcpy(dmc_payload, fw-data[readcount], dmc_header-fw_size);
return dmc_payload;
  }
  


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 17/18] drm/i915/skl: Removed csr firmware load in resume path.

2015-07-28 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 77b35fd..f5e720b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1048,7 +1048,6 @@ static int skl_resume_prepare(struct drm_i915_private 
*dev_priv)
skl_disable_dc6(dev_priv);
  
  	skl_init_cdclk(dev_priv);

-   intel_csr_load_program(dev_priv);


The context save and restore program is reset on cold boot, warm reset, 
PCI function level reset, and hibernate/suspend.


Will it not impact?

- Sunil
  
  	return 0;

  }


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/18] drm/i915/gen9: block disable call for pw1 if dmc firmware is present.

2015-07-28 Thread Sunil Kamath 

On Tuesday 28 July 2015 01:27 PM, Daniel Vetter wrote:

On Mon, Jul 27, 2015 at 10:48:16AM +0200, Daniel Vetter wrote:

On Sun, Jul 26, 2015 at 12:30:25AM +0530, Animesh Manna wrote:

Grabbing a runtime pm reference with intel_runtime_pm_get will only
prevent device D3. But dmc firmware is required even earlier (namely
for the skl power well 1). DMC is responsible to save the status of
power well 1 and shut off the power well when panel is self refresh
mode of display is completely off.

Another interesting criteria to work dmc as expected is pw1 to be
enabled by driver and dmc will shut it off in its execution
sequence. If already disabled by driver dmc will get confuse and
behave differently than expected found during pc10 entry issue
for skl.

So berfore we disable power-well 1, added check if dmc firmware is
present and driver will not disable power well 1, but for any reason
if firmware is not present of failed to load we can shut off the
power well 1 which will save some power.

As skl is currently fully dependent on dmc to go in lowest possible
power state (dc6) but the same is not applicable for bxt. Display
engine can enter into dc9 without dmc, hence unblocking disable call.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@intel.com

Please use the approach I've laid out in my original patch series with
drm/i915: use correct power domain for csr loading and drm/i915: Only
allow rpm on gen9+ with dmc loaded. Your approach here requires a
flush_work in the runtime pm callbacks which can deadlock.

If you want to make dmc optional on bxt then you need to adjust the 2nd
patch a bit to no longer leak the runtime pm reference. Your approach here
is an inversion of control and that doesn't work well since control
inversions very easily lead to locking inversions.

Summary of what we just discussed offline:

Ok I was confused here with the intel_csr_load_status_get() check. If we
apply the above to patch from me first then we don't need that check any
more, and the patch itself looks good as a bugfix for skl dmc firmware
assumptions.
-Daniel

Thanks alot Damien.

Animesh,
As discussed in same call, bug fix should go initial patches in this series.

also its good to add the check in skl_set_power_well function itself 
than intel_display_power_put


- Sunil

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/18] drm/i915/bxt: Path added of dmc firmware ver1 for BXT

2015-07-26 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Rodrigo Vivi rodrigo.v...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
---


Please update the commit message header relevant details.
even one liner is sufficient.

Minor thing to take care.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com

- Sunil

  drivers/gpu/drm/i915/intel_csr.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 6d8a7bf..1866426 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -42,8 +42,10 @@
   */
  
  #define I915_CSR_SKL i915/skl_dmc_ver1.bin

+#define I915_CSR_BXT i915/bxt_dmc_ver1.bin
  
  MODULE_FIRMWARE(I915_CSR_SKL);

+MODULE_FIRMWARE(I915_CSR_BXT);
  
  /*

  * SKL CSR registers for DC5 and DC6
@@ -417,6 +419,8 @@ void intel_csr_ucode_init(struct drm_device *dev)
  
  	if (IS_SKYLAKE(dev))

csr-fw_path = I915_CSR_SKL;
+   else if (IS_BROXTON(dev))
+   csr-fw_path = I915_CSR_BXT;
else {
DRM_ERROR(Unexpected: no known CSR firmware for platform\n);
intel_csr_load_status_set(dev_priv, FW_FAILED);


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/18] drm/i915/bxt: Modified HAS_CSR, added support for BXT.

2015-07-26 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

Modified HAS_CSR macro defination which earlier only supported
for skl, now added support for BXT.

Cc: Vetter, Daniel daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b94ada9..a2eac8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2542,7 +2542,7 @@ struct drm_i915_cmd_table {
  #define HAS_RC6(dev)  (INTEL_INFO(dev)-gen = 6)
  #define HAS_RC6p(dev) (INTEL_INFO(dev)-gen == 6 || IS_IVYBRIDGE(dev))
  
-#define HAS_CSR(dev)	(IS_SKYLAKE(dev))

+#define HAS_CSR(dev)   (IS_SKYLAKE(dev) || IS_BROXTON(dev))


with this change, we are adding DMC support for entire Gen9.
You can replace SKL and BXT specific check with Gen9 check alone.

Again minor change to take care.

Reviewed-by: A.Sunil Kamath sunil.kam...@intel.com

-Sunil

  
  #define HAS_RESOURCE_STREAMER(dev) (IS_HASWELL(dev) || \

INTEL_INFO(dev)-gen = 8)


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/18] Redesign of dmc firmware loading.

2015-07-26 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

Display microcontroller(DMC) used to save and restore display engine status
while entering into low power display states for gen9 platform.
Though skylake and broxton both are gen9 platform but dmc act diferently.
Skylake is solely dependednt on dmc for entering into low power
state - namely dc5 and dc6. Whereas broxton has low power state
dc5 and dc9 and dc9 does not need dmc.

As both gen9 platforms behaviour is different with respect to dmc,
so software design will be different and follow the below design
principles.


Thanks Animesh for pushing these latest patches in sequence. [including 
all latest bug fix and firmware load design change request].

With this we can already discard my older BXT specific DMC patches.

Also below SKL patches are very important to be merged asap. As they fix 
some of the major issues to achieve PC10.[which was blocked because of 
DMC DC6].


Will start reviewing right away.


Though we have positive test results already, need a detailed review to 
ensure that both SKL/BXT everything taken care also compatible with all 
linux distros.


-Sunil



For skylake:

If firmware loading is successful,
- Driver can goto D3 in suspend time.
- Will not disable power-well-1 as dmc will handle save/restore/shutdown.
If firmware loading is failed,
- Leak rpm which will block D3 in suspend.
- Will disable power-well-1 which will save some power.

For broxton:

Irrespective of firmware loading succesful/failed driver should not
block D3 during suspend as dc9 (lowest possible state) is independent
of dmc and will not block disable call of power-well-1.

During debugging PC10 entry issue for skylake found below observation,
- dmc will get confuse if driver already disable power-well-1 during
dc6 entry and will not work as expected.
- The above same applicable for cdclk pll as well.
- mmio read/write after dc6 trigger will cause display engine hang.

Based on Daniel's review comments and thiinking of above pointers
following patches is created.

Animesh Manna (10):
   drm/i915/bxt: Path added of dmc firmware ver1 for BXT
   drm/i915/bxt: Modified HAS_CSR, added support for BXT.
   drm/i915/bxt: Stepping info added for bxt.
   drm/i915/gen9: block disable call for pw1 if dmc firmware is present.
   drm/i915/gen9: csr_init after runtime pm enable
   drm/i915/gen9: Use flush_work to synchronize with dmc loader
   drm/i915/skl: Making DC6 entry is the last call in suspend flow.
   drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.
   drm/i915/skl: Removed csr firmware load in resume path.
   drm/i915/gen9: Removed byte swapping for csr firmware.

Daniel Vetter (8):
   drm/i915/gen9: move assert_csr_loaded into intel_rpm.c
   drm/i915/gen9: Remove csr.state, csr_lock and related code.
   drm/i915/gen9: Align line continuations in intel_csr.c.
   drm/i915/gen9: Simplify csr loading failure printing.
   drm/i915/gen9: extract parse_csr_fw.
   drm/i915/gen9: Don't try to load garbage dmc firmware on resume
   drm/i915/gen9: Use dev_priv in csr functions
   drm/i915: Use request_firmware and our own async work

  drivers/gpu/drm/i915/i915_dma.c |  11 +-
  drivers/gpu/drm/i915/i915_drv.c |  33 +
  drivers/gpu/drm/i915/i915_drv.h |  16 +--
  drivers/gpu/drm/i915/i915_reg.h |  16 +++
  drivers/gpu/drm/i915/intel_csr.c| 245 
  drivers/gpu/drm/i915/intel_display.c|  11 +-
  drivers/gpu/drm/i915/intel_drv.h|  12 +-
  drivers/gpu/drm/i915/intel_runtime_pm.c |  47 +++---
  8 files changed, 155 insertions(+), 236 deletions(-)



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/18] drm/i915/bxt: Stepping info added for bxt.

2015-07-26 Thread Sunil Kamath 

On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:

Added stepping info in intel_csr.c which is required to extract
specific firmware from packaged dmc firmware.

Cc: Vetter, Daniel daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
---
  drivers/gpu/drm/i915/intel_csr.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1866426..f440299 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -183,11 +183,19 @@ static const struct stepping_info skl_stepping_info[] = {
{'G', '0'}, {'H', '0'}, {'I', '0'}
  };
  
+static struct stepping_info bxt_stepping_info[] = {

+   {'A', '0'}, {'A', '1'}, {'A', '2'},
+   {'B', '0'}, {'B', '1'}, {'B', '2'}
+};
+


I understand that these stepping structure is as per present limited info.


  static char intel_get_stepping(struct drm_device *dev)
  {
if (IS_SKYLAKE(dev)  (dev-pdev-revision 
ARRAY_SIZE(skl_stepping_info)))
return skl_stepping_info[dev-pdev-revision].stepping;
+   else if (IS_BROXTON(dev)  (dev-pdev-revision 
+   ARRAY_SIZE(bxt_stepping_info)))
+   return bxt_stepping_info[dev-pdev-revision].stepping;
else
return -ENODATA;
  }
@@ -197,6 +205,9 @@ static char intel_get_substepping(struct drm_device *dev)
if (IS_SKYLAKE(dev)  (dev-pdev-revision 
ARRAY_SIZE(skl_stepping_info)))
return skl_stepping_info[dev-pdev-revision].substepping;
+   else if (IS_BROXTON(dev)  (dev-pdev-revision 
+   ARRAY_SIZE(bxt_stepping_info)))
+   return bxt_stepping_info[dev-pdev-revision].substepping;
else
return -ENODATA;
  }


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx