Re: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: remove wait for previous GFX clk disable request

2015-04-07 Thread Jani Nikula
On Thu, 02 Apr 2015, Darren Hart darren.h...@intel.com wrote:
 Jesse Barnes jbarnes at virtuousgeek.org writes:
 Looks like it was introduced in:
 
 commit 650ad970a39f8b6164fe8613edc150f585315289
 Author: Imre Deak imre.deak at intel.com
 Date:   Fri Apr 18 16:35:02 2014 +0300
 
 drm/i915: vlv: factor out vlv_force_gfx_clock and check for pending
 force-of
 
 but I'm not sure why.  It has caused problems for us in the past (see
 85250ddff7a603dfe0ec0503a9e6395f79424f61 and
 8d4eee9cd7a170342dc6fbc2ee19ae77031a8cd5) and doesn't seem to be
 required, so let's just drop it.
 
 References: https://bugs.freedesktop.org/show_bug.cgi?id=89611
 Signed-off-by: Jesse Barnes jbarnes at virtuousgeek.org


 Thanks Jesse,

 With this and 1/2 applied I was able to suspend/resume twice in a row
 successfully on a MinnowBoard MAX dual-core (E3825) with the 0.78
 firmware.

 Tested-by: Darren Hart dvh...@linux.intel.com

Both patches pushed to drm-intel-fixes, along with a cherry-pick of
85250ddff7a6 (drm/i915/chv: Remove Wait for a previous gfx force-off)
from drm-next to avoid conflicts, all cc: stable. Thanks for the
patches, review, and testing.

BR,
Jani.




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

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: remove wait for previous GFX clk disable request

2015-04-02 Thread Imre Deak
On Wed, 2015-04-01 at 14:22 -0700, Jesse Barnes wrote:
 Looks like it was introduced in:
 
 commit 650ad970a39f8b6164fe8613edc150f585315289
 Author: Imre Deak imre.d...@intel.com
 Date:   Fri Apr 18 16:35:02 2014 +0300
 
 drm/i915: vlv: factor out vlv_force_gfx_clock and check for pending 
 force-of
 
 but I'm not sure why.  It has caused problems for us in the past (see
 85250ddff7a603dfe0ec0503a9e6395f79424f61 and
 8d4eee9cd7a170342dc6fbc2ee19ae77031a8cd5) and doesn't seem to be
 required, so let's just drop it.
 
 References: https://bugs.freedesktop.org/show_bug.cgi?id=89611
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/i915_drv.c | 14 --
  1 file changed, 14 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 4d6d6f0..c3fdbb0 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -1208,21 +1208,7 @@ int vlv_force_gfx_clock(struct drm_i915_private 
 *dev_priv, bool force_on)
   u32 val;
   int err;
  
 - val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
 -
  #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG)  VLV_GFX_CLK_STATUS_BIT)
 - /* Wait for a previous force-off to settle */
 - if (force_on  !IS_CHERRYVIEW(dev_priv-dev)) {
 - /* WARN_ON only for the Valleyview */
 - WARN_ON(!!(val  VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
 -
 - err = wait_for(!COND, 20);
 - if (err) {
 - DRM_ERROR(timeout waiting for GFX clock force-off 
 (%08x)\n,
 -   I915_READ(VLV_GTLC_SURVIVABILITY_REG));
 - return err;
 - }
 - }

The reason I added this is that it's not clear what happens if you try
to force the clock on while the previous force-off operation is still
pending. That is if Punit will correctly cancel turning off the clock in
this case. Since the docs don't clarify this either I thought the above
is safer. Is it the WARN that triggers and only during resume (we also
call the function from vlv_set_rps_idle)? If so, then it's not a real
timeout but BIOS has left the force-on flag set and we could just skip
calling vlv_force_gfx_clock(true) in that case.

If the HW people can confirm that the above isn't needed then I'm also
ok to remove it.

  
   val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
   val = ~VLV_GFX_CLK_FORCE_ON_BIT;


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


Re: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: remove wait for previous GFX clk disable request

2015-04-02 Thread Deepak S



On Thursday 02 April 2015 02:52 AM, Jesse Barnes wrote:

Looks like it was introduced in:

commit 650ad970a39f8b6164fe8613edc150f585315289
Author: Imre Deak imre.d...@intel.com
Date:   Fri Apr 18 16:35:02 2014 +0300

 drm/i915: vlv: factor out vlv_force_gfx_clock and check for pending 
force-of

but I'm not sure why.  It has caused problems for us in the past (see
85250ddff7a603dfe0ec0503a9e6395f79424f61 and
8d4eee9cd7a170342dc6fbc2ee19ae77031a8cd5) and doesn't seem to be
required, so let's just drop it.

References: https://bugs.freedesktop.org/show_bug.cgi?id=89611
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
  drivers/gpu/drm/i915/i915_drv.c | 14 --
  1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4d6d6f0..c3fdbb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1208,21 +1208,7 @@ int vlv_force_gfx_clock(struct drm_i915_private 
*dev_priv, bool force_on)
u32 val;
int err;
  
-	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);

-
  #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG)  VLV_GFX_CLK_STATUS_BIT)
-   /* Wait for a previous force-off to settle */
-   if (force_on  !IS_CHERRYVIEW(dev_priv-dev)) {
-   /* WARN_ON only for the Valleyview */
-   WARN_ON(!!(val  VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
-
-   err = wait_for(!COND, 20);
-   if (err) {
-   DRM_ERROR(timeout waiting for GFX clock force-off 
(%08x)\n,
- I915_READ(VLV_GTLC_SURVIVABILITY_REG));
-   return err;
-   }
-   }
  


I agree, I do not think we need to wait for previous Gfx clk force-off.
Also, I do not see any race condition happening between diff Gfx force clk in 
driver. Lets just drop it :)

Reviewed-by: Deepak S  deepa...@linux.intel.com


val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
val = ~VLV_GFX_CLK_FORCE_ON_BIT;


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


Re: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: remove wait for previous GFX clk disable request

2015-04-02 Thread Darren Hart
Jesse Barnes jbarnes at virtuousgeek.org writes:
 Looks like it was introduced in:
 
 commit 650ad970a39f8b6164fe8613edc150f585315289
 Author: Imre Deak imre.deak at intel.com
 Date:   Fri Apr 18 16:35:02 2014 +0300
 
 drm/i915: vlv: factor out vlv_force_gfx_clock and check for pending
 force-of
 
 but I'm not sure why.  It has caused problems for us in the past (see
 85250ddff7a603dfe0ec0503a9e6395f79424f61 and
 8d4eee9cd7a170342dc6fbc2ee19ae77031a8cd5) and doesn't seem to be
 required, so let's just drop it.
 
 References: https://bugs.freedesktop.org/show_bug.cgi?id=89611
 Signed-off-by: Jesse Barnes jbarnes at virtuousgeek.org


Thanks Jesse,

With this and 1/2 applied I was able to suspend/resume twice in a row
successfully on a MinnowBoard MAX dual-core (E3825) with the 0.78
firmware.

Tested-by: Darren Hart dvh...@linux.intel.com

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


[Intel-gfx] [PATCH 2/2] drm/i915/vlv: remove wait for previous GFX clk disable request

2015-04-01 Thread Jesse Barnes
Looks like it was introduced in:

commit 650ad970a39f8b6164fe8613edc150f585315289
Author: Imre Deak imre.d...@intel.com
Date:   Fri Apr 18 16:35:02 2014 +0300

drm/i915: vlv: factor out vlv_force_gfx_clock and check for pending force-of

but I'm not sure why.  It has caused problems for us in the past (see
85250ddff7a603dfe0ec0503a9e6395f79424f61 and
8d4eee9cd7a170342dc6fbc2ee19ae77031a8cd5) and doesn't seem to be
required, so let's just drop it.

References: https://bugs.freedesktop.org/show_bug.cgi?id=89611
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/i915_drv.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4d6d6f0..c3fdbb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1208,21 +1208,7 @@ int vlv_force_gfx_clock(struct drm_i915_private 
*dev_priv, bool force_on)
u32 val;
int err;
 
-   val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
-
 #define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG)  VLV_GFX_CLK_STATUS_BIT)
-   /* Wait for a previous force-off to settle */
-   if (force_on  !IS_CHERRYVIEW(dev_priv-dev)) {
-   /* WARN_ON only for the Valleyview */
-   WARN_ON(!!(val  VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
-
-   err = wait_for(!COND, 20);
-   if (err) {
-   DRM_ERROR(timeout waiting for GFX clock force-off 
(%08x)\n,
- I915_READ(VLV_GTLC_SURVIVABILITY_REG));
-   return err;
-   }
-   }
 
val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
val = ~VLV_GFX_CLK_FORCE_ON_BIT;
-- 
1.9.1

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