[Intel-gfx] [PATCH] [VPG HSW-A] drm/i915:Added HDMI Audio codec disable sequence for HSW.

2013-08-30 Thread mengdong . lin
From: Mukesh mukeshx.ar...@intel.com

The code implements hsw_hdmi_audio_disable func which sets the
relevant registers for disabling the audio codec in a call to
intel_disable_ddi func.This audio codec disbale sequence is
implemented as per the recommendation of the Bspec.

Change-Id: If6eefbfe5ef821db547c759caa9ff5dc18980738
Signed-off-by: Mukesh Arora mukeshx.ar...@intel.com
---
 drivers/gpu/drm/i915/intel_ddi.c | 41 
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0de236e..2718d9a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1119,6 +1119,43 @@ static void intel_ddi_post_disable(struct intel_encoder 
*intel_encoder)
I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
 }
 
+/* Sets the registers for  audio codec disable sequence as
+* mentioned in the Haswell Bspec.
+*/
+void hsw_hdmi_audio_disable(struct drm_encoder *encoder)
+{
+   u32 temp;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc);
+   struct drm_device *dev = encoder-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int aud_config = HSW_AUD_CFG(intel_crtc-pipe);
+
+   /* HDMI audio disable sequence for Haswell*/
+   if (intel_crtc-eld_vld) {
+   /* disable timestamps */
+   temp = I915_READ(aud_config);
+   /* write 0 for HDMI */
+   temp = ~AUD_CONFIG_N_VALUE_INDEX;
+   /* Set N_programming_enable */
+   temp |= AUD_CONFIG_N_PROG_ENABLE;
+   /* Set Upper_N_value and Lower_N_value
+   (bits 27:20, 15:4) to all 0s */
+   temp = ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
+   I915_WRITE(aud_config, temp);
+   /* Disable ELDV and ELD buffer */
+   temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+   temp = ~(AUDIO_ELD_VALID_A  (intel_crtc-pipe * 4));
+   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
+   /* Wait for 2 vertical blanks */
+   intel_wait_for_vblank(dev, intel_crtc-pipe);
+   intel_wait_for_vblank(dev, intel_crtc-pipe);
+   /* Disable audio PD. This is optional as per Bspec.  */
+   temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+   temp = ~(AUDIO_OUTPUT_ENABLE_A  (intel_crtc-pipe * 4));
+   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
+   }
+
+}
 static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 {
struct drm_encoder *encoder = intel_encoder-base;
@@ -1173,6 +1210,10 @@ static void intel_disable_ddi(struct intel_encoder 
*intel_encoder)
tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) 
 (pipe * 4));
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+   if (IS_HASWELL(dev)  (type == INTEL_OUTPUT_HDMI)) {
+   /*HDMI audio codec disable sequence.*/
+   hsw_hdmi_audio_disable(encoder);
+   }
}
 
if (type == INTEL_OUTPUT_EDP) {
-- 
1.8.1.2

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


[Intel-gfx] [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell

2013-08-30 Thread mengdong . lin
From: Mukesh mukeshx.ar...@intel.com

The code defines a new function intel_disable_audio() to implement the
entire audio codec disable sequence, called by intel_disable_ddi() in
DDI port disablement. This audio codec disbale sequence is implemented
as per the recommendation of the Bspec.

[Patch modified by Mendong to apply to both HDMI and DP port.]

Signed-off-by: Mukesh Arora mukeshx.ar...@intel.com
Signed-off-by: Mengdong Lin mengdong@intel.com

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b042ee5..2946fe7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1088,6 +1088,45 @@ static void intel_ddi_post_disable(struct intel_encoder 
*intel_encoder)
I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
 }
 
+/* audio codec disable sequence, as per Bspec */
+void intel_disable_audio(struct intel_encoder *intel_encoder)
+{
+   int type = intel_encoder-type;
+   struct drm_encoder *encoder = intel_encoder-base;
+   struct drm_device *dev = encoder-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc);
+   int pipe = intel_crtc-pipe;
+   int aud_config = HSW_AUD_CFG(pipe);
+   u32 temp;
+
+   /* disable timestamps */
+   temp = I915_READ(aud_config);
+   if (type == INTEL_OUTPUT_HDMI)
+   temp = ~AUD_CONFIG_N_VALUE_INDEX;
+   else if (type == INTEL_OUTPUT_DISPLAYPORT)
+   temp |= AUD_CONFIG_N_VALUE_INDEX;
+   else
+   return;
+   temp |= AUD_CONFIG_N_PROG_ENABLE;
+   temp = ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
+   I915_WRITE(aud_config, temp);
+
+   /* Disable ELDV and ELD buffer */
+   temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+   temp = ~(AUDIO_ELD_VALID_A  (pipe * 4));
+   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
+
+   /* Wait for 2 vertical blanks */
+   intel_wait_for_vblank(dev, pipe);
+   intel_wait_for_vblank(dev, pipe);
+
+   /* Disable audio PD. This is optional as per Bspec.  */
+   temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
+   temp = ~(AUDIO_OUTPUT_ENABLE_A  (pipe * 4));
+   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
+}
+
 static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 {
struct drm_encoder *encoder = intel_encoder-base;
@@ -1132,18 +1171,10 @@ static void intel_disable_ddi(struct intel_encoder 
*intel_encoder)
struct drm_encoder *encoder = intel_encoder-base;
struct drm_crtc *crtc = encoder-crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   int pipe = intel_crtc-pipe;
int type = intel_encoder-type;
-   struct drm_device *dev = encoder-dev;
-   struct drm_i915_private *dev_priv = dev-dev_private;
-   uint32_t tmp;
 
-   if (intel_crtc-eld_vld  type != INTEL_OUTPUT_EDP) {
-   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
-   tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) 
-(pipe * 4));
-   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
-   }
+   if (intel_crtc-eld_vld  type != INTEL_OUTPUT_EDP)
+   intel_disable_audio(intel_encoder);
 
if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-- 
1.8.1.2

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


Re: [Intel-gfx] [PATCH] [VPG HSW-A] drm/i915:Added HDMI Audio codec disable sequence for HSW.

2013-08-30 Thread Lin, Mengdong
Hi Daniel and Mukesh,

This patch is submitted on behalf of Arora Mukesh. Please have a review.

And the 2nd version of patch is also submitted with subject: [PATCH v2] 
drm/i915/hsw: Add display Audio codec disable sequence for Haswell. Please also 
have a review.
http://lists.freedesktop.org/archives/intel-gfx/2013-August/032488.html

In the new patch, I did some modification to this original patch, to apply to 
both HDMI and DP port:
- rename the function from hsw_hdmi_audio_disable() to intel_disable_audio()
- let intel_disable_ddi() call this new function for both HDMI and DP port.
- remove redundant device check, since only Haswell and its successors use DDI 
port operations in intel_ddi.c

Thanks
Mengdong

 -Original Message-
 From: intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org
 [mailto:intel-gfx-bounces+mengdong.lin=intel@lists.freedesktop.org] On
 Behalf Of mengdong@intel.com
 Sent: Friday, August 30, 2013 7:48 AM
 To: intel-gfx@lists.freedesktop.org
 Cc: Arora, MukeshX
 Subject: [Intel-gfx] [PATCH] [VPG HSW-A] drm/i915:Added HDMI Audio codec
 disable sequence for HSW.
 
 From: Mukesh mukeshx.ar...@intel.com
 
 The code implements hsw_hdmi_audio_disable func which sets the relevant
 registers for disabling the audio codec in a call to intel_disable_ddi 
 func.This
 audio codec disbale sequence is implemented as per the recommendation of
 the Bspec.
 
 Change-Id: If6eefbfe5ef821db547c759caa9ff5dc18980738
 Signed-off-by: Mukesh Arora mukeshx.ar...@intel.com
 ---
  drivers/gpu/drm/i915/intel_ddi.c | 41
 
  1 file changed, 41 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index 0de236e..2718d9a 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -1119,6 +1119,43 @@ static void intel_ddi_post_disable(struct
 intel_encoder *intel_encoder)
   I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);  }
 
 +/* Sets the registers for  audio codec disable sequence as
 +* mentioned in the Haswell Bspec.
 +*/
 +void hsw_hdmi_audio_disable(struct drm_encoder *encoder) {
 + u32 temp;
 + struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc);
 + struct drm_device *dev = encoder-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + int aud_config = HSW_AUD_CFG(intel_crtc-pipe);
 +
 + /* HDMI audio disable sequence for Haswell*/
 + if (intel_crtc-eld_vld) {
 + /* disable timestamps */
 + temp = I915_READ(aud_config);
 + /* write 0 for HDMI */
 + temp = ~AUD_CONFIG_N_VALUE_INDEX;
 + /* Set N_programming_enable */
 + temp |= AUD_CONFIG_N_PROG_ENABLE;
 + /* Set Upper_N_value and Lower_N_value
 + (bits 27:20, 15:4) to all 0s */
 + temp =
 ~(AUD_CONFIG_UPPER_N_VALUE|AUD_CONFIG_LOWER_N_VALUE);
 + I915_WRITE(aud_config, temp);
 + /* Disable ELDV and ELD buffer */
 + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 + temp = ~(AUDIO_ELD_VALID_A  (intel_crtc-pipe * 4));
 + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
 + /* Wait for 2 vertical blanks */
 + intel_wait_for_vblank(dev, intel_crtc-pipe);
 + intel_wait_for_vblank(dev, intel_crtc-pipe);
 + /* Disable audio PD. This is optional as per Bspec.  */
 + temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 + temp = ~(AUDIO_OUTPUT_ENABLE_A  (intel_crtc-pipe * 4));
 + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
 + }
 +
 +}
  static void intel_enable_ddi(struct intel_encoder *intel_encoder)  {
   struct drm_encoder *encoder = intel_encoder-base; @@ -1173,6
 +1210,10 @@ static void intel_disable_ddi(struct intel_encoder
 *intel_encoder)
   tmp = ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) 
(pipe * 4));
   I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
 + if (IS_HASWELL(dev)  (type == INTEL_OUTPUT_HDMI)) {
 + /*HDMI audio codec disable sequence.*/
 + hsw_hdmi_audio_disable(encoder);
 + }
   }
 
   if (type == INTEL_OUTPUT_EDP) {
 --
 1.8.1.2
 
 ___
 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 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.

2013-08-30 Thread Daniel Vetter
On Sat, Aug 31, 2013 at 01:57:44AM +0800, Chon Ming Lee wrote:
 eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
 rate.  Create a structure to store the DPLL divisor data to improve
 readability.
 
 Signed-off-by: Chon Ming Lee chon.ming@intel.com

This is a neat idea and I agree it'll be much better when we add edp link
rates. Small comments below:

 ---
  drivers/gpu/drm/i915/intel_dp.c |   48 +++---
  1 files changed, 24 insertions(+), 24 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 2151d13..ab8a5ff 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -38,6 +38,19 @@
  
  #define DP_LINK_CHECK_TIMEOUT(10 * 1000)
  
 +struct dp_link_dpll{
 + int link_bw;
 + struct dpll dpll;
 +};
 +
 +static const struct dp_link_dpll gen4_dpll[] = 
 + {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
 + { DP_LINK_BW_2_7,  {1,14,2,1,10,0,0,0,0}}};

Usually we only indent by one tab here. Also please use c99 initializers
and you don't need to explicitly initialize to 0. And a bit more space
helps readbility further imo:

static const struct dp_link_dpll gen4_dpll[] = 
{
{ DP_LINK_BW_1_62,
{ .p1 = 2, .p2 = 10, .n = 2, .m1 = 23, .m2 = 8 }},
/* more ... */
};


 +
 +static const struct dp_link_dpll pch_dpll[] = 
 + {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
 + { DP_LINK_BW_2_7,  {2,14,8,1,10,0,0,0,0}}};
 +
  /**
   * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
   * @intel_dp: DP struct
 @@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
  struct intel_crtc_config *pipe_config, int link_bw)
  {
   struct drm_device *dev = encoder-base.dev;
 + int i;
  
   if (IS_G4X(dev)) {
 - if (link_bw == DP_LINK_BW_1_62) {
 - pipe_config-dpll.p1 = 2;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.n = 2;
 - pipe_config-dpll.m1 = 23;
 - pipe_config-dpll.m2 = 8;
 - } else {
 - pipe_config-dpll.p1 = 1;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.n = 1;
 - pipe_config-dpll.m1 = 14;
 - pipe_config-dpll.m2 = 2;
 + for(i = 0; i  sizeof(gen4_dpll) / sizeof(struct dp_link_dpll); 
 i++) {
 + if (link_bw == gen4_dpll[i].link_bw){
 + pipe_config-dpll = gen4_dpll[i].dpll;
 + break;
 + }
   }
   pipe_config-clock_set = true;
   } else if (IS_HASWELL(dev)) {
   /* Haswell has special-purpose DP DDI clocks. */
   } else if (HAS_PCH_SPLIT(dev)) {
 - if (link_bw == DP_LINK_BW_1_62) {
 - pipe_config-dpll.n = 1;
 - pipe_config-dpll.p1 = 2;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.m1 = 12;
 - pipe_config-dpll.m2 = 9;
 - } else {
 - pipe_config-dpll.n = 2;
 - pipe_config-dpll.p1 = 1;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.m1 = 14;
 - pipe_config-dpll.m2 = 8;
 + for(i = 0; i  sizeof(pch_dpll) / sizeof(struct dp_link_dpll); 
 i++) {
 + if (link_bw == pch_dpll[i].link_bw){ 
 + pipe_config-dpll = pch_dpll[i].dpll;
 + break;
 + }

I'd add temporary variables here to first select the right platform (and
size of the array) and then only have one for loop to fish out the right
value.

Cheers, Daniel

   }
   pipe_config-clock_set = true;
   } else if (IS_VALLEYVIEW(dev)) {
 -- 
 1.7.7.6
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.

2013-08-30 Thread Jani Nikula
On Fri, 30 Aug 2013, Chon Ming Lee chon.ming@intel.com wrote:
 eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
 rate.  Create a structure to store the DPLL divisor data to improve
 readability.

DP 1.2 already supports 3 link rates, right?

 Signed-off-by: Chon Ming Lee chon.ming@intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c |   48 +++---
  1 files changed, 24 insertions(+), 24 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 2151d13..ab8a5ff 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -38,6 +38,19 @@
  
  #define DP_LINK_CHECK_TIMEOUT(10 * 1000)
  
 +struct dp_link_dpll{

Missing space before {.

 + int link_bw;
 + struct dpll dpll;
 +};
 +
 +static const struct dp_link_dpll gen4_dpll[] = 
 + {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
 + { DP_LINK_BW_2_7,  {1,14,2,1,10,0,0,0,0}}};
 +
 +static const struct dp_link_dpll pch_dpll[] = 
 + {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
 + { DP_LINK_BW_2_7,  {2,14,8,1,10,0,0,0,0}}};
 +

Please switch these to use C99 designated initializers for clarity,
something like this:

static const struct dp_link_dpll gen4_dpll[] = {
{
.link_bw = DP_LINK_BW_1_62,
.dpll = {
.n = 2,
.m1 = 23, m2 = 8,
p1 = 2, p2 = 10, 
},
}, {
.link_bw = DP_LINK_BW_2_7,
.dpll = {
.n = 1,
.m1 = 14, m2 = 2,
p1 = 1, p2 = 10, 
},
}
};

  /**
   * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
   * @intel_dp: DP struct
 @@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
  struct intel_crtc_config *pipe_config, int link_bw)
  {
   struct drm_device *dev = encoder-base.dev;
 + int i;
  
   if (IS_G4X(dev)) {
 - if (link_bw == DP_LINK_BW_1_62) {
 - pipe_config-dpll.p1 = 2;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.n = 2;
 - pipe_config-dpll.m1 = 23;
 - pipe_config-dpll.m2 = 8;
 - } else {
 - pipe_config-dpll.p1 = 1;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.n = 1;
 - pipe_config-dpll.m1 = 14;
 - pipe_config-dpll.m2 = 2;
 + for(i = 0; i  sizeof(gen4_dpll) / sizeof(struct dp_link_dpll); 
 i++) {

Please use i  ARRAY_SIZE(gen4_dpll).

 + if (link_bw == gen4_dpll[i].link_bw){
 + pipe_config-dpll = gen4_dpll[i].dpll;
 + break;
 + }
   }

The old if-else used some values even if link_bw was bogus. I'm not sure
how likely that is. But if the link_bw is not found in the table for
some obscure reason (e.g. the hw doesn't support the link rate), this
fails. Perhaps we should have a test for i == ARRAY_SIZE(gen4_dpll) here
and complain loudly if we hit that, and perhaps use a fallback value.

   pipe_config-clock_set = true;
   } else if (IS_HASWELL(dev)) {
   /* Haswell has special-purpose DP DDI clocks. */
   } else if (HAS_PCH_SPLIT(dev)) {
 - if (link_bw == DP_LINK_BW_1_62) {
 - pipe_config-dpll.n = 1;
 - pipe_config-dpll.p1 = 2;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.m1 = 12;
 - pipe_config-dpll.m2 = 9;
 - } else {
 - pipe_config-dpll.n = 2;
 - pipe_config-dpll.p1 = 1;
 - pipe_config-dpll.p2 = 10;
 - pipe_config-dpll.m1 = 14;
 - pipe_config-dpll.m2 = 8;
 + for(i = 0; i  sizeof(pch_dpll) / sizeof(struct dp_link_dpll); 
 i++) {
 + if (link_bw == pch_dpll[i].link_bw){ 
 + pipe_config-dpll = pch_dpll[i].dpll;
 + break;
 + }
   }

Same here.

BR,
Jani.


   pipe_config-clock_set = true;
   } else if (IS_VALLEYVIEW(dev)) {
 -- 
 1.7.7.6

 ___
 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: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock

2013-08-30 Thread Jani Nikula

[Okay, I missed Daniel's review, and noticed I hadn't actually hit send
on this one either... but here goes anyway...]

On Fri, 30 Aug 2013, Chon Ming Lee chon.ming@intel.com wrote:
 For DP pll settings, there is only two golden configs.  Instead of running
 through the algorithm to determine it, hardcode the value and get it
 determine in intel_dp_set_clock.

 Signed-off-by: Chon Ming Lee chon.ming@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c |   22 --
  drivers/gpu/drm/i915/intel_dp.c  |   12 +++-
  2 files changed, 15 insertions(+), 19 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f526ea9..453fa16 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -339,19 +339,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
   .p2_slow = 2, .p2_fast = 20 },
  };
  
 -static const intel_limit_t intel_limits_vlv_dp = {
 - .dot = { .min = 25000, .max = 27 },
 - .vco = { .min = 400, .max = 600 },
 - .n = { .min = 1, .max = 7 },
 - .m = { .min = 22, .max = 450 },
 - .m1 = { .min = 2, .max = 3 },
 - .m2 = { .min = 11, .max = 156 },
 - .p = { .min = 10, .max = 30 },
 - .p1 = { .min = 1, .max = 3 },
 - .p2 = { .dot_limit = 27,
 - .p2_slow = 2, .p2_fast = 20 },
 -};
 -
  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
   int refclk)
  {
 @@ -414,10 +401,8 @@ static const intel_limit_t *intel_limit(struct drm_crtc 
 *crtc, int refclk)
   } else if (IS_VALLEYVIEW(dev)) {
   if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
   limit = intel_limits_vlv_dac;
 - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
 + else 
   limit = intel_limits_vlv_hdmi;
 - else
 - limit = intel_limits_vlv_dp;
   } else if (!IS_GEN2(dev)) {
   if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
   limit = intel_limits_i9xx_lvds;
 @@ -4889,15 +4874,16 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
   }
  
   refclk = i9xx_get_refclk(crtc, num_connectors);
 + 
 + limit = intel_limit(crtc, refclk);

Did you move this here just to avoid the warning about uninitialized
limit? It's a bit ugly... but then again the the whole is_dsi vs. not is
rather ugly already. *shrug*.

  
 - if (!is_dsi) {
 + if (!is_dsi  !intel_crtc-config.clock_set) {
   /*
* Returns a set of divisors for the desired target clock with
* the given refclk, or FALSE.  The returned values represent
* the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
* 2) / p1 / p2.
*/
 - limit = intel_limit(crtc, refclk);
   ok = dev_priv-display.find_dpll(limit, crtc,
intel_crtc-config.port_clock,
refclk, NULL, clock);
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index ab8a5ff..89a2606 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -51,6 +51,10 @@ static const struct dp_link_dpll pch_dpll[] =
   {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
   { DP_LINK_BW_2_7,  {2,14,8,1,10,0,0,0,0}}};
  
 +static const struct dp_link_dpll vlv_dpll[] =
 + {{ DP_LINK_BW_1_62, {5,3,81,3,2,0,0,0,0}},
 + { DP_LINK_BW_2_7, {1,2,27,2,2,0,0,0,0}}};
 +
  /**
   * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
   * @intel_dp: DP struct
 @@ -683,7 +687,13 @@ intel_dp_set_clock(struct intel_encoder *encoder,
   }
   pipe_config-clock_set = true;
   } else if (IS_VALLEYVIEW(dev)) {
 - /* FIXME: Need to figure out optimized DP clocks for vlv. */
 + for(i = 0; i  sizeof(vlv_dpll) / sizeof(struct dp_link_dpll); 
 i++) {
 + if (link_bw == vlv_dpll[i].link_bw){ 
 + pipe_config-dpll = vlv_dpll[i].dpll;
 + break;
 + }
 + }
 + pipe_config-clock_set = true;

You now have three similar loops in the function. A follow-up patch
could pick the table to use in the if branches, and have a single loop
at the end. You could handle the array size by having .link_bw = 0 in
the last entry as a stop condition, and using that as the fallback entry
too (see my review of patch 1 about unknown link_bw values).

BR,
Jani.


   }
  }
  
 -- 
 1.7.7.6

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org

[Intel-gfx] [QA] Testing report for `drm-intel-testing` (was: Updated -next) on ww35

2013-08-30 Thread Yang, Guang A
Summary
We covered the platform: Haswell mobile, HSW desktop, HSW ULT, IvyBridge, 
SandyBridge, IronLake. in this circle, 5 new bugs are filed, 31 bugs are still 
opened, 1 WONTFIX bugs, 1 NOTABUG bugs,2 NOTOURBUG bugs, 0 Duplicated bugs,1 
REOPENED bugs, 3 bugs are fixed, 13 bugs are closed.



Test Environment

Kernel: (drm-intel-testing)ff7528fe24d3576a7472d37ebdf665ad092ba6a2

Some additional commit info:

Merge: a6e7e3d 6f1e120

Author: Daniel Vetter daniel.vet...@ffwll.ch

Date:   Fri Aug 23 14:53:34 2013 +0200



Finding

New Bugs:5 bugs

Bug 68643https://bugs.freedesktop.org/show_bug.cgi?id=68643 - [SNB DP]Some 
modes can't light up

Bug 68640https://bugs.freedesktop.org/show_bug.cgi?id=68640 - [HSW 
nv]igt/prime_nv_pcopy/test3_1 timeout with debug kernel

Bug 68638https://bugs.freedesktop.org/show_bug.cgi?id=68638 - [HSW 
nv]igt/prime_nv_test/i915_import_cpu_mmap fails with debug kernel

Bug 68462https://bugs.freedesktop.org/show_bug.cgi?id=68462 - 
[SNB]IGT/gem_evict_alignment system hang

Bug 68251https://bugs.freedesktop.org/show_bug.cgi?id=68251 - [IVB Apple 
MacBook pro] dmesg of machine boot up with call trace and *ERROR* mismatch in 
pch_pfit.pos (expected 0, found 80)



Opened Bugs:   31 bugs

Bug 68165https://bugs.freedesktop.org/show_bug.cgi?id=68165 - [SNB IVB HSW 
eDP bisected]I-G-T/testdisplay -f causes eDP black

Bug 68004https://bugs.freedesktop.org/show_bug.cgi?id=68004 - 
igt/prime_self_import/with_one_bo_two_files aborted

Bug 67891https://bugs.freedesktop.org/show_bug.cgi?id=67891 - 
igt/prime_self_import/export-vs-gem_close-race fail and causes call trace

Bug 67889https://bugs.freedesktop.org/show_bug.cgi?id=67889 - [Baytrail-M] 
Blackscreen occurred after running xrandr setting twice

Bug 67813https://bugs.freedesktop.org/show_bug.cgi?id=67813 - [HSW 
bisected]igt/module_reload causes [drm:hsw_unclaimed_reg_check] *ERROR* 
Unclaimed write to 44004 and system hang with headless, with power well disabled

Bug 67734https://bugs.freedesktop.org/show_bug.cgi?id=67734 - [Baytrail-M] 
gt_RP0_freq_mhz value is equal to 0

Bug 67732https://bugs.freedesktop.org/show_bug.cgi?id=67732 - [Baytrail-M] 
eDP and VGA can't light up simultaneously

Bug 67520https://bugs.freedesktop.org/show_bug.cgi?id=67520 - [HSW ULT] 
System boots with *ERROR* conflict detected with stolen region: [0xada0 - 
0xafa0]

Bug 67462https://bugs.freedesktop.org/show_bug.cgi?id=67462 - [SNB] Call 
trace appears after system boots with DP

Bug 67345https://bugs.freedesktop.org/show_bug.cgi?id=67345 - [BayTrail-M] 
[drm:intel_pipe_config_compare] *ERROR* mismatch in clock (expected 146250, 
found 0)

Bug 67287https://bugs.freedesktop.org/show_bug.cgi?id=67287 - 
igt/gem_flink_race fails

Bug 67243https://bugs.freedesktop.org/show_bug.cgi?id=67243 - 
[ILK]igt/kms_render/gpu-blit randomly causes system hang

Bug 67030https://bugs.freedesktop.org/show_bug.cgi?id=67030 - [HSW] no modes 
bigger than 1080p in the parsed EDID for 4k TV on both HDMI/VGA

Bug 67027https://bugs.freedesktop.org/show_bug.cgi?id=67027 - [HSW] some 
modes oversan, on a 4K HDMI TV

Bug 66301https://bugs.freedesktop.org/show_bug.cgi?id=66301 - [HSW mobile] 
resume from s4 sporadically causes call trace and machine is reachable

Bug 65995https://bugs.freedesktop.org/show_bug.cgi?id=65995 - [HSW mobile] 
eDP can't light up when boot up

Bug 65927https://bugs.freedesktop.org/show_bug.cgi?id=65927 - [HSW] crash 
when vga output set to mirror mode

Bug 65700https://bugs.freedesktop.org/show_bug.cgi?id=65700 - 
[SNB]3[drm:ironlake_disable_pch_transcoder] *ERROR* failed to disable 
transcoder 0

Bug 65496https://bugs.freedesktop.org/show_bug.cgi?id=65496 - [HSW] 
Probabilistic resume from s4 causes call trace and system hang, with warm boot

Bug 64415https://bugs.freedesktop.org/show_bug.cgi?id=64415 - [ILK] Some 
modes unable to light up on DP display

Bug 64379https://bugs.freedesktop.org/show_bug.cgi?id=64379 - [HSW desktop 
bisected] ddi pll tracking botch-up due to vt-switchless resume

Bug 63981https://bugs.freedesktop.org/show_bug.cgi?id=63981 - MacBook Pro 
retina 13 inch early 2013 jittery display

Bug 61041https://bugs.freedesktop.org/show_bug.cgi?id=61041 - 
[Pineview]I-G-T/testdisplay  VGA 1600x1200 65Hz messing the screen

Bug 60002https://bugs.freedesktop.org/show_bug.cgi?id=60002 - 
[PNV]I-G-T/kms_flip subtest:'blocking-absolute-wf_vblank' fail

Bug 5https://bugs.freedesktop.org/show_bug.cgi?id=5 - 
[ILK,IVB]kms_flip error: inter-flip ts jitter

Bug 59836https://bugs.freedesktop.org/show_bug.cgi?id=59836 - 
[PNV]igt/kms_flip/absolute-wf_vblank fail

Bug 59321https://bugzilla.kernel.org/show_bug.cgi?id=59321 - S4 broken with 
Haswell

Bug 58701https://bugs.freedesktop.org/show_bug.cgi?id=58701 - [SNB 
DP]I-G-T/testdisplay  1920x1080i showing not full

Bug 57441https://bugs.freedesktop.org/show_bug.cgi?id=57441 - 
[HSW]I-G-T/sysfs_l3_parity fail

Bug 54687https://bugs.freedesktop.org/show_bug.cgi?id=54687 - [ilk] pipe 

[Intel-gfx] [PATCH 00/19] drm/i915: More HSW watermark prep work

2013-08-30 Thread ville . syrjala
Another big set of prep work to get the watermarks ready for the atomic
age.

The main themes here are some new structures to allow us to eventually
pre-compute the watermarks and trying to reduce the overhead of the
watermark update itself.

My plan has been to attempt to update the watermarks from the vblank irq
handler, but I'm not 100% sure that I can do that. I have a big patch set
at [1] that does it (and more), and for the most part it is working. But
I am seeing an occasional underrun (very rarely though). I'm not sure
that's caused by missing the dealine in the vblank handler or something
else.

In my big patch set, the next step is adding the vblank update mechanism,
and after that convert ILK/SNB/IVB over to the HSW watermark code. As I'm
not yet 100% convinced that the vblank thing will work out, I think I will
reverse those steps, so that we can get the ILK+ stuff in while I keep
looking into the underrun issue a bit more.

The big patchset also has some other random bits and pieces (mainly
sprite related), and I'll probably be sending some of that out as well.

[1] git://gitorious.org/vsyrjala/linux.git watermarks_full
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 01/19] drm/i915: Pass crtc to intel_update_watermarks()

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Passing the appropriate crtc to intel_update_watermarks() should help
in avoiding needless work in the future.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 20 -
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 43 +---
 drivers/gpu/drm/i915/intel_sprite.c  |  6 ++---
 5 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7594356..ab46eb3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -357,7 +357,7 @@ struct drm_i915_display_funcs {
  int target, int refclk,
  struct dpll *match_clock,
  struct dpll *best_clock);
-   void (*update_wm)(struct drm_device *dev);
+   void (*update_wm)(struct drm_crtc *crtc);
void (*update_sprite_wm)(struct drm_plane *plane,
 struct drm_crtc *crtc,
 uint32_t sprite_width, int pixel_size,
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f526ea9..13856ec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3278,7 +3278,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
intel_set_pch_fifo_underrun_reporting(dev, pipe, true);
 
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_enable)
@@ -3388,7 +3388,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc-config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
 
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
if (intel_crtc-config.has_pch_encoder)
dev_priv-display.fdi_link_train(crtc);
@@ -3520,7 +3520,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
}
 
intel_crtc-active = false;
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
mutex_lock(dev-struct_mutex);
intel_update_fbc(dev);
@@ -3577,7 +3577,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
}
 
intel_crtc-active = false;
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
mutex_lock(dev-struct_mutex);
intel_update_fbc(dev);
@@ -3677,7 +3677,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
return;
 
intel_crtc-active = true;
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_pll_enable)
@@ -3722,7 +3722,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
return;
 
intel_crtc-active = true;
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_enable)
@@ -3806,7 +3806,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 
intel_crtc-active = false;
intel_update_fbc(dev);
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 }
 
 static void i9xx_crtc_off(struct drm_crtc *crtc)
@@ -4972,7 +4972,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 
ret = intel_pipe_set_base(crtc, x, y, fb);
 
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
return ret;
 }
@@ -5860,7 +5860,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
ret = intel_pipe_set_base(crtc, x, y, fb);
 
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
return ret;
 }
@@ -6316,7 +6316,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
ret = intel_pipe_set_base(crtc, x, y, fb);
 
-   intel_update_watermarks(dev);
+   intel_update_watermarks(crtc);
 
return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a38f5b2..8b132e2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -714,7 +714,7 @@ extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
 
 /* For use by IVB LP watermark workaround in intel_sprite.c */
-extern void intel_update_watermarks(struct drm_device *dev);
+extern void intel_update_watermarks(struct drm_crtc *crtc);
 extern void intel_update_sprite_watermarks(struct drm_plane *plane,
   

[Intel-gfx] [PATCH 02/19] drm/i915: Call intel_update_watermarks() in specific place during modeset

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Make the call to intel_update_watermarks() just once or twice during
modeset. Ideally it should happen independently when each plane gets
enabled/disabled, but for now it seems better to keep it in central
place. We can improve things when we get all the planes sorted out
in a better way.

When enabling set up the watermarks just before the pipe is enabled.
And when disabling we need to wait until we've marked the crtc as
inactive.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 13856ec..a5181fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3278,8 +3278,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
intel_set_pch_fifo_underrun_reporting(dev, pipe, true);
 
-   intel_update_watermarks(crtc);
-
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_enable)
encoder-pre_enable(encoder);
@@ -3302,6 +3300,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 */
intel_crtc_load_lut(crtc);
 
+   intel_update_watermarks(crtc);
intel_enable_pipe(dev_priv, pipe,
  intel_crtc-config.has_pch_encoder, false);
intel_enable_plane(dev_priv, plane, pipe);
@@ -3388,8 +3387,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc-config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
 
-   intel_update_watermarks(crtc);
-
if (intel_crtc-config.has_pch_encoder)
dev_priv-display.fdi_link_train(crtc);
 
@@ -3410,6 +3407,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_ddi_set_pipe_settings(crtc);
intel_ddi_enable_transcoder_func(crtc);
 
+   intel_update_watermarks(crtc);
intel_enable_pipe(dev_priv, pipe,
  intel_crtc-config.has_pch_encoder, false);
intel_enable_plane(dev_priv, plane, pipe);
@@ -3677,7 +3675,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
return;
 
intel_crtc-active = true;
-   intel_update_watermarks(crtc);
 
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_pll_enable)
@@ -3696,6 +3693,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
intel_crtc_load_lut(crtc);
 
+   intel_update_watermarks(crtc);
intel_enable_pipe(dev_priv, pipe, false, is_dsi);
intel_enable_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
@@ -3722,7 +3720,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
return;
 
intel_crtc-active = true;
-   intel_update_watermarks(crtc);
 
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-pre_enable)
@@ -3734,6 +3731,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
intel_crtc_load_lut(crtc);
 
+   intel_update_watermarks(crtc);
intel_enable_pipe(dev_priv, pipe, false, false);
intel_enable_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
@@ -3805,8 +3803,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
i9xx_disable_pll(dev_priv, pipe);
 
intel_crtc-active = false;
-   intel_update_fbc(dev);
intel_update_watermarks(crtc);
+
+   intel_update_fbc(dev);
 }
 
 static void i9xx_crtc_off(struct drm_crtc *crtc)
@@ -4972,8 +4971,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 
ret = intel_pipe_set_base(crtc, x, y, fb);
 
-   intel_update_watermarks(crtc);
-
return ret;
 }
 
@@ -5860,8 +5857,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
ret = intel_pipe_set_base(crtc, x, y, fb);
 
-   intel_update_watermarks(crtc);
-
return ret;
 }
 
@@ -6316,8 +6311,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
ret = intel_pipe_set_base(crtc, x, y, fb);
 
-   intel_update_watermarks(crtc);
-
return ret;
 }
 
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 03/19] drm/i915: Constify some watermark data

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

hsw_pipe_wm_parameters and hsw_wm_maximums typically are read only. Make
them const.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 774c57f..96493dc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2202,7 +2202,7 @@ struct intel_wm_config {
  * For both WM_PIPE and WM_LP.
  * mem_value must be in 0.1us units.
  */
-static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_pri_wm(const struct hsw_pipe_wm_parameters *params,
   uint32_t mem_value,
   bool is_lp)
 {
@@ -2231,7 +2231,7 @@ static uint32_t ilk_compute_pri_wm(struct 
hsw_pipe_wm_parameters *params,
  * For both WM_PIPE and WM_LP.
  * mem_value must be in 0.1us units.
  */
-static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_spr_wm(const struct hsw_pipe_wm_parameters *params,
   uint32_t mem_value)
 {
uint32_t method1, method2;
@@ -2254,7 +2254,7 @@ static uint32_t ilk_compute_spr_wm(struct 
hsw_pipe_wm_parameters *params,
  * For both WM_PIPE and WM_LP.
  * mem_value must be in 0.1us units.
  */
-static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_cur_wm(const struct hsw_pipe_wm_parameters *params,
   uint32_t mem_value)
 {
if (!params-active || !params-cur.enabled)
@@ -2268,7 +2268,7 @@ static uint32_t ilk_compute_cur_wm(struct 
hsw_pipe_wm_parameters *params,
 }
 
 /* Only for WM_LP. */
-static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_fbc_wm(const struct hsw_pipe_wm_parameters *params,
   uint32_t pri_val)
 {
if (!params-active || !params-pri.enabled)
@@ -2419,7 +2419,7 @@ static bool ilk_check_wm(int level,
 
 static void ilk_compute_wm_level(struct drm_i915_private *dev_priv,
 int level,
-struct hsw_pipe_wm_parameters *p,
+const struct hsw_pipe_wm_parameters *p,
 struct intel_wm_level *result)
 {
uint16_t pri_latency = dev_priv-wm.pri_latency[level];
@@ -2441,8 +2441,8 @@ static void ilk_compute_wm_level(struct drm_i915_private 
*dev_priv,
 }
 
 static bool hsw_compute_lp_wm(struct drm_i915_private *dev_priv,
- int level, struct hsw_wm_maximums *max,
- struct hsw_pipe_wm_parameters *params,
+ int level, const struct hsw_wm_maximums *max,
+ const struct hsw_pipe_wm_parameters *params,
  struct intel_wm_level *result)
 {
enum pipe pipe;
@@ -2462,7 +2462,7 @@ static bool hsw_compute_lp_wm(struct drm_i915_private 
*dev_priv,
 
 static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
enum pipe pipe,
-   struct hsw_pipe_wm_parameters *params)
+   const struct hsw_pipe_wm_parameters *params)
 {
uint32_t pri_val, cur_val, spr_val;
/* WM0 latency values stored in 0.1us units */
@@ -2670,8 +2670,8 @@ static void hsw_compute_wm_parameters(struct drm_device 
*dev,
 }
 
 static void hsw_compute_wm_results(struct drm_device *dev,
-  struct hsw_pipe_wm_parameters *params,
-  struct hsw_wm_maximums *lp_maximums,
+  const struct hsw_pipe_wm_parameters *params,
+  const struct hsw_wm_maximums *lp_maximums,
   struct hsw_wm_values *results)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 04/19] drm/i915: Use ilk_compute_wm_level to compute WM_PIPE values

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Unify the code a bit to use ilk_compute_wm_level for all watermark
levels.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 44 -
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 96493dc..d118ee1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2460,33 +2460,31 @@ static bool hsw_compute_lp_wm(struct drm_i915_private 
*dev_priv,
return ilk_check_wm(level, max, result);
 }
 
-static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
-   enum pipe pipe,
+
+static uint32_t hsw_compute_wm_pipe(struct drm_device *dev,
const struct hsw_pipe_wm_parameters *params)
 {
-   uint32_t pri_val, cur_val, spr_val;
-   /* WM0 latency values stored in 0.1us units */
-   uint16_t pri_latency = dev_priv-wm.pri_latency[0];
-   uint16_t spr_latency = dev_priv-wm.spr_latency[0];
-   uint16_t cur_latency = dev_priv-wm.cur_latency[0];
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   struct intel_wm_config config = {
+   .num_pipes_active = 1,
+   .sprites_enabled = params-spr.enabled,
+   .sprites_scaled = params-spr.scaled,
+   };
+   struct hsw_wm_maximums max;
+   struct intel_wm_level res;
+
+   if (!params-active)
+   return 0;
+
+   ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);
 
-   pri_val = ilk_compute_pri_wm(params, pri_latency, false);
-   spr_val = ilk_compute_spr_wm(params, spr_latency);
-   cur_val = ilk_compute_cur_wm(params, cur_latency);
+   ilk_compute_wm_level(dev_priv, 0, params, res);
 
-   WARN(pri_val  127,
-Primary WM error, mode not supported for pipe %c\n,
-pipe_name(pipe));
-   WARN(spr_val  127,
-Sprite WM error, mode not supported for pipe %c\n,
-pipe_name(pipe));
-   WARN(cur_val  63,
-Cursor WM error, mode not supported for pipe %c\n,
-pipe_name(pipe));
+   ilk_check_wm(0, max, res);
 
-   return (pri_val  WM0_PIPE_PLANE_SHIFT) |
-  (spr_val  WM0_PIPE_SPRITE_SHIFT) |
-  cur_val;
+   return (res.pri_val  WM0_PIPE_PLANE_SHIFT) |
+  (res.spr_val  WM0_PIPE_SPRITE_SHIFT) |
+  res.cur_val;
 }
 
 static uint32_t
@@ -2715,7 +2713,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
}
 
for_each_pipe(pipe)
-   results-wm_pipe[pipe] = hsw_compute_wm_pipe(dev_priv, pipe,
+   results-wm_pipe[pipe] = hsw_compute_wm_pipe(dev,
 params[pipe]);
 
for_each_pipe(pipe) {
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 05/19] drm/i915: Refactor max WM level

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Pull the expected max WM level determinations out to a separate
function. Will have another user soon.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d118ee1..163ba74 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2558,19 +2558,22 @@ static void intel_fixup_cur_wm_latency(struct 
drm_device *dev, uint16_t wm[5])
wm[3] *= 2;
 }
 
-static void intel_print_wm_latency(struct drm_device *dev,
-  const char *name,
-  const uint16_t wm[5])
+static int ilk_wm_max_level(const struct drm_device *dev)
 {
-   int level, max_level;
-
/* how many WM levels are we expecting */
if (IS_HASWELL(dev))
-   max_level = 4;
+   return 4;
else if (INTEL_INFO(dev)-gen = 6)
-   max_level = 3;
+   return 3;
else
-   max_level = 2;
+   return 2;
+}
+
+static void intel_print_wm_latency(struct drm_device *dev,
+  const char *name,
+  const uint16_t wm[5])
+{
+   int level, max_level = ilk_wm_max_level(dev);
 
for (level = 0; level = max_level; level++) {
unsigned int latency = wm[level];
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 06/19] drm/i915: Add intel_pipe_wm and prepare for watermark pre-compute

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Introduce a new struct intel_pipe_wm which contains all the
watermarks for a single pipe. Use it to unify the LP0 and LP1+
watermark computations so that we can just iterate through the
watermark levels neatly and call ilk_compute_wm_level() for each.

Also add another tool ilk_wm_merge() that merges the LP1+ watermarks
from all pipes. For that, embed one intel_pipe_wm inside intel_crtc that
contains the currently valid watermarks for each pipe.

This is mainly preparatory work for pre-computing the watermarks for
each pipe and merging them at a later time. For now the merging still
happens immediately.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_drv.h |  12 +++
 drivers/gpu/drm/i915/intel_pm.c  | 190 +++
 2 files changed, 126 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8b132e2..664df77 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -293,6 +293,12 @@ struct intel_crtc_config {
bool ips_enabled;
 };
 
+struct intel_pipe_wm {
+   struct intel_wm_level wm[5];
+   uint32_t linetime;
+   bool fbc_wm_enabled;
+};
+
 struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
@@ -333,6 +339,12 @@ struct intel_crtc {
/* Access to these should be protected by dev_priv-irq_lock. */
bool cpu_fifo_underrun_disabled;
bool pch_fifo_underrun_disabled;
+
+   /* per-pipe watermark state */
+   struct {
+   /* watermarks currently being used  */
+   struct intel_pipe_wm active;
+   } wm;
 };
 
 struct intel_plane_wm_parameters {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 163ba74..c6f105f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2440,53 +2440,6 @@ static void ilk_compute_wm_level(struct drm_i915_private 
*dev_priv,
result-enable = true;
 }
 
-static bool hsw_compute_lp_wm(struct drm_i915_private *dev_priv,
- int level, const struct hsw_wm_maximums *max,
- const struct hsw_pipe_wm_parameters *params,
- struct intel_wm_level *result)
-{
-   enum pipe pipe;
-   struct intel_wm_level res[3];
-
-   for (pipe = PIPE_A; pipe = PIPE_C; pipe++)
-   ilk_compute_wm_level(dev_priv, level, params[pipe], 
res[pipe]);
-
-   result-pri_val = max3(res[0].pri_val, res[1].pri_val, res[2].pri_val);
-   result-spr_val = max3(res[0].spr_val, res[1].spr_val, res[2].spr_val);
-   result-cur_val = max3(res[0].cur_val, res[1].cur_val, res[2].cur_val);
-   result-fbc_val = max3(res[0].fbc_val, res[1].fbc_val, res[2].fbc_val);
-   result-enable = true;
-
-   return ilk_check_wm(level, max, result);
-}
-
-
-static uint32_t hsw_compute_wm_pipe(struct drm_device *dev,
-   const struct hsw_pipe_wm_parameters *params)
-{
-   struct drm_i915_private *dev_priv = dev-dev_private;
-   struct intel_wm_config config = {
-   .num_pipes_active = 1,
-   .sprites_enabled = params-spr.enabled,
-   .sprites_scaled = params-spr.scaled,
-   };
-   struct hsw_wm_maximums max;
-   struct intel_wm_level res;
-
-   if (!params-active)
-   return 0;
-
-   ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);
-
-   ilk_compute_wm_level(dev_priv, 0, params, res);
-
-   ilk_check_wm(0, max, res);
-
-   return (res.pri_val  WM0_PIPE_PLANE_SHIFT) |
-  (res.spr_val  WM0_PIPE_SPRITE_SHIFT) |
-  res.cur_val;
-}
-
 static uint32_t
 hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 {
@@ -2670,44 +2623,121 @@ static void hsw_compute_wm_parameters(struct 
drm_device *dev,
*lp_max_5_6 = *lp_max_1_2;
 }
 
+/* Compute new watermarks for the pipe */
+static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
+ const struct hsw_pipe_wm_parameters *params,
+ struct intel_pipe_wm *pipe_wm)
+{
+   struct drm_device *dev = crtc-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int level, max_level = ilk_wm_max_level(dev);
+   struct intel_wm_config config = {
+   .num_pipes_active = 1,
+   .sprites_enabled = params-spr.enabled,
+   .sprites_scaled = params-spr.scaled,
+   };
+   struct hsw_wm_maximums max;
+
+   memset(pipe_wm, 0, sizeof(*pipe_wm));
+
+   ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);
+
+   for (level = 0; level = max_level; level++)
+   ilk_compute_wm_level(dev_priv, level, params,
+pipe_wm-wm[level]);
+
+   pipe_wm-linetime = 

[Intel-gfx] [PATCH 07/19] drm/i915: Don't re-compute pipe watermarks except for the affected pipe

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

No point in re-computing the watermarks for all pipes, when only one
pipe has changed. The watermarks stored under intel_crtc.wm.active are
still valid for the other pipes. We just need to redo the merging.

We can also skip the merge/update procedure completely if the new
watermarks for the affected pipe come out unchanged.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 67 +
 1 file changed, 28 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c6f105f..1fb904a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2566,29 +2566,19 @@ static void intel_setup_wm_latency(struct drm_device 
*dev)
intel_print_wm_latency(dev, Cursor, dev_priv-wm.cur_latency);
 }
 
-static void hsw_compute_wm_parameters(struct drm_device *dev,
- struct hsw_pipe_wm_parameters *params,
+static void hsw_compute_wm_parameters(struct drm_crtc *crtc,
+ struct hsw_pipe_wm_parameters *p,
  struct hsw_wm_maximums *lp_max_1_2,
  struct hsw_wm_maximums *lp_max_5_6)
 {
-   struct drm_crtc *crtc;
-   struct drm_plane *plane;
-   enum pipe pipe;
+   struct drm_device *dev = crtc-dev;
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   enum pipe pipe = intel_crtc-pipe;
struct intel_wm_config config = {};
+   struct drm_plane *plane;
 
-   list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   struct hsw_pipe_wm_parameters *p;
-
-   pipe = intel_crtc-pipe;
-   p = params[pipe];
-
-   p-active = intel_crtc_active(crtc);
-   if (!p-active)
-   continue;
-
-   config.num_pipes_active++;
-
+   p-active = intel_crtc_active(crtc);
+   if (p-active) {
p-pipe_htotal = intel_crtc-config.adjusted_mode.htotal;
p-pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
p-pri.bytes_per_pixel = crtc-fb-bits_per_pixel / 8;
@@ -2601,17 +2591,17 @@ static void hsw_compute_wm_parameters(struct drm_device 
*dev,
p-cur.enabled = true;
}
 
+   list_for_each_entry(crtc, dev-mode_config.crtc_list, head)
+   config.num_pipes_active += intel_crtc_active(crtc);
+
list_for_each_entry(plane, dev-mode_config.plane_list, head) {
struct intel_plane *intel_plane = to_intel_plane(plane);
-   struct hsw_pipe_wm_parameters *p;
-
-   pipe = intel_plane-pipe;
-   p = params[pipe];
 
-   p-spr = intel_plane-wm;
+   if (intel_plane-pipe == pipe)
+   p-spr = intel_plane-wm;
 
-   config.sprites_enabled |= p-spr.enabled;
-   config.sprites_scaled |= p-spr.scaled;
+   config.sprites_enabled |= intel_plane-wm.enabled;
+   config.sprites_scaled |= intel_plane-wm.scaled;
}
 
ilk_wm_max(dev, 1, config, INTEL_DDB_PART_1_2, lp_max_1_2);
@@ -2638,8 +2628,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
};
struct hsw_wm_maximums max;
 
-   memset(pipe_wm, 0, sizeof(*pipe_wm));
-
ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);
 
for (level = 0; level = max_level; level++)
@@ -2709,7 +2697,6 @@ static void ilk_wm_merge(struct drm_device *dev,
 }
 
 static void hsw_compute_wm_results(struct drm_device *dev,
-  const struct hsw_pipe_wm_parameters *params,
   const struct hsw_wm_maximums *lp_maximums,
   struct hsw_wm_values *results)
 {
@@ -2717,11 +2704,6 @@ static void hsw_compute_wm_results(struct drm_device 
*dev,
int level, wm_lp;
struct intel_pipe_wm lp_wm = {};
 
-   list_for_each_entry(intel_crtc, dev-mode_config.crtc_list, base.head)
-   intel_compute_pipe_wm(intel_crtc-base,
- params[intel_crtc-pipe],
- intel_crtc-wm.active);
-
ilk_wm_merge(dev, lp_maximums, lp_wm);
 
memset(results, 0, sizeof(*results));
@@ -2888,20 +2870,27 @@ static void hsw_write_wm_values(struct drm_i915_private 
*dev_priv,
 
 static void haswell_update_wm(struct drm_crtc *crtc)
 {
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
-   struct hsw_pipe_wm_parameters params[3];
+   struct hsw_pipe_wm_parameters params = {};
  

[Intel-gfx] [PATCH 09/19] drm/i915: Use intel_pipe_wm in hsw_find_best_results

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Let's try to keep using the intermediate intel_pipe_wm representation
for as long as possible. It avoids subtle knowledge about the
internals of the hardware registers when trying to choose the
best watermark configuration.

While at it replace the memset() w/ zero initialization.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 42 -
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b538d09..d1184f9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2703,8 +2703,6 @@ static void hsw_compute_wm_results(struct drm_device *dev,
struct intel_crtc *intel_crtc;
int level, wm_lp;
 
-   memset(results, 0, sizeof(*results));
-
results-enable_fbc_wm = lp_wm-fbc_wm_enabled;
 
/* LP1+ register values */
@@ -2744,24 +2742,26 @@ static void hsw_compute_wm_results(struct drm_device 
*dev,
 
 /* Find the result with the highest level enabled. Check for enable_fbc_wm in
  * case both are at the same level. Prefer r1 in case they're the same. */
-static struct hsw_wm_values *hsw_find_best_result(struct hsw_wm_values *r1,
- struct hsw_wm_values *r2)
+static struct intel_pipe_wm *hsw_find_best_result(struct drm_device *dev,
+ struct intel_pipe_wm *r1,
+ struct intel_pipe_wm *r2)
 {
-   int i, val_r1 = 0, val_r2 = 0;
+   int level, max_level = ilk_wm_max_level(dev);
+   int level1 = 0, level2 = 0;
 
-   for (i = 0; i  3; i++) {
-   if (r1-wm_lp[i]  WM3_LP_EN)
-   val_r1 = r1-wm_lp[i]  WM1_LP_LATENCY_MASK;
-   if (r2-wm_lp[i]  WM3_LP_EN)
-   val_r2 = r2-wm_lp[i]  WM1_LP_LATENCY_MASK;
+   for (level = 1; level = max_level; level++) {
+   if (r1-wm[level].enable)
+   level1 = level;
+   if (r2-wm[level].enable)
+   level2 = level;
}
 
-   if (val_r1 == val_r2) {
-   if (r2-enable_fbc_wm  !r1-enable_fbc_wm)
+   if (level1 == level2) {
+   if (r2-fbc_wm_enabled  !r1-fbc_wm_enabled)
return r2;
else
return r1;
-   } else if (val_r1  val_r2) {
+   } else if (level1  level2) {
return r1;
} else {
return r2;
@@ -2872,10 +2872,10 @@ static void haswell_update_wm(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev-dev_private;
struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
struct hsw_pipe_wm_parameters params = {};
-   struct hsw_wm_values results_1_2, results_5_6, *best_results;
+   struct hsw_wm_values results = {};
enum intel_ddb_partitioning partitioning;
struct intel_pipe_wm pipe_wm = {};
-   struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {};
+   struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
 
hsw_compute_wm_parameters(crtc, params, lp_max_1_2, lp_max_5_6);
 
@@ -2889,18 +2889,18 @@ static void haswell_update_wm(struct drm_crtc *crtc)
ilk_wm_merge(dev, lp_max_1_2, lp_wm_1_2);
ilk_wm_merge(dev, lp_max_5_6, lp_wm_5_6);
 
-   hsw_compute_wm_results(dev, lp_wm_1_2, results_1_2);
if (lp_max_1_2.pri != lp_max_5_6.pri) {
-   hsw_compute_wm_results(dev, lp_wm_5_6, results_5_6);
-   best_results = hsw_find_best_result(results_1_2, results_5_6);
+   best_lp_wm = hsw_find_best_result(dev, lp_wm_1_2, lp_wm_5_6);
} else {
-   best_results = results_1_2;
+   best_lp_wm = lp_wm_1_2;
}
 
-   partitioning = (best_results == results_1_2) ?
+   hsw_compute_wm_results(dev, best_lp_wm, results);
+
+   partitioning = (best_lp_wm == lp_wm_1_2) ?
   INTEL_DDB_PART_1_2 : INTEL_DDB_PART_5_6;
 
-   hsw_write_wm_values(dev_priv, best_results, partitioning);
+   hsw_write_wm_values(dev_priv, results, partitioning);
 }
 
 static void haswell_update_sprite_wm(struct drm_plane *plane,
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 10/19] drm/i915: Move some computations out from hsw_compute_wm_parameters()

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Move the watermark max computations into haswell_update_wm(). This
allows keeping the 1/2 vs. 5/6 split code in one place, and avoid having
to pass around so many things. We also save a bit of stack space by only
requiring one copy of struct hsw_wm_maximums.

Also move the intel_wm_config out from hsw_compute_wm_parameters() and
pass it it. We'll have some need for it in haswell_update_wm() later.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 33 ++---
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d1184f9..5c5fba4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2568,13 +2568,11 @@ static void intel_setup_wm_latency(struct drm_device 
*dev)
 
 static void hsw_compute_wm_parameters(struct drm_crtc *crtc,
  struct hsw_pipe_wm_parameters *p,
- struct hsw_wm_maximums *lp_max_1_2,
- struct hsw_wm_maximums *lp_max_5_6)
+ struct intel_wm_config *config)
 {
struct drm_device *dev = crtc-dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc-pipe;
-   struct intel_wm_config config = {};
struct drm_plane *plane;
 
p-active = intel_crtc_active(crtc);
@@ -2592,7 +2590,7 @@ static void hsw_compute_wm_parameters(struct drm_crtc 
*crtc,
}
 
list_for_each_entry(crtc, dev-mode_config.crtc_list, head)
-   config.num_pipes_active += intel_crtc_active(crtc);
+   config-num_pipes_active += intel_crtc_active(crtc);
 
list_for_each_entry(plane, dev-mode_config.plane_list, head) {
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -2600,17 +2598,9 @@ static void hsw_compute_wm_parameters(struct drm_crtc 
*crtc,
if (intel_plane-pipe == pipe)
p-spr = intel_plane-wm;
 
-   config.sprites_enabled |= intel_plane-wm.enabled;
-   config.sprites_scaled |= intel_plane-wm.scaled;
+   config-sprites_enabled |= intel_plane-wm.enabled;
+   config-sprites_scaled |= intel_plane-wm.scaled;
}
-
-   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_1_2, lp_max_1_2);
-
-   /* 5/6 split only in single pipe config on IVB+ */
-   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active = 1)
-   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_5_6, lp_max_5_6);
-   else
-   *lp_max_5_6 = *lp_max_1_2;
 }
 
 /* Compute new watermarks for the pipe */
@@ -2870,14 +2860,15 @@ static void haswell_update_wm(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
+   struct hsw_wm_maximums max;
struct hsw_pipe_wm_parameters params = {};
struct hsw_wm_values results = {};
enum intel_ddb_partitioning partitioning;
struct intel_pipe_wm pipe_wm = {};
struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
+   struct intel_wm_config config = {};
 
-   hsw_compute_wm_parameters(crtc, params, lp_max_1_2, lp_max_5_6);
+   hsw_compute_wm_parameters(crtc, params, config);
 
intel_compute_pipe_wm(crtc, params, pipe_wm);
 
@@ -2886,10 +2877,14 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 
intel_crtc-wm.active = pipe_wm;
 
-   ilk_wm_merge(dev, lp_max_1_2, lp_wm_1_2);
-   ilk_wm_merge(dev, lp_max_5_6, lp_wm_5_6);
+   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_1_2, max);
+   ilk_wm_merge(dev, max, lp_wm_1_2);
+
+   /* 5/6 split only in single pipe config on IVB+ */
+   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active = 1) {
+   ilk_wm_max(dev, 1, config, INTEL_DDB_PART_5_6, max);
+   ilk_wm_merge(dev, max, lp_wm_5_6);
 
-   if (lp_max_1_2.pri != lp_max_5_6.pri) {
best_lp_wm = hsw_find_best_result(dev, lp_wm_1_2, lp_wm_5_6);
} else {
best_lp_wm = lp_wm_1_2;
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 12/19] drm/i915: Refactor wm_lp to level calculation

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

On HSW the LP1,LP2,LP3 levels are either 1,2,3 or 1,3,4. We make the
conversion from LPn to to the level at one point current. Later we're
going to do it in a few places, so move it to a separate function.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7b4f7d9..ee74352 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2686,6 +2686,12 @@ static void ilk_wm_merge(struct drm_device *dev,
}
 }
 
+static int ilk_wm_lp_to_level(int wm_lp, const struct intel_pipe_wm *pipe_wm)
+{
+   /* LP1,LP2,LP3 levels are either 1,2,3 or 1,3,4 */
+   return wm_lp + (wm_lp = 2  pipe_wm-wm[4].enable);
+}
+
 static void hsw_compute_wm_results(struct drm_device *dev,
   const struct intel_pipe_wm *lp_wm,
   struct hsw_wm_values *results)
@@ -2699,7 +2705,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
for (wm_lp = 1; wm_lp = 3; wm_lp++) {
const struct intel_wm_level *r;
 
-   level = wm_lp + (wm_lp = 2  lp_wm-wm[4].enable);
+   level = ilk_wm_lp_to_level(wm_lp, lp_wm);
 
r = lp_wm-wm[level];
if (!r-enable)
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 13/19] drm/i915: Kill fbc_wm_enabled from intel_wm_config

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

The fbc_wm_enabled member in intel_wm_config is useless for the time
being. The original idea for it was that we'd pre-compute it and so
that the WM merging process could know whether it needs to worry
about FBC watermarks at all.

But we don't have a convenient way to pre-check for the possibility
of FBC being used. intel_update_fbc() should be split up for that.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ee74352..c0f2837 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2195,7 +2195,6 @@ struct intel_wm_config {
unsigned int num_pipes_active;
bool sprites_enabled;
bool sprites_scaled;
-   bool fbc_wm_enabled;
 };
 
 /*
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 08/19] drm/i915: Move LP1+ watermark merging out from hsw_compute_wm_results()

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

I want to convert hsw_find_best_result() to use intel_pipe_wm, so we
need to move the merging to happen outside hsw_compute_wm_results().

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1fb904a..b538d09 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2697,26 +2697,23 @@ static void ilk_wm_merge(struct drm_device *dev,
 }
 
 static void hsw_compute_wm_results(struct drm_device *dev,
-  const struct hsw_wm_maximums *lp_maximums,
+  const struct intel_pipe_wm *lp_wm,
   struct hsw_wm_values *results)
 {
struct intel_crtc *intel_crtc;
int level, wm_lp;
-   struct intel_pipe_wm lp_wm = {};
-
-   ilk_wm_merge(dev, lp_maximums, lp_wm);
 
memset(results, 0, sizeof(*results));
 
-   results-enable_fbc_wm = lp_wm.fbc_wm_enabled;
+   results-enable_fbc_wm = lp_wm-fbc_wm_enabled;
 
/* LP1+ register values */
for (wm_lp = 1; wm_lp = 3; wm_lp++) {
const struct intel_wm_level *r;
 
-   level = wm_lp + (wm_lp = 2  lp_wm.wm[4].enable);
+   level = wm_lp + (wm_lp = 2  lp_wm-wm[4].enable);
 
-   r = lp_wm.wm[level];
+   r = lp_wm-wm[level];
if (!r-enable)
break;
 
@@ -2878,6 +2875,7 @@ static void haswell_update_wm(struct drm_crtc *crtc)
struct hsw_wm_values results_1_2, results_5_6, *best_results;
enum intel_ddb_partitioning partitioning;
struct intel_pipe_wm pipe_wm = {};
+   struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {};
 
hsw_compute_wm_parameters(crtc, params, lp_max_1_2, lp_max_5_6);
 
@@ -2888,9 +2886,12 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 
intel_crtc-wm.active = pipe_wm;
 
-   hsw_compute_wm_results(dev, lp_max_1_2, results_1_2);
+   ilk_wm_merge(dev, lp_max_1_2, lp_wm_1_2);
+   ilk_wm_merge(dev, lp_max_5_6, lp_wm_5_6);
+
+   hsw_compute_wm_results(dev, lp_wm_1_2, results_1_2);
if (lp_max_1_2.pri != lp_max_5_6.pri) {
-   hsw_compute_wm_results(dev, lp_max_5_6, results_5_6);
+   hsw_compute_wm_results(dev, lp_wm_5_6, results_5_6);
best_results = hsw_find_best_result(results_1_2, results_5_6);
} else {
best_results = results_1_2;
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 11/19] drm/i915: Don't compute 5/6 DDB split w/ zero active pipes

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

When there are zero active pipes, all the watermarks should be zero
also. No point in wasting time w/ computing the 5/6 split watermark
config.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5c5fba4..7b4f7d9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2881,7 +2881,7 @@ static void haswell_update_wm(struct drm_crtc *crtc)
ilk_wm_merge(dev, max, lp_wm_1_2);
 
/* 5/6 split only in single pipe config on IVB+ */
-   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active = 1) {
+   if (INTEL_INFO(dev)-gen = 7  config.num_pipes_active == 1) {
ilk_wm_max(dev, 1, config, INTEL_DDB_PART_5_6, max);
ilk_wm_merge(dev, max, lp_wm_5_6);
 
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 14/19] drm/i915: Store current watermark state in dev_priv-wm

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

To make it easier to check what watermark updates are actually
necessary, keep copies of the relevant bits that match the current
hardware state.

Also add DDB partitioning into hsw_wm_values as that's another piece
of state we want to track.

We don't read out the hardware state on init yet, so we can't really
start using this yet, but it will be used later.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_drv.h | 12 +++
 drivers/gpu/drm/i915/intel_pm.c | 46 ++---
 2 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ab46eb3..35db44b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1088,6 +1088,15 @@ struct intel_wm_level {
uint32_t fbc_val;
 };
 
+struct hsw_wm_values {
+   uint32_t wm_pipe[3];
+   uint32_t wm_lp[3];
+   uint32_t wm_lp_spr[3];
+   uint32_t wm_linetime[3];
+   bool enable_fbc_wm;
+   enum intel_ddb_partitioning partitioning;
+};
+
 /*
  * This struct tracks the state needed for the Package C8+ feature.
  *
@@ -1339,6 +1348,9 @@ typedef struct drm_i915_private {
uint16_t spr_latency[5];
/* cursor */
uint16_t cur_latency[5];
+
+   /* current hardware state */
+   struct hsw_wm_values hw;
} wm;
 
struct i915_package_c8 pc8;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c0f2837..27cc69d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2182,14 +2182,6 @@ struct hsw_wm_maximums {
uint16_t fbc;
 };
 
-struct hsw_wm_values {
-   uint32_t wm_pipe[3];
-   uint32_t wm_lp[3];
-   uint32_t wm_lp_spr[3];
-   uint32_t wm_linetime[3];
-   bool enable_fbc_wm;
-};
-
 /* used in computing the new watermarks state */
 struct intel_wm_config {
unsigned int num_pipes_active;
@@ -2693,12 +2685,14 @@ static int ilk_wm_lp_to_level(int wm_lp, const struct 
intel_pipe_wm *pipe_wm)
 
 static void hsw_compute_wm_results(struct drm_device *dev,
   const struct intel_pipe_wm *lp_wm,
+  enum intel_ddb_partitioning partitioning,
   struct hsw_wm_values *results)
 {
struct intel_crtc *intel_crtc;
int level, wm_lp;
 
results-enable_fbc_wm = lp_wm-fbc_wm_enabled;
+   results-partitioning = partitioning;
 
/* LP1+ register values */
for (wm_lp = 1; wm_lp = 3; wm_lp++) {
@@ -2768,13 +2762,10 @@ static struct intel_pipe_wm 
*hsw_find_best_result(struct drm_device *dev,
  * causes WMs to be re-evaluated, expending some power.
  */
 static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
-   struct hsw_wm_values *results,
-   enum intel_ddb_partitioning partitioning)
+   struct hsw_wm_values *results)
 {
struct hsw_wm_values previous;
uint32_t val;
-   enum intel_ddb_partitioning prev_partitioning;
-   bool prev_enable_fbc_wm;
 
previous.wm_pipe[0] = I915_READ(WM0_PIPEA_ILK);
previous.wm_pipe[1] = I915_READ(WM0_PIPEB_ILK);
@@ -2789,21 +2780,12 @@ static void hsw_write_wm_values(struct drm_i915_private 
*dev_priv,
previous.wm_linetime[1] = I915_READ(PIPE_WM_LINETIME(PIPE_B));
previous.wm_linetime[2] = I915_READ(PIPE_WM_LINETIME(PIPE_C));
 
-   prev_partitioning = (I915_READ(WM_MISC)  WM_MISC_DATA_PARTITION_5_6) ?
+   previous.partitioning = (I915_READ(WM_MISC)  
WM_MISC_DATA_PARTITION_5_6) ?
INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
 
-   prev_enable_fbc_wm = !(I915_READ(DISP_ARB_CTL)  DISP_FBC_WM_DIS);
-
-   if (memcmp(results-wm_pipe, previous.wm_pipe,
-  sizeof(results-wm_pipe)) == 0 
-   memcmp(results-wm_lp, previous.wm_lp,
-  sizeof(results-wm_lp)) == 0 
-   memcmp(results-wm_lp_spr, previous.wm_lp_spr,
-  sizeof(results-wm_lp_spr)) == 0 
-   memcmp(results-wm_linetime, previous.wm_linetime,
-  sizeof(results-wm_linetime)) == 0 
-   partitioning == prev_partitioning 
-   results-enable_fbc_wm == prev_enable_fbc_wm)
+   previous.enable_fbc_wm = !(I915_READ(DISP_ARB_CTL)  DISP_FBC_WM_DIS);
+
+   if (memcmp(results, previous, sizeof(*results)) == 0)
return;
 
if (previous.wm_lp[2] != 0)
@@ -2827,16 +2809,16 @@ static void hsw_write_wm_values(struct drm_i915_private 
*dev_priv,
if (previous.wm_linetime[2] != results-wm_linetime[2])
I915_WRITE(PIPE_WM_LINETIME(PIPE_C), results-wm_linetime[2]);
 
-   if (prev_partitioning != partitioning) {
+   if 

[Intel-gfx] [PATCH 15/19] drm/i915: Improve watermark dirtyness checks

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Currently hsw_write_vm_values() may write to certain watermark
registers needlessly. For instance it will disable LP1+ watermarks
around WM_PIPE changes even though that's not needed. Also if only,
say, LP3 changes, the current code will again disable all LP1+
watermarks even though only LP3 needs to be reconfigured.

Add an easy to read function that will compute the dirtyness of the
watermarks, and use that information to further optimize the watermark
programming.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 95 +
 1 file changed, 77 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 27cc69d..82e1af6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2757,6 +2757,63 @@ static struct intel_pipe_wm *hsw_find_best_result(struct 
drm_device *dev,
}
 }
 
+/* dirty bits used to track which watermarks need changes */
+#define WM_DIRTY_PIPE(pipe) (1  (pipe))
+#define WM_DIRTY_LINETIME(pipe) (1  (8 + (pipe)))
+#define WM_DIRTY_LP(wm_lp) (1  (15 + (wm_lp)))
+#define WM_DIRTY_LP_ALL (WM_DIRTY_LP(1) | WM_DIRTY_LP(2) | WM_DIRTY_LP(3))
+#define WM_DIRTY_FBC (1  24)
+#define WM_DIRTY_DDB (1  25)
+
+static unsigned int ilk_compute_wm_dirty(struct drm_device *dev,
+const struct hsw_wm_values *old,
+const struct hsw_wm_values *new)
+{
+   unsigned int dirty = 0;
+   enum pipe pipe;
+   int wm_lp;
+
+   for_each_pipe(pipe) {
+   if (old-wm_linetime[pipe] != new-wm_linetime[pipe]) {
+   dirty |= WM_DIRTY_LINETIME(pipe);
+   /* Must disable LP1+ watermarks too */
+   dirty |= WM_DIRTY_LP_ALL;
+   }
+
+   if (old-wm_pipe[pipe] != new-wm_pipe[pipe])
+   dirty |= WM_DIRTY_PIPE(pipe);
+   }
+
+   if (old-enable_fbc_wm != new-enable_fbc_wm) {
+   dirty |= WM_DIRTY_FBC;
+   /* Must disable LP1+ watermarks too */
+   dirty |= WM_DIRTY_LP_ALL;
+   }
+
+   if (old-partitioning != new-partitioning) {
+   dirty |= WM_DIRTY_DDB;
+   /* Must disable LP1+ watermarks too */
+   dirty |= WM_DIRTY_LP_ALL;
+   }
+
+   /* LP1+ watermarks already deemed dirty, no need to continue */
+   if (dirty  WM_DIRTY_LP_ALL)
+   return dirty;
+
+   /* Find the lowest numbered LP1+ watermark in need of an update... */
+   for (wm_lp = 1; wm_lp = 3; wm_lp++) {
+   if (old-wm_lp[wm_lp - 1] != new-wm_lp[wm_lp - 1] ||
+   old-wm_lp_spr[wm_lp - 1] != new-wm_lp_spr[wm_lp - 1])
+   break;
+   }
+
+   /* ...and mark it and all higher numbered LP1+ watermarks as dirty */
+   for (; wm_lp = 3; wm_lp++)
+   dirty |= WM_DIRTY_LP(wm_lp);
+
+   return dirty;
+}
+
 /*
  * The spec says we shouldn't write when we don't need, because every write
  * causes WMs to be re-evaluated, expending some power.
@@ -2765,6 +2822,7 @@ static void hsw_write_wm_values(struct drm_i915_private 
*dev_priv,
struct hsw_wm_values *results)
 {
struct hsw_wm_values previous;
+   unsigned int dirty;
uint32_t val;
 
previous.wm_pipe[0] = I915_READ(WM0_PIPEA_ILK);
@@ -2785,31 +2843,32 @@ static void hsw_write_wm_values(struct drm_i915_private 
*dev_priv,
 
previous.enable_fbc_wm = !(I915_READ(DISP_ARB_CTL)  DISP_FBC_WM_DIS);
 
-   if (memcmp(results, previous, sizeof(*results)) == 0)
+   dirty = ilk_compute_wm_dirty(dev_priv-dev, previous, results);
+   if (!dirty)
return;
 
-   if (previous.wm_lp[2] != 0)
+   if (dirty  WM_DIRTY_LP(3)  previous.wm_lp[2] != 0)
I915_WRITE(WM3_LP_ILK, 0);
-   if (previous.wm_lp[1] != 0)
+   if (dirty  WM_DIRTY_LP(2)  previous.wm_lp[1] != 0)
I915_WRITE(WM2_LP_ILK, 0);
-   if (previous.wm_lp[0] != 0)
+   if (dirty  WM_DIRTY_LP(1)  previous.wm_lp[0] != 0)
I915_WRITE(WM1_LP_ILK, 0);
 
-   if (previous.wm_pipe[0] != results-wm_pipe[0])
+   if (dirty  WM_DIRTY_PIPE(PIPE_A))
I915_WRITE(WM0_PIPEA_ILK, results-wm_pipe[0]);
-   if (previous.wm_pipe[1] != results-wm_pipe[1])
+   if (dirty  WM_DIRTY_PIPE(PIPE_B))
I915_WRITE(WM0_PIPEB_ILK, results-wm_pipe[1]);
-   if (previous.wm_pipe[2] != results-wm_pipe[2])
+   if (dirty  WM_DIRTY_PIPE(PIPE_C))
I915_WRITE(WM0_PIPEC_IVB, results-wm_pipe[2]);
 
-   if (previous.wm_linetime[0] != results-wm_linetime[0])
+   if (dirty  WM_DIRTY_LINETIME(PIPE_A))
I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results-wm_linetime[0]);
-   if 

[Intel-gfx] [PATCH 17/19] drm/i915: Remove a somewhat silly debug print from watermark code

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

This debug print just adds overhead to the watermark merging process,
and doesn't really give enough information to be useful. Just kill
and let's add something much better a bit later.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fe44e86..62c85d9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2403,8 +2403,6 @@ static bool ilk_check_wm(int level,
result-enable = true;
}
 
-   DRM_DEBUG_KMS(WM%d: %sabled\n, level, result-enable ? en : dis);
-
return ret;
 }
 
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 16/19] drm/i915: Init HSW watermark tracking in intel_modeset_setup_hw_state()

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

Fill out the HSW watermark s/w tracking structures with the current
hardware state in intel_modeset_setup_hw_state(). This allows us to skip
the HW state readback during watermark programming and just use the values
we keep around in dev_priv-wm. Reduces the overhead of the watermark
programming quite a bit.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_display.c |   3 +
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 drivers/gpu/drm/i915/intel_pm.c  | 104 ++-
 3 files changed, 82 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a5181fe..738e285 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10473,6 +10473,9 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
pll-on = false;
}
 
+   if (IS_HASWELL(dev))
+   ilk_init_wm(dev);
+
if (force_restore) {
/*
 * We need to use raw interfaces for restoring state to avoid
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 664df77..a684d5e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -809,5 +809,6 @@ extern void hsw_pc8_disable_interrupts(struct drm_device 
*dev);
 extern void hsw_pc8_restore_interrupts(struct drm_device *dev);
 extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
+extern void ilk_init_wm(struct drm_device *dev);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 82e1af6..fe44e86 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2821,37 +2821,19 @@ static unsigned int ilk_compute_wm_dirty(struct 
drm_device *dev,
 static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
struct hsw_wm_values *results)
 {
-   struct hsw_wm_values previous;
+   struct hsw_wm_values *previous = dev_priv-wm.hw;
unsigned int dirty;
uint32_t val;
 
-   previous.wm_pipe[0] = I915_READ(WM0_PIPEA_ILK);
-   previous.wm_pipe[1] = I915_READ(WM0_PIPEB_ILK);
-   previous.wm_pipe[2] = I915_READ(WM0_PIPEC_IVB);
-   previous.wm_lp[0] = I915_READ(WM1_LP_ILK);
-   previous.wm_lp[1] = I915_READ(WM2_LP_ILK);
-   previous.wm_lp[2] = I915_READ(WM3_LP_ILK);
-   previous.wm_lp_spr[0] = I915_READ(WM1S_LP_ILK);
-   previous.wm_lp_spr[1] = I915_READ(WM2S_LP_IVB);
-   previous.wm_lp_spr[2] = I915_READ(WM3S_LP_IVB);
-   previous.wm_linetime[0] = I915_READ(PIPE_WM_LINETIME(PIPE_A));
-   previous.wm_linetime[1] = I915_READ(PIPE_WM_LINETIME(PIPE_B));
-   previous.wm_linetime[2] = I915_READ(PIPE_WM_LINETIME(PIPE_C));
-
-   previous.partitioning = (I915_READ(WM_MISC)  
WM_MISC_DATA_PARTITION_5_6) ?
-   INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
-
-   previous.enable_fbc_wm = !(I915_READ(DISP_ARB_CTL)  DISP_FBC_WM_DIS);
-
-   dirty = ilk_compute_wm_dirty(dev_priv-dev, previous, results);
+   dirty = ilk_compute_wm_dirty(dev_priv-dev, previous, results);
if (!dirty)
return;
 
-   if (dirty  WM_DIRTY_LP(3)  previous.wm_lp[2] != 0)
+   if (dirty  WM_DIRTY_LP(3)  previous-wm_lp[2] != 0)
I915_WRITE(WM3_LP_ILK, 0);
-   if (dirty  WM_DIRTY_LP(2)  previous.wm_lp[1] != 0)
+   if (dirty  WM_DIRTY_LP(2)  previous-wm_lp[1] != 0)
I915_WRITE(WM2_LP_ILK, 0);
-   if (dirty  WM_DIRTY_LP(1)  previous.wm_lp[0] != 0)
+   if (dirty  WM_DIRTY_LP(1)  previous-wm_lp[0] != 0)
I915_WRITE(WM1_LP_ILK, 0);
 
if (dirty  WM_DIRTY_PIPE(PIPE_A))
@@ -2886,11 +2868,11 @@ static void hsw_write_wm_values(struct drm_i915_private 
*dev_priv,
I915_WRITE(DISP_ARB_CTL, val);
}
 
-   if (dirty  WM_DIRTY_LP(1)  previous.wm_lp_spr[0] != 
results-wm_lp_spr[0])
+   if (dirty  WM_DIRTY_LP(1)  previous-wm_lp_spr[0] != 
results-wm_lp_spr[0])
I915_WRITE(WM1S_LP_ILK, results-wm_lp_spr[0]);
-   if (dirty  WM_DIRTY_LP(2)  previous.wm_lp_spr[1] != 
results-wm_lp_spr[1])
+   if (dirty  WM_DIRTY_LP(2)  previous-wm_lp_spr[1] != 
results-wm_lp_spr[1])
I915_WRITE(WM2S_LP_IVB, results-wm_lp_spr[1]);
-   if (dirty  WM_DIRTY_LP(3)  previous.wm_lp_spr[2] != 
results-wm_lp_spr[2])
+   if (dirty  WM_DIRTY_LP(3)  previous-wm_lp_spr[2] != 
results-wm_lp_spr[2])
I915_WRITE(WM3S_LP_IVB, results-wm_lp_spr[2]);
 
if (dirty  WM_DIRTY_LP(1)  results-wm_lp[0] != 0)
@@ -3123,6 +3105,76 @@ static void sandybridge_update_sprite_wm(struct 
drm_plane *plane,
I915_WRITE(WM3S_LP_IVB, 

[Intel-gfx] [PATCH 18/19] drm/i915: Adjust watermark register masks

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

We want to be able to use the masks to decode the register contents
regardless of the hardware generation. So just expand the masks to
cover all available bits, even if those are reserved on some
generations.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_reg.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 35a6c74..ff23680 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3219,11 +3219,11 @@
 
 /* define the Watermark register on Ironlake */
 #define WM0_PIPEA_ILK  0x45100
-#define  WM0_PIPE_PLANE_MASK   (0x7f16)
+#define  WM0_PIPE_PLANE_MASK   (0x16)
 #define  WM0_PIPE_PLANE_SHIFT  16
-#define  WM0_PIPE_SPRITE_MASK  (0x3f8)
+#define  WM0_PIPE_SPRITE_MASK  (0xff8)
 #define  WM0_PIPE_SPRITE_SHIFT 8
-#define  WM0_PIPE_CURSOR_MASK  (0x1f)
+#define  WM0_PIPE_CURSOR_MASK  (0xff)
 
 #define WM0_PIPEB_ILK  0x45104
 #define WM0_PIPEC_IVB  0x45200
@@ -3233,9 +3233,9 @@
 #define  WM1_LP_LATENCY_MASK   (0x7f24)
 #define  WM1_LP_FBC_MASK   (0xf20)
 #define  WM1_LP_FBC_SHIFT  20
-#define  WM1_LP_SR_MASK(0x1ff8)
+#define  WM1_LP_SR_MASK(0xfff8)
 #define  WM1_LP_SR_SHIFT   8
-#define  WM1_LP_CURSOR_MASK(0x3f)
+#define  WM1_LP_CURSOR_MASK(0xff)
 #define WM2_LP_ILK 0x4510c
 #define  WM2_LP_EN (131)
 #define WM3_LP_ILK 0x45110
-- 
1.8.1.5

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


[Intel-gfx] [PATCH 19/19] drm/i915: Add watermark tracepoints

2013-08-30 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

We may want to know what kind of watermarks got computed and programmed
into the hardware. Using tracepoints is much leaner than debug prints.

Also add trace call for the watermark state we read out of the
hardware during init, though I;m not sure there's any way to see that
trace as the events aren't available until the module is loaded.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_trace.h | 181 ++
 drivers/gpu/drm/i915/intel_pm.c   |  42 -
 2 files changed, 220 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index e2c5ee6..6dc9a91 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -14,6 +14,187 @@
 #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
 #define TRACE_INCLUDE_FILE i915_trace
 
+/* watermark */
+
+TRACE_EVENT(i915_wm_update_start,
+   TP_PROTO(enum pipe pipe),
+   TP_ARGS(pipe),
+
+   TP_STRUCT__entry(
+__field(enum pipe, pipe)
+),
+
+   TP_fast_assign(
+  __entry-pipe = pipe;
+  ),
+
+   TP_printk(pipe %c, pipe_name(__entry-pipe))
+);
+
+TRACE_EVENT(i915_wm_update_end,
+   TP_PROTO(enum pipe pipe, bool changed),
+   TP_ARGS(pipe, changed),
+
+   TP_STRUCT__entry(
+__field(enum pipe, pipe)
+__field(bool, changed)
+),
+
+   TP_fast_assign(
+  __entry-pipe = pipe;
+  __entry-changed = changed;
+  ),
+
+   TP_printk(pipe %c, changed=%s,
+ pipe_name(__entry-pipe), __entry-changed ? yes : no)
+);
+
+TRACE_EVENT_CONDITION(i915_wm_misc,
+   TP_PROTO(const struct hsw_wm_values *hw, bool trace),
+   TP_ARGS(hw, trace),
+
+   TP_CONDITION(trace),
+
+   TP_STRUCT__entry(
+   __field(bool, enable_fbc_wm)
+   __field(enum intel_ddb_partitioning, partitioning)
+   ),
+
+   TP_fast_assign(
+   __entry-enable_fbc_wm = hw-enable_fbc_wm;
+   __entry-partitioning = hw-partitioning;
+   ),
+
+   TP_printk(fbc=%s, ddb partitioning=%s,
+   __entry-enable_fbc_wm ? yes : no,
+   __entry-partitioning == INTEL_DDB_PART_5_6 ? 5/6 : 1/2)
+);
+
+TRACE_EVENT_CONDITION(i915_wm_pipe,
+   TP_PROTO(struct drm_device *dev, enum pipe pipe, const struct 
hsw_wm_values *hw, bool trace),
+   TP_ARGS(dev, pipe, hw, trace),
+
+   TP_CONDITION(pipe  INTEL_INFO(dev)-num_pipes  trace),
+
+   TP_STRUCT__entry(
+   __field(enum pipe, pipe)
+   __field(uint32_t, wm_pipe)
+   ),
+
+   TP_fast_assign(
+   __entry-pipe = pipe;
+   __entry-wm_pipe = hw-wm_pipe[pipe];
+   ),
+
+   TP_printk(pipe %c: pri=%u, spr=%u, cur=%u,
+   pipe_name(__entry-pipe),
+   (__entry-wm_pipe  WM0_PIPE_PLANE_MASK)  
WM0_PIPE_PLANE_SHIFT,
+   (__entry-wm_pipe  WM0_PIPE_SPRITE_MASK)  
WM0_PIPE_SPRITE_SHIFT,
+   __entry-wm_pipe  WM0_PIPE_CURSOR_MASK)
+);
+
+TRACE_EVENT_CONDITION(i915_wm_linetime,
+   TP_PROTO(struct drm_device *dev, enum pipe pipe, const struct 
hsw_wm_values *hw, bool trace),
+   TP_ARGS(dev, pipe, hw, trace),
+
+   TP_CONDITION(IS_HASWELL(dev)  pipe  INTEL_INFO(dev)-num_pipes  
trace),
+
+   TP_STRUCT__entry(
+   __field(enum pipe, pipe)
+   __field(uint32_t, wm_linetime)
+   ),
+
+   TP_fast_assign(
+   __entry-pipe = pipe;
+   __entry-wm_linetime = hw-wm_linetime[pipe];
+   ),
+
+   TP_printk(pipe %c: linetime=%u, ips linetime=%u,
+   pipe_name(__entry-pipe),
+   __entry-wm_linetime  PIPE_WM_LINETIME_MASK,
+   (__entry-wm_linetime  PIPE_WM_LINETIME_IPS_LINETIME_MASK)  
16)
+);
+
+
+TRACE_EVENT_CONDITION(i915_wm_lp1_ilk,
+   TP_PROTO(struct drm_device *dev, const struct hsw_wm_values *hw, bool 
trace),
+   TP_ARGS(dev, hw, trace),
+
+   TP_CONDITION(INTEL_INFO(dev)-gen = 6  trace),
+
+   TP_STRUCT__entry(
+   __field(uint32_t, wm_lp)
+   __field(uint32_t, wm_lp_spr)
+   ),
+
+   TP_fast_assign(
+   __entry-wm_lp = hw-wm_lp[0];
+   __entry-wm_lp_spr = hw-wm_lp_spr[0];
+   ),
+
+   TP_printk(LP1: en=%s, lat=%u, fbc=%u, pri=%u, cur=%u, spr=%u, spr 
en=%s,
+   __entry-wm_lp  WM1_LP_SR_EN ? yes : no,
+   (__entry-wm_lp  WM1_LP_LATENCY_MASK)  WM1_LP_LATENCY_SHIFT,
+   (__entry-wm_lp  WM1_LP_FBC_MASK)  WM1_LP_FBC_SHIFT,
+   

Re: [Intel-gfx] [PATCH 0/2] vgaarb: Fixes for partial VGA opt-out

2013-08-30 Thread Ville Syrjälä
On Thu, Aug 15, 2013 at 04:37:47PM -0600, Alex Williamson wrote:
 I'm trying to add support for VGA arbitration on newer Intel graphics
 devices.  The existing code attempts to do this, but appear to have
 not been updated since GMCH devices roamed the Earth.  On newer
 devices like Haswell, we can disable VGA memory through an MSR on the
 device, but we rely on the VGA arbiter to manage VGA IO using the PCI
 COMMAND register.  In trying to unregister legacy VGA memory, I found
 that the VGA arbiter still wanted to disable both memory and IO on
 the device and that it forgot to actually program the device to
 disable IO when the decoding is updated.  This series attempts to fix
 both of those.  Thanks,

The series looks good to me.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 
 Alex
 
 ---
 
 Alex Williamson (2):
   vgaarb: Don't disable resources that are not owned
   vgaarb: Fix VGA decodes changes
 
 
  drivers/gpu/vga/vgaarb.c |   50 
 --
  1 file changed, 22 insertions(+), 28 deletions(-)

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: ban badly behaving contexts

2013-08-30 Thread Mika Kuoppala
Now when we have mechanism in place to track which context
was guilty of hanging the gpu, it is possible to punish
for bad behaviour.

If context has recently submitted a faulty batchbuffers guilty of
gpu hang and submits another batch which hangs gpu in quick
succession, ban it permanently. If ctx is banned, no more
batchbuffers will be queued for execution.

There is no need for global wedge machinery anymore and
it would be unwise to wedge the whole gpu if we have multiple
hanging batches queued for execution. Instead just ban
the guilty ones and carry on.

v2: Store guilty ban status bool in gpu_error instead of pointers
that might become danling before hang is declared.

v3: Use return value for banned status instead of stashing state
into gpu_error (Chris Wilson)

v4: - rebase on top of fixed hang stats api
- add define for ban period
- rename commit and improve commit msg

v5: - rely context banning instead of wedging the gpu
- beautification and fix for ban calculation (Chris)

Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c|   29 
 drivers/gpu/drm/i915/i915_drv.h|   11 +--
 drivers/gpu/drm/i915/i915_gem.c|   22 +++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   12 
 4 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index eb56f68..a3beef3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -719,24 +719,19 @@ int i915_reset(struct drm_device *dev)
 
simulated = dev_priv-gpu_error.stop_rings != 0;
 
-   if (!simulated  get_seconds() - dev_priv-gpu_error.last_reset  5) {
-   DRM_ERROR(GPU hanging too fast, declaring wedged!\n);
-   ret = -ENODEV;
-   } else {
-   ret = intel_gpu_reset(dev);
-
-   /* Also reset the gpu hangman. */
-   if (simulated) {
-   DRM_INFO(Simulated gpu hang, resetting stop_rings\n);
-   dev_priv-gpu_error.stop_rings = 0;
-   if (ret == -ENODEV) {
-   DRM_ERROR(Reset not implemented, but ignoring 
- error for simulated gpu hangs\n);
-   ret = 0;
-   }
-   } else
-   dev_priv-gpu_error.last_reset = get_seconds();
+   ret = intel_gpu_reset(dev);
+
+   /* Also reset the gpu hangman. */
+   if (simulated) {
+   DRM_INFO(Simulated gpu hang, resetting stop_rings\n);
+   dev_priv-gpu_error.stop_rings = 0;
+   if (ret == -ENODEV) {
+   DRM_ERROR(Reset not implemented, but ignoring 
+ error for simulated gpu hangs\n);
+   ret = 0;
+   }
}
+
if (ret) {
DRM_ERROR(Failed to reset chip.\n);
mutex_unlock(dev-struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4da1f86..2c5f3bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -583,6 +583,12 @@ struct i915_ctx_hang_stats {
 
/* This context had batch active when hang was declared */
unsigned batch_active;
+
+   /* Time when this context was last blamed for a GPU reset */
+   unsigned long guilty_ts;
+
+   /* This context is banned to submit more work */
+   bool banned;
 };
 
 /* This must match up with the value previously used for execbuf2.rsvd1. */
@@ -984,6 +990,9 @@ struct i915_gpu_error {
/* For hangcheck timer */
 #define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
 #define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
+   /* Hang gpu twice in this window and your context gets banned */
+#define DRM_I915_CTX_BAN_PERIOD DIV_ROUND_UP(8*DRM_I915_HANGCHECK_PERIOD, 1000)
+
struct timer_list hangcheck_timer;
 
/* For reset and error_state handling. */
@@ -992,8 +1001,6 @@ struct i915_gpu_error {
struct drm_i915_error_state *first_error;
struct work_struct work;
 
-   unsigned long last_reset;
-
/**
 * State variable and reset counter controlling the reset flow
 *
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7392f64..754f875 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2201,6 +2201,21 @@ static bool i915_request_guilty(struct 
drm_i915_gem_request *request,
return false;
 }
 
+static bool i915_context_is_banned(const struct i915_ctx_hang_stats *hs)
+{
+   const unsigned long elapsed = get_seconds() - hs-guilty_ts;
+
+   if (hs-banned)
+   return true;
+
+   if (elapsed = 

[Intel-gfx] [PATCH 2/2] drm/i915: add hangcheck action and score to error state

2013-08-30 Thread Mika Kuoppala
Score and action reveal what the rings were doing
when hang was declared.

Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h   |2 ++
 drivers/gpu/drm/i915/i915_gpu_error.c |   22 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2c5f3bc..16629cc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -326,6 +326,8 @@ struct drm_i915_error_state {
u32 *active_bo_count, *pinned_bo_count;
struct intel_overlay_error_state *overlay;
struct intel_display_error_state *display;
+   int hangcheck_score[I915_NUM_RINGS];
+   enum intel_ring_hangcheck_action hangcheck_action[I915_NUM_RINGS];
 };
 
 struct intel_crtc_config;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index aba9d74..7e92ba8 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -213,6 +213,22 @@ static void print_error_buffers(struct 
drm_i915_error_state_buf *m,
}
 }
 
+static const char *hangcheck_action_to_str(enum intel_ring_hangcheck_action a)
+{
+   switch (a) {
+   case HANGCHECK_WAIT:
+   return wait;
+   case HANGCHECK_ACTIVE:
+   return active;
+   case HANGCHECK_KICK:
+   return kick;
+   case HANGCHECK_HUNG:
+   return hung;
+   }
+
+   return unknown;
+}
+
 static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
  struct drm_device *dev,
  struct drm_i915_error_state *error,
@@ -253,6 +269,9 @@ static void i915_ring_error_state(struct 
drm_i915_error_state_buf *m,
err_printf(m,   waiting: %s\n, yesno(error-waiting[ring]));
err_printf(m,   ring-head: 0x%08x\n, error-cpu_ring_head[ring]);
err_printf(m,   ring-tail: 0x%08x\n, error-cpu_ring_tail[ring]);
+   err_printf(m,   hangcheck_action: %s\n,
+  hangcheck_action_to_str(error-hangcheck_action[ring]));
+   err_printf(m,   hangcheck_score: %d\n, error-hangcheck_score[ring]);
 }
 
 void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
@@ -718,6 +737,9 @@ static void i915_record_ring_state(struct drm_device *dev,
 
error-cpu_ring_head[ring-id] = ring-head;
error-cpu_ring_tail[ring-id] = ring-tail;
+
+   error-hangcheck_score[ring-id] = ring-hangcheck.score;
+   error-hangcheck_action[ring-id] = ring-hangcheck.action;
 }
 
 
-- 
1.7.9.5

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


[Intel-gfx] [PATCH] drm/i915: It's its!

2013-08-30 Thread Damien Lespiau
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 403309c..e038513 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -73,7 +73,7 @@
  *
  * There are two confusing terms used above:
  *  The current context means the context which is currently running on the
- *  GPU. The GPU has loaded it's state already and has stored away the gtt
+ *  GPU. The GPU has loaded its state already and has stored away the gtt
  *  offset of the BO. The GPU is not actively referencing the data at this
  *  offset, but it will on the next context switch. The only way to avoid this
  *  is to do a GPU reset.
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch

2013-08-30 Thread Damien Lespiau
On Mon, Aug 26, 2013 at 07:50:53PM -0300, Rodrigo Vivi wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 We use the request to ensure we hold a reference to the context for the
 duration that it remains in use by the ring. Each request only holds a
 reference to the current context, hence we emit a request after
 switching contexts with the final reference to the old context. However,
 the extra interrupt caused by that request is not useful (no timing
 critical function will wait for the context object), instead the overhead
 of servicing the IRQ shows up in some (lightweight) benchmarks. In order
 to keep the useful property of using the request to manage the context
 lifetime, we want to add a dummy request that is associated with the
 interrupt from the subsequent real request following the batch.
 
 The extra interrupt was added as a side-effect of using
 i915_add_request() in
 
 commit 112522f6789581824903f6f72082b5b841a7f0f9
 Author: Chris Wilson ch...@chris-wilson.co.uk
 Date:   Thu May 2 16:48:07 2013 +0300
 
 drm/i915: put context upon switching
 
 v2: Daniel convinced me that the request here was solely for context
 lifetime tracking and that we have the active ref to keep the object
 alive whilst the MI_SET_CONTEXT. So the only concern then is which
 context should get the blame for MI_SET_CONTEXT failing. The old scheme
 added a request for the old context so that any hang upto and including
 the switch away would mark the old context as guilty. Now any hang here
 implicates the new context. However since we have already gone through a
 complete flush with the last context in its last request, and all that
 lies in no-man's-land is an invalidate flush and the MI_SET_CONTEXT, we
 should be safe in not unduly placing blame on the new context.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky b...@bwidawsk.net
 Cc: Paulo Zanoni paulo.r.zan...@intel.com

I'm somewhat new to the core GEM code, but I'm convinced by the commit
message and the patch does what it says. So:

Reviewed-by: Damien Lespiau damien.lesp...@intel.com

-- 
Damien

 ---
  drivers/gpu/drm/i915/i915_gem.c |  1 -
  drivers/gpu/drm/i915/i915_gem_context.c | 12 +---
  2 files changed, 1 insertion(+), 12 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 2d1cb10..56c9104 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2046,7 +2046,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
   if (request == NULL)
   return -ENOMEM;
  
 -
   /* Record the position of the start of the request so that
* should we detect the updated seqno part-way through the
* GPU processing the request, we never over-estimate the
 diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
 b/drivers/gpu/drm/i915/i915_gem_context.c
 index 403309c..b6da70b 100644
 --- a/drivers/gpu/drm/i915/i915_gem_context.c
 +++ b/drivers/gpu/drm/i915/i915_gem_context.c
 @@ -451,17 +451,7 @@ static int do_switch(struct i915_hw_context *to)
   from-obj-dirty = 1;
   BUG_ON(from-obj-ring != ring);
  
 - ret = i915_add_request(ring, NULL);
 - if (ret) {
 - /* Too late, we've already scheduled a context switch.
 -  * Try to undo the change so that the hw state is
 -  * consistent with out tracking. In case of emergency,
 -  * scream.
 -  */
 - WARN_ON(mi_set_context(ring, from, MI_RESTORE_INHIBIT));
 - return ret;
 - }
 -
 + /* obj is kept alive until the next request by its active ref */
   i915_gem_object_unpin(from-obj);
   i915_gem_context_unreference(from);
   }
 -- 
 1.8.1.4
 
 ___
 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 03/17] drm/i915: Pin pages whilst mapping the dma-buf

2013-08-30 Thread Damien Lespiau
On Mon, Aug 26, 2013 at 07:50:55PM -0300, Rodrigo Vivi wrote:
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 As we attempt to kmalloc after calling get_pages, there is a possibility
 that the shrinker may reap the pages we just acquired. To prevent this
 we need to increment the pages_pin_count early, so rearrange the code
 and error paths to make it so.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Reviewed-by: Damien Lespiau damien.lesp...@intel.com

-- 
Damien

 ---
  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 41 
 ++
  1 file changed, 22 insertions(+), 19 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
 b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
 index e918b05..7d5752f 100644
 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
 +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
 @@ -42,27 +42,24 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
 dma_buf_attachment *attachme
  
   ret = i915_mutex_lock_interruptible(obj-base.dev);
   if (ret)
 - return ERR_PTR(ret);
 + goto err;
  
   ret = i915_gem_object_get_pages(obj);
 - if (ret) {
 - st = ERR_PTR(ret);
 - goto out;
 - }
 + if (ret)
 + goto err_unlock;
 +
 + i915_gem_object_pin_pages(obj);
  
   /* Copy sg so that we make an independent mapping */
   st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
   if (st == NULL) {
 - st = ERR_PTR(-ENOMEM);
 - goto out;
 + ret = -ENOMEM;
 + goto err_unpin;
   }
  
   ret = sg_alloc_table(st, obj-pages-nents, GFP_KERNEL);
 - if (ret) {
 - kfree(st);
 - st = ERR_PTR(ret);
 - goto out;
 - }
 + if (ret)
 + goto err_free;
  
   src = obj-pages-sgl;
   dst = st-sgl;
 @@ -73,17 +70,23 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
 dma_buf_attachment *attachme
   }
  
   if (!dma_map_sg(attachment-dev, st-sgl, st-nents, dir)) {
 - sg_free_table(st);
 - kfree(st);
 - st = ERR_PTR(-ENOMEM);
 - goto out;
 + ret =-ENOMEM;
 + goto err_free_sg;
   }
  
 - i915_gem_object_pin_pages(obj);
 -
 -out:
   mutex_unlock(obj-base.dev-struct_mutex);
   return st;
 +
 +err_free_sg:
 + sg_free_table(st);
 +err_free:
 + kfree(st);
 +err_unpin:
 + i915_gem_object_unpin_pages(obj);
 +err_unlock:
 + mutex_unlock(obj-base.dev-struct_mutex);
 +err:
 + return ERR_PTR(ret);
  }
  
  static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
 -- 
 1.8.1.4
 
 ___
 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] drm/i915: Don't call sg_free_table() if sg_alloc_table() fails

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 03:39:26PM +0100, Damien Lespiau wrote:
 One needs to call __sg_free_table() if __sg_alloc_table() fails, but
 sg_alloc_table() does that for us already.
 
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewd-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Don't call sg_free_table() if sg_alloc_table() fails

2013-08-30 Thread Damien Lespiau
One needs to call __sg_free_table() if __sg_alloc_table() fails, but
sg_alloc_table() does that for us already.

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_gem.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f705314..8eb8e17 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1772,7 +1772,6 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
 
page_count = obj-base.size / PAGE_SIZE;
if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
-   sg_free_table(st);
kfree(st);
return -ENOMEM;
}
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state

2013-08-30 Thread Jani Nikula
Notifying the bios lets it enter power saving states.

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h   |7 +++
 drivers/gpu/drm/i915/intel_opregion.c |   27 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5339297..7daae2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2190,6 +2190,8 @@ extern void intel_opregion_fini(struct drm_device *dev);
 extern void intel_opregion_asle_intr(struct drm_device *dev);
 extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 bool enable);
+extern int intel_opregion_notify_adapter(struct drm_device *dev,
+pci_power_t state);
 #else
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
 static inline void intel_opregion_fini(struct drm_device *dev) { return; }
@@ -2199,6 +2201,11 @@ intel_opregion_notify_encoder(struct intel_encoder 
*intel_encoder, bool enable)
 {
return 0;
 }
+static inline int
+intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
+{
+   return 0;
+}
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index 791991b..05243df 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -342,6 +342,33 @@ int intel_opregion_notify_encoder(struct intel_encoder 
*intel_encoder,
return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
 }
 
+static const struct {
+   pci_power_t pci_power_state;
+   u32 parm;
+} power_state_map[] = {
+   { PCI_D0,   0x00 },
+   { PCI_D1,   0x01 },
+   { PCI_D2,   0x02 },
+   { PCI_D3hot,0x04 },
+   { PCI_D3cold,   0x04 },
+};
+
+int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
+{
+   int i;
+
+   if (!HAS_DDI(dev))
+   return 0;
+
+   for (i = 0; i  ARRAY_SIZE(power_state_map); i++) {
+   if (state == power_state_map[i].pci_power_state)
+   return swsci(dev, SWSCI_SBCB_ADAPTER_POWER_STATE,
+power_state_map[i].parm, NULL);
+   }
+
+   return -EINVAL;
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable

2013-08-30 Thread Jani Nikula
The spec says to notify prior to power down and after power up. It is
unclear whether it makes a difference.

Signed-off-by: Jani Nikula jani.nik...@intel.com
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

---

Paulo, still okay with the r-b?
---
 drivers/gpu/drm/i915/intel_display.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9222e570..83d853f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3425,8 +3425,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_update_fbc(dev);
mutex_unlock(dev-struct_mutex);
 
-   for_each_encoder_on_crtc(dev, crtc, encoder)
+   for_each_encoder_on_crtc(dev, crtc, encoder) {
encoder-enable(encoder);
+   intel_opregion_notify_encoder(encoder, true);
+   }
 
/*
 * There seems to be a race in PCH platform hw (at least on some
@@ -3540,8 +3542,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
if (!intel_crtc-active)
return;
 
-   for_each_encoder_on_crtc(dev, crtc, encoder)
+   for_each_encoder_on_crtc(dev, crtc, encoder) {
+   intel_opregion_notify_encoder(encoder, false);
encoder-disable(encoder);
+   }
 
intel_crtc_wait_for_pending_flips(crtc);
drm_vblank_off(dev, pipe);
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable

2013-08-30 Thread Jani Nikula
v2:
 - go to PCI_D3cold
 - shuffle the call site a bit

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 83d853f..e33fa6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6126,6 +6126,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
 
lpt_disable_clkout_dp(dev);
hsw_pc8_disable_interrupts(dev);
+   intel_opregion_notify_adapter(dev, PCI_D3cold);
hsw_disable_lcpll(dev_priv, true, true);
 }
 
@@ -6163,6 +6164,7 @@ static void __hsw_disable_package_c8(struct 
drm_i915_private *dev_priv)
DRM_DEBUG_KMS(Disabling package C8+\n);
 
hsw_restore_lcpll(dev_priv);
+   intel_opregion_notify_adapter(dev, PCI_D0);
hsw_pc8_restore_interrupts(dev);
lpt_init_pch_refclk(dev);
 
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications

2013-08-30 Thread Jani Nikula
Hi all, a new version of [1] addressing Paulo's review comments. I hope
I didn't rush the changes too much; the new info about requested
vs. supported callbacks is confusing to say the least...

I'm sure Daniel will appreciate any Tested-by's!


BR,
Jani.


[1] http://mid.gmane.org/cover.1377246881.git.jani.nik...@intel.com

Jani Nikula (6):
  drm/i915: expose intel_ddi_get_encoder_port()
  drm/i915: add plumbing for SWSCI
  drm/i915: add opregion function to notify bios of encoder
enable/disable
  drm/i915: add opregion function to notify bios of adapter power state
  drm/i915: do display power state notification on crtc enable/disable
  DRAFT: drm/i915: do adapter power state notification on PC8+
enable/disable

 drivers/gpu/drm/i915/i915_drv.h   |   17 ++
 drivers/gpu/drm/i915/intel_ddi.c  |2 +-
 drivers/gpu/drm/i915/intel_display.c  |   10 +-
 drivers/gpu/drm/i915/intel_drv.h  |1 +
 drivers/gpu/drm/i915/intel_opregion.c |  276 -
 5 files changed, 300 insertions(+), 6 deletions(-)

-- 
1.7.10.4

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


[Intel-gfx] [PATCH 2/6] drm/i915: add plumbing for SWSCI

2013-08-30 Thread Jani Nikula
SWSCI is a driver to bios call interface.

This checks for SWSCI availability and bios requested callbacks, and
filters out any calls that shouldn't happen. This way the callers don't
need to do the checks all over the place.

v2: silence some checkpatch nagging

v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)

v4: remove an extra #define (Jesse)

v5: spec says s/w is responsible for clearing PCI_SWSCI bit 0 too

v6: per Paulo's review and more:
 - fix sub-function mask
 - add exit parameter
 - add define for set panel details call
 - return more errors from swsci
 - clean up the supported/requested callbacks bit masks mess
 - use DSLP for timeout
 - fix build for CONFIG_ACPI=n

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h   |2 +
 drivers/gpu/drm/i915/intel_opregion.c |  198 -
 2 files changed, 197 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0602c4b..67673e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -225,6 +225,8 @@ struct intel_opregion {
struct opregion_header __iomem *header;
struct opregion_acpi __iomem *acpi;
struct opregion_swsci __iomem *swsci;
+   u32 swsci_gbda_sub_functions;
+   u32 swsci_sbcb_sub_functions;
struct opregion_asle __iomem *asle;
void __iomem *vbt;
u32 __iomem *lid_state;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index cfb8fb6..c6a8d61 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -36,8 +36,11 @@
 #include i915_drv.h
 #include intel_drv.h
 
-#define PCI_ASLE 0xe4
-#define PCI_ASLS 0xfc
+#define PCI_ASLE   0xe4
+#define PCI_ASLS   0xfc
+#define PCI_SWSCI  0xe8
+#define PCI_SWSCI_SCISEL   (1  15)
+#define PCI_SWSCI_GSSCIE   (1  0)
 
 #define OPREGION_HEADER_OFFSET 0
 #define OPREGION_ACPI_OFFSET   0x100
@@ -151,6 +154,51 @@ struct opregion_asle {
 
 #define ASLE_CBLV_VALID (131)
 
+/* Software System Control Interrupt (SWSCI) */
+#define SWSCI_SCIC_INDICATOR   (1  0)
+#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1
+#define SWSCI_SCIC_MAIN_FUNCTION_MASK  (0xf  1)
+#define SWSCI_SCIC_SUB_FUNCTION_SHIFT  8
+#define SWSCI_SCIC_SUB_FUNCTION_MASK   (0xff  8)
+#define SWSCI_SCIC_EXIT_PARAMETER_SHIFT8
+#define SWSCI_SCIC_EXIT_PARAMETER_MASK (0xff  8)
+#define SWSCI_SCIC_EXIT_STATUS_SHIFT   5
+#define SWSCI_SCIC_EXIT_STATUS_MASK(7  5)
+#define SWSCI_SCIC_EXIT_STATUS_SUCCESS 1
+
+#define SWSCI_FUNCTION_CODE(main, sub) \
+   ((main)  SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
+(sub)  SWSCI_SCIC_SUB_FUNCTION_SHIFT)
+
+/* SWSCI: Get BIOS Data (GBDA) */
+#define SWSCI_GBDA 4
+#define SWSCI_GBDA_SUPPORTED_CALLS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
+#define SWSCI_GBDA_REQUESTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
+#define SWSCI_GBDA_BOOT_DISPLAY_PREF   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
+#define SWSCI_GBDA_PANEL_DETAILS   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
+#define SWSCI_GBDA_TV_STANDARD SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
+#define SWSCI_GBDA_INTERNAL_GRAPHICS   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
+#define SWSCI_GBDA_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
+
+/* SWSCI: System BIOS Callbacks (SBCB) */
+#define SWSCI_SBCB 6
+#define SWSCI_SBCB_SUPPORTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
+#define SWSCI_SBCB_INIT_COMPLETION SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
+#define SWSCI_SBCB_PRE_HIRES_SET_MODE  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
+#define SWSCI_SBCB_POST_HIRES_SET_MODE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
+#define SWSCI_SBCB_DISPLAY_SWITCH  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
+#define SWSCI_SBCB_SET_TV_FORMAT   SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
+#define SWSCI_SBCB_ADAPTER_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
+#define SWSCI_SBCB_DISPLAY_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
+#define SWSCI_SBCB_SET_BOOT_DISPLAYSWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
+#define SWSCI_SBCB_SET_PANEL_DETAILS   SWSCI_FUNCTION_CODE(SWSCI_SBCB, 10)
+#define SWSCI_SBCB_SET_INTERNAL_GFXSWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
+#define SWSCI_SBCB_POST_HIRES_TO_DOS_FSSWSCI_FUNCTION_CODE(SWSCI_SBCB, 
16)
+#define SWSCI_SBCB_SUSPEND_RESUME  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
+#define SWSCI_SBCB_SET_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
+#define SWSCI_SBCB_POST_VBE_PM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
+#define SWSCI_SBCB_ENABLE_DISABLE_AUDIOSWSCI_FUNCTION_CODE(SWSCI_SBCB, 
21)
+
 #define ACPI_OTHER_OUTPUT (08)
 #define ACPI_VGA_OUTPUT (18)
 #define ACPI_TV_OUTPUT (28)
@@ -158,6 +206,91 @@ struct opregion_asle {
 #define ACPI_LVDS_OUTPUT (48)
 
 #ifdef CONFIG_ACPI
+static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
+{
+   

[Intel-gfx] [PATCH 3/6] drm/i915: add opregion function to notify bios of encoder enable/disable

2013-08-30 Thread Jani Nikula
The bios interface seems messy, and it's hard to tell what the bios
really wants. At first, only add support for DDI based machines (hsw+),
and see how it turns out.

The spec says to notify prior to power down and after power up. It is
unclear whether it makes a difference.

v2:
 - squash notification function and callers patches together (Daniel)
 - move callers to haswell_crtc_{enable,disable} (Daniel)
 - rename notification function (Chris)

v3:
 - separate notification function and callers again, as it's not clear
   whether the display power state notification is the right thing to do
   after all

v4: per Paulo's review:
 - drop LVDS
 - WARN on unsupported encoder types

Signed-off-by: Jani Nikula jani.nik...@intel.com
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h   |8 ++
 drivers/gpu/drm/i915/intel_opregion.c |   51 +
 2 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 67673e2..5339297 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2182,15 +2182,23 @@ static inline bool intel_gmbus_is_forced_bit(struct 
i2c_adapter *adapter)
 extern void intel_i2c_reset(struct drm_device *dev);
 
 /* intel_opregion.c */
+struct intel_encoder;
 extern int intel_opregion_setup(struct drm_device *dev);
 #ifdef CONFIG_ACPI
 extern void intel_opregion_init(struct drm_device *dev);
 extern void intel_opregion_fini(struct drm_device *dev);
 extern void intel_opregion_asle_intr(struct drm_device *dev);
+extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
+bool enable);
 #else
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
 static inline void intel_opregion_fini(struct drm_device *dev) { return; }
 static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
+static inline int
+intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
+{
+   return 0;
+}
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index c6a8d61..791991b 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -291,6 +291,57 @@ static int swsci(struct drm_device *dev, u32 function, u32 
parm, u32 *parm_out)
 #undef C
 }
 
+#define DISPLAY_TYPE_CRT   0
+#define DISPLAY_TYPE_TV1
+#define DISPLAY_TYPE_EXTERNAL_FLAT_PANEL   2
+#define DISPLAY_TYPE_INTERNAL_FLAT_PANEL   3
+
+int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
+ bool enable)
+{
+   struct drm_device *dev = intel_encoder-base.dev;
+   u32 parm = 0;
+   u32 type = 0;
+   u32 port;
+
+   /* don't care about old stuff for now */
+   if (!HAS_DDI(dev))
+   return 0;
+
+   port = intel_ddi_get_encoder_port(intel_encoder);
+   if (port == PORT_E) {
+   port = 0;
+   } else {
+   parm |= 1  port;
+   port++;
+   }
+
+   if (!enable)
+   parm |= 4  8;
+
+   switch (intel_encoder-type) {
+   case INTEL_OUTPUT_ANALOG:
+   type = DISPLAY_TYPE_CRT;
+   break;
+   case INTEL_OUTPUT_UNKNOWN:
+   case INTEL_OUTPUT_DISPLAYPORT:
+   case INTEL_OUTPUT_HDMI:
+   type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
+   break;
+   case INTEL_OUTPUT_EDP:
+   type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
+   break;
+   default:
+   WARN_ONCE(1, unsupported intel_encoder type %d\n,
+ intel_encoder-type);
+   return -EINVAL;
+   }
+
+   parm |= type  (16 + port * 3);
+
+   return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port()

2013-08-30 Thread Jani Nikula
In preparation for followup work.

Signed-off-by: Jani Nikula jani.nik...@intel.com
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/intel_ddi.c |2 +-
 drivers/gpu/drm/i915/intel_drv.h |1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 63aca49..060ea50 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -58,7 +58,7 @@ static const u32 hsw_ddi_translations_fdi[] = {
0x00FF, 0x00040006  /* HDMI parameters */
 };
 
-static enum port intel_ddi_get_encoder_port(struct intel_encoder 
*intel_encoder)
+enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
 {
struct drm_encoder *encoder = intel_encoder-base;
int type = intel_encoder-type;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a38f5b2..5efb844 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -712,6 +712,7 @@ extern void intel_write_eld(struct drm_encoder *encoder,
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
+extern enum port intel_ddi_get_encoder_port(struct intel_encoder 
*intel_encoder);
 
 /* For use by IVB LP watermark workaround in intel_sprite.c */
 extern void intel_update_watermarks(struct drm_device *dev);
-- 
1.7.10.4

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


Re: [Intel-gfx] [PATCH] drm/i915: enable trickle feed on Haswell

2013-08-30 Thread Ville Syrjälä
On Fri, Aug 23, 2013 at 07:51:28PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 We shouldn't disable the trickle feed bits on Haswell. Our
 documentation explicitly says the trickle feed bits of PRI_CTL and
 CUR_CTL should not be programmed to 1, and the hardware engineer also
 asked us to not program the SPR_CTL field to 1. Leaving the bits as 1
 could cause underflows.
 
 Reported-by: Arthur Runyan arthur.j.run...@intel.com
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 ---
  drivers/gpu/drm/i915/i915_reg.h  |  1 +
  drivers/gpu/drm/i915/intel_display.c | 10 +++---
  drivers/gpu/drm/i915/intel_pm.c  |  2 --
  drivers/gpu/drm/i915/intel_sprite.c  |  7 +--
  4 files changed, 13 insertions(+), 7 deletions(-)
 
 Some side discussions:
 
  1 - It seems we always do read-modify-write with the PRI/SPR/CUR registers. I
  think this is dangerous since we may forget to disable something and keep our
  code in a bugged state forever. Shouldn't we try to completely set the full
  state of the bits from scratch every time we enable PRI/SPR/CUR?

Yeah I dislike the RMW. In my atomic branch I think I got rid of it, if
for no other reason than less overhead. I think the reason for the RMW
was that the whole plane handling has been spread around all over the
place. We should just centralize it nicely as part of the all planes
are drm_planes change.

The base address RMW is an open question though since it was explicitly
made that way. But on HSW that's supposedly bugged and can revert the
based address update from a command streamer flip. I haven't actually
tried which way it's bugged: does it immediately revert and cause the
display to scan out garbage until the new value is latches on the next
vblank, or does the revert also need a vblank to latch. The latter
option is the less dangerous since we'd need to get a vblank between
the read and write to see garbage. I should write a test now that
there's a hsw machine on my desk...

  2 - We also have code to enable the trickle feed bits at init_clock_gating.
  Isn't this dangerous? Trickle fee changes the memory is read, my first guess
  would be that it's probably not safe to change this bit when things are
  enabled. Also, we lose the state of these bits when the power well is 
 disabled,
  so the code in init_clock_gating really makes no sense on Haswell.

I don't recall seeing any garbage when toggling trickle feed on the fly.
I assume that if the watermarks are sufficiently high toggling trickle
feed should be safe, well apparently before HSW that is :)

 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index bbbc177..e92bb47 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -3312,6 +3312,7 @@
  #define   MCURSOR_PIPE_A 0x00
  #define   MCURSOR_PIPE_B (1  28)
  #define   MCURSOR_GAMMA_ENABLE  (1  26)
 +#define   CURSOR_TRICKLE_FEED_DISABLE(1  14)
  #define _CURABASE(dev_priv-info-display_mmio_offset + 0x70084)
  #define _CURAPOS (dev_priv-info-display_mmio_offset + 0x70088)
  #define   CURSOR_POS_MASK   0x007FF
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index c64631d..d5f038e 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2077,8 +2077,10 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
   else
   dspcntr = ~DISPPLANE_TILED;
  
 - /* must disable */
 - dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 + if (IS_HASWELL(dev))
 + dspcntr = ~DISPPLANE_TRICKLE_FEED_DISABLE;
 + else
 + dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
  
   I915_WRITE(reg, dspcntr);
  
 @@ -6768,8 +6770,10 @@ static void ivb_update_cursor(struct drm_crtc *crtc, 
 u32 base)
   cntl = ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
   cntl |= CURSOR_MODE_DISABLE;
   }
 - if (IS_HASWELL(dev))
 + if (IS_HASWELL(dev)) {
   cntl |= CURSOR_PIPE_CSC_ENABLE;
 + cntl = ~CURSOR_TRICKLE_FEED_DISABLE;
 + }
   I915_WRITE(CURCNTR_IVB(pipe), cntl);
  
   intel_crtc-cursor_visible = visible;
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 4605682..3a5c0bb 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -4950,8 +4950,6 @@ static void haswell_init_clock_gating(struct drm_device 
 *dev)
   I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) |
   GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
  
 - g4x_disable_trickle_feed(dev);
 -
   /* WaVSRefCountFullforceMissDisable:hsw */
   gen7_setup_fixed_func_scheduler(dev_priv);
  
 diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
 

Re: [Intel-gfx] [PATCH 2/6] drm/i915: add plumbing for SWSCI

2013-08-30 Thread Paulo Zanoni
2013/8/30 Jani Nikula jani.nik...@intel.com:
 SWSCI is a driver to bios call interface.

 This checks for SWSCI availability and bios requested callbacks, and
 filters out any calls that shouldn't happen. This way the callers don't
 need to do the checks all over the place.

 v2: silence some checkpatch nagging

 v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)

 v4: remove an extra #define (Jesse)

 v5: spec says s/w is responsible for clearing PCI_SWSCI bit 0 too

 v6: per Paulo's review and more:
  - fix sub-function mask
  - add exit parameter
  - add define for set panel details call
  - return more errors from swsci
  - clean up the supported/requested callbacks bit masks mess
  - use DSLP for timeout
  - fix build for CONFIG_ACPI=n

 Signed-off-by: Jani Nikula jani.nik...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h   |2 +
  drivers/gpu/drm/i915/intel_opregion.c |  198 
 -
  2 files changed, 197 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 0602c4b..67673e2 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -225,6 +225,8 @@ struct intel_opregion {
 struct opregion_header __iomem *header;
 struct opregion_acpi __iomem *acpi;
 struct opregion_swsci __iomem *swsci;
 +   u32 swsci_gbda_sub_functions;
 +   u32 swsci_sbcb_sub_functions;
 struct opregion_asle __iomem *asle;
 void __iomem *vbt;
 u32 __iomem *lid_state;
 diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
 b/drivers/gpu/drm/i915/intel_opregion.c
 index cfb8fb6..c6a8d61 100644
 --- a/drivers/gpu/drm/i915/intel_opregion.c
 +++ b/drivers/gpu/drm/i915/intel_opregion.c
 @@ -36,8 +36,11 @@
  #include i915_drv.h
  #include intel_drv.h

 -#define PCI_ASLE 0xe4
 -#define PCI_ASLS 0xfc
 +#define PCI_ASLE   0xe4
 +#define PCI_ASLS   0xfc
 +#define PCI_SWSCI  0xe8
 +#define PCI_SWSCI_SCISEL   (1  15)
 +#define PCI_SWSCI_GSSCIE   (1  0)

  #define OPREGION_HEADER_OFFSET 0
  #define OPREGION_ACPI_OFFSET   0x100
 @@ -151,6 +154,51 @@ struct opregion_asle {

  #define ASLE_CBLV_VALID (131)

 +/* Software System Control Interrupt (SWSCI) */
 +#define SWSCI_SCIC_INDICATOR   (1  0)
 +#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1
 +#define SWSCI_SCIC_MAIN_FUNCTION_MASK  (0xf  1)
 +#define SWSCI_SCIC_SUB_FUNCTION_SHIFT  8
 +#define SWSCI_SCIC_SUB_FUNCTION_MASK   (0xff  8)
 +#define SWSCI_SCIC_EXIT_PARAMETER_SHIFT8
 +#define SWSCI_SCIC_EXIT_PARAMETER_MASK (0xff  8)
 +#define SWSCI_SCIC_EXIT_STATUS_SHIFT   5
 +#define SWSCI_SCIC_EXIT_STATUS_MASK(7  5)
 +#define SWSCI_SCIC_EXIT_STATUS_SUCCESS 1
 +
 +#define SWSCI_FUNCTION_CODE(main, sub) \
 +   ((main)  SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
 +(sub)  SWSCI_SCIC_SUB_FUNCTION_SHIFT)
 +
 +/* SWSCI: Get BIOS Data (GBDA) */
 +#define SWSCI_GBDA 4
 +#define SWSCI_GBDA_SUPPORTED_CALLS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
 +#define SWSCI_GBDA_REQUESTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
 +#define SWSCI_GBDA_BOOT_DISPLAY_PREF   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
 +#define SWSCI_GBDA_PANEL_DETAILS   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
 +#define SWSCI_GBDA_TV_STANDARD SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
 +#define SWSCI_GBDA_INTERNAL_GRAPHICS   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
 +#define SWSCI_GBDA_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
 +
 +/* SWSCI: System BIOS Callbacks (SBCB) */
 +#define SWSCI_SBCB 6
 +#define SWSCI_SBCB_SUPPORTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
 +#define SWSCI_SBCB_INIT_COMPLETION SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
 +#define SWSCI_SBCB_PRE_HIRES_SET_MODE  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
 +#define SWSCI_SBCB_POST_HIRES_SET_MODE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
 +#define SWSCI_SBCB_DISPLAY_SWITCH  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
 +#define SWSCI_SBCB_SET_TV_FORMAT   SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
 +#define SWSCI_SBCB_ADAPTER_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
 +#define SWSCI_SBCB_DISPLAY_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
 +#define SWSCI_SBCB_SET_BOOT_DISPLAYSWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
 +#define SWSCI_SBCB_SET_PANEL_DETAILS   SWSCI_FUNCTION_CODE(SWSCI_SBCB, 10)
 +#define SWSCI_SBCB_SET_INTERNAL_GFXSWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
 +#define SWSCI_SBCB_POST_HIRES_TO_DOS_FS
 SWSCI_FUNCTION_CODE(SWSCI_SBCB, 16)
 +#define SWSCI_SBCB_SUSPEND_RESUME  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
 +#define SWSCI_SBCB_SET_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
 +#define SWSCI_SBCB_POST_VBE_PM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
 +#define SWSCI_SBCB_ENABLE_DISABLE_AUDIO
 SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
 +
  #define ACPI_OTHER_OUTPUT (08)
  #define ACPI_VGA_OUTPUT (18)
  #define ACPI_TV_OUTPUT (28)
 @@ -158,6 +206,91 @@ struct 

Re: [Intel-gfx] [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable

2013-08-30 Thread Paulo Zanoni
2013/8/30 Jani Nikula jani.nik...@intel.com:
 v2:
  - go to PCI_D3cold
  - shuffle the call site a bit

Ok, so I know I'm the one who requested to shuffle the call site, but
it's because I thought that when we disable LCPLL we actually put the
device in D3. After some experimentation last week, we discovered we
need to write a PCI config register to actually enable D3, so your
call to intel_opregion_notify_adapter should probably be glued to that
write (which we don't have yet). The problem is that if we really put
the device in D3, all the registers go away, so we need to properly
recover from that. I think it makes sense to postpone this patch until
we have the actual code to put our device in D3 and properly restore
from it. So my suggestion is to merge patches 1-5 so we can work on
the D3 feature on top of the work you already completed.

Thanks,
Paulo


 Signed-off-by: Jani Nikula jani.nik...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c |2 ++
  1 file changed, 2 insertions(+)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 83d853f..e33fa6d 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -6126,6 +6126,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)

 lpt_disable_clkout_dp(dev);
 hsw_pc8_disable_interrupts(dev);
 +   intel_opregion_notify_adapter(dev, PCI_D3cold);
 hsw_disable_lcpll(dev_priv, true, true);
  }

 @@ -6163,6 +6164,7 @@ static void __hsw_disable_package_c8(struct 
 drm_i915_private *dev_priv)
 DRM_DEBUG_KMS(Disabling package C8+\n);

 hsw_restore_lcpll(dev_priv);
 +   intel_opregion_notify_adapter(dev, PCI_D0);
 hsw_pc8_restore_interrupts(dev);
 lpt_init_pch_refclk(dev);

 --
 1.7.10.4




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


Re: [Intel-gfx] [PATCH 01/19] drm/i915: Pass crtc to intel_update_watermarks()

2013-08-30 Thread Paulo Zanoni
2013/8/30  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Passing the appropriate crtc to intel_update_watermarks() should help
 in avoiding needless work in the future.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

I like the fact that now we're passing the CRTC, but bikeshed my
only worry is that now some functions overwrite the crtc we pass as
argument, so this might be confusing and maybe lead to bugs in the
future: perhaps the argument could be called unused_crtc or
ignored_crtc, then we'd keep the crtc variables they already have
/bikeshed. But it's just a bikeshed, so with or without changes:
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com.


 ---
  drivers/gpu/drm/i915/i915_drv.h  |  2 +-
  drivers/gpu/drm/i915/intel_display.c | 20 -
  drivers/gpu/drm/i915/intel_drv.h |  2 +-
  drivers/gpu/drm/i915/intel_pm.c  | 43 
 +---
  drivers/gpu/drm/i915/intel_sprite.c  |  6 ++---
  5 files changed, 40 insertions(+), 33 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 7594356..ab46eb3 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -357,7 +357,7 @@ struct drm_i915_display_funcs {
   int target, int refclk,
   struct dpll *match_clock,
   struct dpll *best_clock);
 -   void (*update_wm)(struct drm_device *dev);
 +   void (*update_wm)(struct drm_crtc *crtc);
 void (*update_sprite_wm)(struct drm_plane *plane,
  struct drm_crtc *crtc,
  uint32_t sprite_width, int pixel_size,
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f526ea9..13856ec 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3278,7 +3278,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
 intel_set_pch_fifo_underrun_reporting(dev, pipe, true);

 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 for_each_encoder_on_crtc(dev, crtc, encoder)
 if (encoder-pre_enable)
 @@ -3388,7 +3388,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 if (intel_crtc-config.has_pch_encoder)
 intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, 
 true);

 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 if (intel_crtc-config.has_pch_encoder)
 dev_priv-display.fdi_link_train(crtc);
 @@ -3520,7 +3520,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 }

 intel_crtc-active = false;
 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 mutex_lock(dev-struct_mutex);
 intel_update_fbc(dev);
 @@ -3577,7 +3577,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 }

 intel_crtc-active = false;
 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 mutex_lock(dev-struct_mutex);
 intel_update_fbc(dev);
 @@ -3677,7 +3677,7 @@ static void valleyview_crtc_enable(struct drm_crtc 
 *crtc)
 return;

 intel_crtc-active = true;
 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 for_each_encoder_on_crtc(dev, crtc, encoder)
 if (encoder-pre_pll_enable)
 @@ -3722,7 +3722,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 return;

 intel_crtc-active = true;
 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 for_each_encoder_on_crtc(dev, crtc, encoder)
 if (encoder-pre_enable)
 @@ -3806,7 +3806,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)

 intel_crtc-active = false;
 intel_update_fbc(dev);
 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);
  }

  static void i9xx_crtc_off(struct drm_crtc *crtc)
 @@ -4972,7 +4972,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,

 ret = intel_pipe_set_base(crtc, x, y, fb);

 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 return ret;
  }
 @@ -5860,7 +5860,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,

 ret = intel_pipe_set_base(crtc, x, y, fb);

 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 return ret;
  }
 @@ -6316,7 +6316,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,

 ret = intel_pipe_set_base(crtc, x, y, fb);

 -   intel_update_watermarks(dev);
 +   intel_update_watermarks(crtc);

 return ret;
  }
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 

Re: [Intel-gfx] [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state

2013-08-30 Thread Paulo Zanoni
2013/8/30 Jani Nikula jani.nik...@intel.com:
 Notifying the bios lets it enter power saving states.

 Signed-off-by: Jani Nikula jani.nik...@intel.com

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/i915_drv.h   |7 +++
  drivers/gpu/drm/i915/intel_opregion.c |   27 +++
  2 files changed, 34 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 5339297..7daae2a 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2190,6 +2190,8 @@ extern void intel_opregion_fini(struct drm_device *dev);
  extern void intel_opregion_asle_intr(struct drm_device *dev);
  extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
  bool enable);
 +extern int intel_opregion_notify_adapter(struct drm_device *dev,
 +pci_power_t state);
  #else
  static inline void intel_opregion_init(struct drm_device *dev) { return; }
  static inline void intel_opregion_fini(struct drm_device *dev) { return; }
 @@ -2199,6 +2201,11 @@ intel_opregion_notify_encoder(struct intel_encoder 
 *intel_encoder, bool enable)
  {
 return 0;
  }
 +static inline int
 +intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 +{
 +   return 0;
 +}
  #endif

  /* intel_acpi.c */
 diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
 b/drivers/gpu/drm/i915/intel_opregion.c
 index 791991b..05243df 100644
 --- a/drivers/gpu/drm/i915/intel_opregion.c
 +++ b/drivers/gpu/drm/i915/intel_opregion.c
 @@ -342,6 +342,33 @@ int intel_opregion_notify_encoder(struct intel_encoder 
 *intel_encoder,
 return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
  }

 +static const struct {
 +   pci_power_t pci_power_state;
 +   u32 parm;
 +} power_state_map[] = {
 +   { PCI_D0,   0x00 },
 +   { PCI_D1,   0x01 },
 +   { PCI_D2,   0x02 },
 +   { PCI_D3hot,0x04 },
 +   { PCI_D3cold,   0x04 },
 +};
 +
 +int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 +{
 +   int i;
 +
 +   if (!HAS_DDI(dev))
 +   return 0;
 +
 +   for (i = 0; i  ARRAY_SIZE(power_state_map); i++) {
 +   if (state == power_state_map[i].pci_power_state)
 +   return swsci(dev, SWSCI_SBCB_ADAPTER_POWER_STATE,
 +power_state_map[i].parm, NULL);
 +   }
 +
 +   return -EINVAL;
 +}
 +
  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
  {
 struct drm_i915_private *dev_priv = dev-dev_private;
 --
 1.7.10.4




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


Re: [Intel-gfx] [PATCH 02/19] drm/i915: Call intel_update_watermarks() in specific place during modeset

2013-08-30 Thread Paulo Zanoni
2013/8/30  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Make the call to intel_update_watermarks() just once or twice during
 modeset. Ideally it should happen independently when each plane gets
 enabled/disabled, but for now it seems better to keep it in central
 place. We can improve things when we get all the planes sorted out
 in a better way.

 When enabling set up the watermarks just before the pipe is enabled.
 And when disabling we need to wait until we've marked the crtc as
 inactive.

Why do we need to wait until we've marked the CRTC as inactive?
(Daniel/Ville should put the answer in the commit message)

The patch looks correct, but bikeshed I'd separate the don't update
watermarks at crtc-mode_set part from the current patch, since if we
start getting regressions we won't know if they're caused by the fact
that now we don't adjust the watermarks before enabling the PCH and
doing link training, or if they're caused by the fact that we don't
update them anymore at mode_set /bikeshed. But since it looks
correct, we shouldn't really be getting regressions...

With or without changes: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com




 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 19 ++-
  1 file changed, 6 insertions(+), 13 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 13856ec..a5181fe 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3278,8 +3278,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
 intel_set_pch_fifo_underrun_reporting(dev, pipe, true);

 -   intel_update_watermarks(crtc);
 -
 for_each_encoder_on_crtc(dev, crtc, encoder)
 if (encoder-pre_enable)
 encoder-pre_enable(encoder);
 @@ -3302,6 +3300,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
  */
 intel_crtc_load_lut(crtc);

 +   intel_update_watermarks(crtc);
 intel_enable_pipe(dev_priv, pipe,
   intel_crtc-config.has_pch_encoder, false);
 intel_enable_plane(dev_priv, plane, pipe);
 @@ -3388,8 +3387,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 if (intel_crtc-config.has_pch_encoder)
 intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, 
 true);

 -   intel_update_watermarks(crtc);
 -
 if (intel_crtc-config.has_pch_encoder)
 dev_priv-display.fdi_link_train(crtc);

 @@ -3410,6 +3407,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 intel_ddi_set_pipe_settings(crtc);
 intel_ddi_enable_transcoder_func(crtc);

 +   intel_update_watermarks(crtc);
 intel_enable_pipe(dev_priv, pipe,
   intel_crtc-config.has_pch_encoder, false);
 intel_enable_plane(dev_priv, plane, pipe);
 @@ -3677,7 +3675,6 @@ static void valleyview_crtc_enable(struct drm_crtc 
 *crtc)
 return;

 intel_crtc-active = true;
 -   intel_update_watermarks(crtc);

 for_each_encoder_on_crtc(dev, crtc, encoder)
 if (encoder-pre_pll_enable)
 @@ -3696,6 +3693,7 @@ static void valleyview_crtc_enable(struct drm_crtc 
 *crtc)

 intel_crtc_load_lut(crtc);

 +   intel_update_watermarks(crtc);
 intel_enable_pipe(dev_priv, pipe, false, is_dsi);
 intel_enable_plane(dev_priv, plane, pipe);
 intel_enable_planes(crtc);
 @@ -3722,7 +3720,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 return;

 intel_crtc-active = true;
 -   intel_update_watermarks(crtc);

 for_each_encoder_on_crtc(dev, crtc, encoder)
 if (encoder-pre_enable)
 @@ -3734,6 +3731,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)

 intel_crtc_load_lut(crtc);

 +   intel_update_watermarks(crtc);
 intel_enable_pipe(dev_priv, pipe, false, false);
 intel_enable_plane(dev_priv, plane, pipe);
 intel_enable_planes(crtc);
 @@ -3805,8 +3803,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 i9xx_disable_pll(dev_priv, pipe);

 intel_crtc-active = false;
 -   intel_update_fbc(dev);
 intel_update_watermarks(crtc);
 +
 +   intel_update_fbc(dev);
  }

  static void i9xx_crtc_off(struct drm_crtc *crtc)
 @@ -4972,8 +4971,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,

 ret = intel_pipe_set_base(crtc, x, y, fb);

 -   intel_update_watermarks(crtc);
 -
 return ret;
  }

 @@ -5860,8 +5857,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,

 ret = intel_pipe_set_base(crtc, x, y, fb);

 -   intel_update_watermarks(crtc);
 -
 return ret;
  }

 @@ -6316,8 +6311,6 @@ static int 

Re: [Intel-gfx] [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable

2013-08-30 Thread Paulo Zanoni
2013/8/30 Jani Nikula jani.nik...@intel.com:
 The spec says to notify prior to power down and after power up. It is
 unclear whether it makes a difference.

 Signed-off-by: Jani Nikula jani.nik...@intel.com
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---

 Paulo, still okay with the r-b?

Yes :)

 ---
  drivers/gpu/drm/i915/intel_display.c |8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 9222e570..83d853f 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3425,8 +3425,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 intel_update_fbc(dev);
 mutex_unlock(dev-struct_mutex);

 -   for_each_encoder_on_crtc(dev, crtc, encoder)
 +   for_each_encoder_on_crtc(dev, crtc, encoder) {
 encoder-enable(encoder);
 +   intel_opregion_notify_encoder(encoder, true);
 +   }

 /*
  * There seems to be a race in PCH platform hw (at least on some
 @@ -3540,8 +3542,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 if (!intel_crtc-active)
 return;

 -   for_each_encoder_on_crtc(dev, crtc, encoder)
 +   for_each_encoder_on_crtc(dev, crtc, encoder) {
 +   intel_opregion_notify_encoder(encoder, false);
 encoder-disable(encoder);
 +   }

 intel_crtc_wait_for_pending_flips(crtc);
 drm_vblank_off(dev, pipe);
 --
 1.7.10.4




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


Re: [Intel-gfx] [PATCH 01/19] drm/i915: Pass crtc to intel_update_watermarks()

2013-08-30 Thread Ville Syrjälä
On Fri, Aug 30, 2013 at 05:09:49PM -0300, Paulo Zanoni wrote:
 2013/8/30  ville.syrj...@linux.intel.com:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  Passing the appropriate crtc to intel_update_watermarks() should help
  in avoiding needless work in the future.
 
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 
 I like the fact that now we're passing the CRTC, but bikeshed my
 only worry is that now some functions overwrite the crtc we pass as
 argument, so this might be confusing and maybe lead to bugs in the
 future: perhaps the argument could be called unused_crtc or
 ignored_crtc, then we'd keep the crtc variables they already have
 /bikeshed. But it's just a bikeshed, so with or without changes:
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com.

Right. It could be a bit confusing. I'll change it.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/19] drm/i915: Constify some watermark data

2013-08-30 Thread Paulo Zanoni
2013/8/30  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 hsw_pipe_wm_parameters and hsw_wm_maximums typically are read only. Make
 them const.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Reviewed-by: GCC 4.7.3
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 774c57f..96493dc 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2202,7 +2202,7 @@ struct intel_wm_config {
   * For both WM_PIPE and WM_LP.
   * mem_value must be in 0.1us units.
   */
 -static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_pri_wm(const struct hsw_pipe_wm_parameters 
 *params,
uint32_t mem_value,
bool is_lp)
  {
 @@ -2231,7 +2231,7 @@ static uint32_t ilk_compute_pri_wm(struct 
 hsw_pipe_wm_parameters *params,
   * For both WM_PIPE and WM_LP.
   * mem_value must be in 0.1us units.
   */
 -static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_spr_wm(const struct hsw_pipe_wm_parameters 
 *params,
uint32_t mem_value)
  {
 uint32_t method1, method2;
 @@ -2254,7 +2254,7 @@ static uint32_t ilk_compute_spr_wm(struct 
 hsw_pipe_wm_parameters *params,
   * For both WM_PIPE and WM_LP.
   * mem_value must be in 0.1us units.
   */
 -static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_cur_wm(const struct hsw_pipe_wm_parameters 
 *params,
uint32_t mem_value)
  {
 if (!params-active || !params-cur.enabled)
 @@ -2268,7 +2268,7 @@ static uint32_t ilk_compute_cur_wm(struct 
 hsw_pipe_wm_parameters *params,
  }

  /* Only for WM_LP. */
 -static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_fbc_wm(const struct hsw_pipe_wm_parameters 
 *params,
uint32_t pri_val)
  {
 if (!params-active || !params-pri.enabled)
 @@ -2419,7 +2419,7 @@ static bool ilk_check_wm(int level,

  static void ilk_compute_wm_level(struct drm_i915_private *dev_priv,
  int level,
 -struct hsw_pipe_wm_parameters *p,
 +const struct hsw_pipe_wm_parameters *p,
  struct intel_wm_level *result)
  {
 uint16_t pri_latency = dev_priv-wm.pri_latency[level];
 @@ -2441,8 +2441,8 @@ static void ilk_compute_wm_level(struct 
 drm_i915_private *dev_priv,
  }

  static bool hsw_compute_lp_wm(struct drm_i915_private *dev_priv,
 - int level, struct hsw_wm_maximums *max,
 - struct hsw_pipe_wm_parameters *params,
 + int level, const struct hsw_wm_maximums *max,
 + const struct hsw_pipe_wm_parameters *params,
   struct intel_wm_level *result)
  {
 enum pipe pipe;
 @@ -2462,7 +2462,7 @@ static bool hsw_compute_lp_wm(struct drm_i915_private 
 *dev_priv,

  static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
 enum pipe pipe,
 -   struct hsw_pipe_wm_parameters *params)
 +   const struct hsw_pipe_wm_parameters 
 *params)
  {
 uint32_t pri_val, cur_val, spr_val;
 /* WM0 latency values stored in 0.1us units */
 @@ -2670,8 +2670,8 @@ static void hsw_compute_wm_parameters(struct drm_device 
 *dev,
  }

  static void hsw_compute_wm_results(struct drm_device *dev,
 -  struct hsw_pipe_wm_parameters *params,
 -  struct hsw_wm_maximums *lp_maximums,
 +  const struct hsw_pipe_wm_parameters 
 *params,
 +  const struct hsw_wm_maximums *lp_maximums,
struct hsw_wm_values *results)
  {
 struct drm_i915_private *dev_priv = dev-dev_private;
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 02/19] drm/i915: Call intel_update_watermarks() in specific place during modeset

2013-08-30 Thread Ville Syrjälä
On Fri, Aug 30, 2013 at 05:26:29PM -0300, Paulo Zanoni wrote:
 2013/8/30  ville.syrj...@linux.intel.com:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  Make the call to intel_update_watermarks() just once or twice during
  modeset. Ideally it should happen independently when each plane gets
  enabled/disabled, but for now it seems better to keep it in central
  place. We can improve things when we get all the planes sorted out
  in a better way.
 
  When enabling set up the watermarks just before the pipe is enabled.
  And when disabling we need to wait until we've marked the crtc as
  inactive.
 
 Why do we need to wait until we've marked the CRTC as inactive?
 (Daniel/Ville should put the answer in the commit message)

Because the watermark compute code looks at intel_crtc-active. If we
compute the watermarks before, the code thinks the pipe is active.

Hmm. BTW now that I look at intel_crtc_active() I start to wonder why it
looks at the clock in the user specified mode. In fact most (maybe all?)
of the pre-hsw watermark code is fscked up and it looks at the wrong
mode. Sigh. Suppose I need to make a quick for all that as well...

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 04/19] drm/i915: Use ilk_compute_wm_level to compute WM_PIPE values

2013-08-30 Thread Paulo Zanoni
2013/8/30  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Unify the code a bit to use ilk_compute_wm_level for all watermark
 levels.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Looks correct and doesn't change the WM values set on my specific
eDP+HDMI config here...

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 44 
 -
  1 file changed, 21 insertions(+), 23 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 96493dc..d118ee1 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2460,33 +2460,31 @@ static bool hsw_compute_lp_wm(struct drm_i915_private 
 *dev_priv,
 return ilk_check_wm(level, max, result);
  }

 -static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
 -   enum pipe pipe,
 +
 +static uint32_t hsw_compute_wm_pipe(struct drm_device *dev,
 const struct hsw_pipe_wm_parameters 
 *params)
  {
 -   uint32_t pri_val, cur_val, spr_val;
 -   /* WM0 latency values stored in 0.1us units */
 -   uint16_t pri_latency = dev_priv-wm.pri_latency[0];
 -   uint16_t spr_latency = dev_priv-wm.spr_latency[0];
 -   uint16_t cur_latency = dev_priv-wm.cur_latency[0];
 +   struct drm_i915_private *dev_priv = dev-dev_private;
 +   struct intel_wm_config config = {
 +   .num_pipes_active = 1,
 +   .sprites_enabled = params-spr.enabled,
 +   .sprites_scaled = params-spr.scaled,
 +   };
 +   struct hsw_wm_maximums max;
 +   struct intel_wm_level res;
 +
 +   if (!params-active)
 +   return 0;
 +
 +   ilk_wm_max(dev, 0, config, INTEL_DDB_PART_1_2, max);

 -   pri_val = ilk_compute_pri_wm(params, pri_latency, false);
 -   spr_val = ilk_compute_spr_wm(params, spr_latency);
 -   cur_val = ilk_compute_cur_wm(params, cur_latency);
 +   ilk_compute_wm_level(dev_priv, 0, params, res);

 -   WARN(pri_val  127,
 -Primary WM error, mode not supported for pipe %c\n,
 -pipe_name(pipe));
 -   WARN(spr_val  127,
 -Sprite WM error, mode not supported for pipe %c\n,
 -pipe_name(pipe));
 -   WARN(cur_val  63,
 -Cursor WM error, mode not supported for pipe %c\n,
 -pipe_name(pipe));
 +   ilk_check_wm(0, max, res);

 -   return (pri_val  WM0_PIPE_PLANE_SHIFT) |
 -  (spr_val  WM0_PIPE_SPRITE_SHIFT) |
 -  cur_val;
 +   return (res.pri_val  WM0_PIPE_PLANE_SHIFT) |
 +  (res.spr_val  WM0_PIPE_SPRITE_SHIFT) |
 +  res.cur_val;
  }

  static uint32_t
 @@ -2715,7 +2713,7 @@ static void hsw_compute_wm_results(struct drm_device 
 *dev,
 }

 for_each_pipe(pipe)
 -   results-wm_pipe[pipe] = hsw_compute_wm_pipe(dev_priv, pipe,
 +   results-wm_pipe[pipe] = hsw_compute_wm_pipe(dev,
  params[pipe]);

 for_each_pipe(pipe) {
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 05/19] drm/i915: Refactor max WM level

2013-08-30 Thread Paulo Zanoni
2013/8/30  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Pull the expected max WM level determinations out to a separate
 function. Will have another user soon.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index d118ee1..163ba74 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2558,19 +2558,22 @@ static void intel_fixup_cur_wm_latency(struct 
 drm_device *dev, uint16_t wm[5])
 wm[3] *= 2;
  }

 -static void intel_print_wm_latency(struct drm_device *dev,
 -  const char *name,
 -  const uint16_t wm[5])
 +static int ilk_wm_max_level(const struct drm_device *dev)
  {
 -   int level, max_level;
 -
 /* how many WM levels are we expecting */
 if (IS_HASWELL(dev))
 -   max_level = 4;
 +   return 4;
 else if (INTEL_INFO(dev)-gen = 6)
 -   max_level = 3;
 +   return 3;
 else
 -   max_level = 2;
 +   return 2;
 +}
 +
 +static void intel_print_wm_latency(struct drm_device *dev,
 +  const char *name,
 +  const uint16_t wm[5])
 +{
 +   int level, max_level = ilk_wm_max_level(dev);

 for (level = 0; level = max_level; level++) {
 unsigned int latency = wm[level];
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH] drm/i915: It's its!

2013-08-30 Thread Ben Widawsky
On Fri, Aug 30, 2013 at 02:40:26PM +0100, Damien Lespiau wrote:
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com
 ---
  drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
 b/drivers/gpu/drm/i915/i915_gem_context.c
 index 403309c..e038513 100644
 --- a/drivers/gpu/drm/i915/i915_gem_context.c
 +++ b/drivers/gpu/drm/i915/i915_gem_context.c
 @@ -73,7 +73,7 @@
   *
   * There are two confusing terms used above:
   *  The current context means the context which is currently running on the
 - *  GPU. The GPU has loaded it's state already and has stored away the gtt
 + *  GPU. The GPU has loaded its state already and has stored away the gtt
   *  offset of the BO. The GPU is not actively referencing the data at this
   *  offset, but it will on the next context switch. The only way to avoid 
 this
   *  is to do a GPU reset.

Acked-by: Ben Widawsky b...@bwidawsk.net

By the way, I plan to rewrite a lot of this once we land full PPGTT.

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


[Intel-gfx] [PATCH 0/8] [RESEND] VMA patches

2013-08-30 Thread Ben Widawsky
Now that [hopefully all] of the fallout from the last round of VMA merging has
been taken care of - Resending the remaining VMA patches which are required for
the full PPGTT support.

Some of these patches are new as a result of the last round of review. For the
ones which are just a RESEND, I've only rebased and un on an IVB.

Ben Widawsky (8):
  drm/i915: Synchronize pread/pwrite with wait_rendering
  drm/i915: Extract vm specific part of eviction
  drm/i915: evict VM instead of everything
  drm/1915: trace vm eviction instead of everything
  drm/i915: Convert active API to VMA
  drm/i915: Add bind/unbind object functions to VM
  drm/i915: Use the new vm [un]bind functions
  drm/i915: eliminate vm-insert_entries()

 drivers/gpu/drm/i915/i915_drv.h|  84 ++---
 drivers/gpu/drm/i915/i915_gem.c|  61 +++
 drivers/gpu/drm/i915/i915_gem_context.c|  16 ++--
 drivers/gpu/drm/i915/i915_gem_evict.c  |  47 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  40 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c| 116 -
 drivers/gpu/drm/i915/i915_trace.h  |  12 +--
 7 files changed, 228 insertions(+), 148 deletions(-)

-- 
1.8.4

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


[Intel-gfx] [PATCH 2/8] drm/i915: Extract vm specific part of eviction

2013-08-30 Thread Ben Widawsky
As we'll see in the next patch, being able to evict for just 1 VM is
handy.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_gem_evict.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
b/drivers/gpu/drm/i915/i915_gem_evict.c
index cc8974f..e9033f0 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -155,12 +155,31 @@ found:
return ret;
 }
 
+static int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle)
+{
+   struct i915_vma *vma, *next;
+   int ret;
+
+   if (do_idle) {
+   ret = i915_gpu_idle(vm-dev);
+   if (ret)
+   return ret;
+
+   i915_gem_retire_requests(vm-dev);
+   }
+
+   list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
+   if (vma-obj-pin_count == 0)
+   WARN_ON(i915_vma_unbind(vma));
+
+   return 0;
+}
+
 int
 i915_gem_evict_everything(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = dev-dev_private;
struct i915_address_space *vm;
-   struct i915_vma *vma, *next;
bool lists_empty = true;
int ret;
 
@@ -187,11 +206,8 @@ i915_gem_evict_everything(struct drm_device *dev)
i915_gem_retire_requests(dev);
 
/* Having flushed everything, unbind() should never raise an error */
-   list_for_each_entry(vm, dev_priv-vm_list, global_link) {
-   list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
-   if (vma-obj-pin_count == 0)
-   WARN_ON(i915_vma_unbind(vma));
-   }
+   list_for_each_entry(vm, dev_priv-vm_list, global_link)
+   WARN_ON(i915_gem_evict_vm(vm, false));
 
return 0;
 }
-- 
1.8.4

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


[Intel-gfx] [PATCH 1/8] drm/i915: Synchronize pread/pwrite with wait_rendering

2013-08-30 Thread Ben Widawsky
lifted from Daniel:
pread/pwrite isn't about the object's domain at all, but purely about
synchronizing for outstanding rendering. Replacing the call to
set_to_gtt_domain with a wait_rendering would imo improve code
readability. Furthermore we could pimp pread to only block for
outstanding writes and not for reads.

Since you're not the first one to trip over this: Can I volunteer you
for a follow-up patch to fix this?

Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_gem.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f026153..a839bcb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -41,6 +41,9 @@ static void i915_gem_object_flush_gtt_write_domain(struct 
drm_i915_gem_object *o
 static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object 
*obj,
   bool force);
 static __must_check int
+i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
+  bool readonly);
+static __must_check int
 i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
   struct i915_address_space *vm,
   unsigned alignment,
@@ -430,11 +433,9 @@ i915_gem_shmem_pread(struct drm_device *dev,
 * optimizes for the case when the gpu will dirty the data
 * anyway again before the next pread happens. */
needs_clflush = !cpu_cache_is_coherent(dev, obj-cache_level);
-   if (i915_gem_obj_bound_any(obj)) {
-   ret = i915_gem_object_set_to_gtt_domain(obj, false);
-   if (ret)
-   return ret;
-   }
+   ret = i915_gem_object_wait_rendering(obj, false);
+   if (ret)
+   return ret;
}
 
ret = i915_gem_object_get_pages(obj);
@@ -746,11 +747,9 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
 * optimizes for the case when the gpu will use the data
 * right away and we therefore have to clflush anyway. */
needs_clflush_after = cpu_write_needs_clflush(obj);
-   if (i915_gem_obj_bound_any(obj)) {
-   ret = i915_gem_object_set_to_gtt_domain(obj, true);
-   if (ret)
-   return ret;
-   }
+   ret = i915_gem_object_wait_rendering(obj, false);
+   if (ret)
+   return ret;
}
/* Same trick applies to invalidate partially written cachelines read
 * before writing. */
-- 
1.8.4

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


[Intel-gfx] [PATCH 3/8] drm/i915: evict VM instead of everything

2013-08-30 Thread Ben Widawsky
When reserving objects during execbuf, it is possible to come across an
object which will not fit given the current fragmentation of the address
space. We do not have any defragment in drm_mm, so the strategy is to
instead evict everything, and reallocate objects.

With the upcoming addition of multiple VMs, there is no point to evict
everything since doing so is overkill for the specific case mentioned
above.

Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_drv.h|  1 +
 drivers/gpu/drm/i915/i915_gem_evict.c  | 17 -
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 +++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0602c4b..6e8ade0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2088,6 +2088,7 @@ int __must_check i915_gem_evict_something(struct 
drm_device *dev,
  unsigned cache_level,
  bool mappable,
  bool nonblock);
+int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
 int i915_gem_evict_everything(struct drm_device *dev);
 
 /* i915_gem_stolen.c */
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
b/drivers/gpu/drm/i915/i915_gem_evict.c
index e9033f0..a3e279d 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -155,7 +155,22 @@ found:
return ret;
 }
 
-static int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle)
+/**
+ * i915_gem_evict_vm - Try to free up VM space
+ *
+ * @vm: Address space to evict from
+ * @do_idle: Boolean directing whether to idle first.
+ *
+ * VM eviction is about freeing up virtual address space. If one wants fine
+ * grained eviction, they should see evict something for more details. In terms
+ * of freeing up actual system memory, this function may not accomplish the
+ * desired result. An object may be shared in multiple address space, and this
+ * function will not assert those objects be freed.
+ *
+ * Using do_idle will result in a more complete eviction because it retires, 
and
+ * inactivates current BOs.
+ */
+int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle)
 {
struct i915_vma *vma, *next;
int ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index ea4bacb..f0a0bd7 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -546,10 +546,16 @@ i915_gem_execbuffer_reserve(struct intel_ring_buffer 
*ring,
 {
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
+   struct i915_address_space *vm;
struct list_head ordered_vmas;
bool has_fenced_gpu_access = INTEL_INFO(ring-dev)-gen  4;
int retry;
 
+   if (list_empty(vmas))
+   return 0;
+
+   vm = list_first_entry(vmas, struct i915_vma, exec_list)-vm;
+
INIT_LIST_HEAD(ordered_vmas);
while (!list_empty(vmas)) {
struct drm_i915_gem_exec_object2 *entry;
@@ -638,7 +644,7 @@ err:/* Decrement pin count for bound 
objects */
if (ret != -ENOSPC || retry++)
return ret;
 
-   ret = i915_gem_evict_everything(ring-dev);
+   ret = i915_gem_evict_vm(vm, true);
if (ret)
return ret;
} while (1);
-- 
1.8.4

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


[Intel-gfx] [PATCH 4/8] drm/i915: trace vm eviction instead of everything

2013-08-30 Thread Ben Widawsky
Tracing vm eviction is really the event we care about. For the cases we
evict everything, we still will get the trace.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_gem_evict.c |  4 ++--
 drivers/gpu/drm/i915/i915_trace.h | 12 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
b/drivers/gpu/drm/i915/i915_gem_evict.c
index a3e279d..ff91ac6 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -175,6 +175,8 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool 
do_idle)
struct i915_vma *vma, *next;
int ret;
 
+   trace_i915_gem_evict_vm(vm);
+
if (do_idle) {
ret = i915_gpu_idle(vm-dev);
if (ret)
@@ -208,8 +210,6 @@ i915_gem_evict_everything(struct drm_device *dev)
if (lists_empty)
return -ENOSPC;
 
-   trace_i915_gem_evict_everything(dev);
-
/* The gpu_idle will flush everything in the write domain to the
 * active list. Then we must move everything off the active list
 * with retire requests.
diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index e2c5ee6..d2a5502 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -218,19 +218,19 @@ TRACE_EVENT(i915_gem_evict,
  __entry-mappable ? , mappable : )
 );
 
-TRACE_EVENT(i915_gem_evict_everything,
-   TP_PROTO(struct drm_device *dev),
-   TP_ARGS(dev),
+TRACE_EVENT(i915_gem_evict_vm,
+   TP_PROTO(struct i915_address_space *vm),
+   TP_ARGS(vm),
 
TP_STRUCT__entry(
-__field(u32, dev)
+__field(struct i915_address_space *, vm)
),
 
TP_fast_assign(
-  __entry-dev = dev-primary-index;
+  __entry-vm = vm;
  ),
 
-   TP_printk(dev=%d, __entry-dev)
+   TP_printk(vm=%p, __entry-vm)
 );
 
 TRACE_EVENT(i915_gem_ring_dispatch,
-- 
1.8.4

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


[Intel-gfx] [PATCH 5/8] drm/i915: Convert active API to VMA

2013-08-30 Thread Ben Widawsky
From: Ben Widawsky b...@bwidawsk.net

Even though we track object activity and not VMA, because we have the
active_list be based on the VM, it makes the most sense to use VMAs in the
APIs.

NOTE: Daniel intends to eventually rip out active/inactive LRUs, but for
now, leave them be.

Signed-off-by: Ben Widawsky b...@bwidawsk.net

Conflicts:
drivers/gpu/drm/i915/i915_gem_execbuffer.c
---
 drivers/gpu/drm/i915/i915_drv.h|  5 ++---
 drivers/gpu/drm/i915/i915_gem.c| 11 +--
 drivers/gpu/drm/i915/i915_gem_context.c|  8 
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +--
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6e8ade0..c9ed77a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1866,9 +1866,8 @@ static inline void i915_gem_object_unpin_pages(struct 
drm_i915_gem_object *obj)
 int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
 int i915_gem_object_sync(struct drm_i915_gem_object *obj,
 struct intel_ring_buffer *to);
-void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
-   struct intel_ring_buffer *ring);
-
+void i915_vma_move_to_active(struct i915_vma *vma,
+struct intel_ring_buffer *ring);
 int i915_gem_dumb_create(struct drm_file *file_priv,
 struct drm_device *dev,
 struct drm_mode_create_dumb *args);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a839bcb..8547b97 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1881,11 +1881,11 @@ i915_gem_object_get_pages(struct drm_i915_gem_object 
*obj)
return 0;
 }
 
-void
+static void
 i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
   struct intel_ring_buffer *ring)
 {
-   struct drm_device *dev = obj-base.dev;
+   struct drm_device *dev = ring-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
u32 seqno = intel_ring_get_seqno(ring);
 
@@ -1920,6 +1920,13 @@ i915_gem_object_move_to_active(struct 
drm_i915_gem_object *obj,
}
 }
 
+void i915_vma_move_to_active(struct i915_vma *vma,
+struct intel_ring_buffer *ring)
+{
+   list_move_tail(vma-mm_list, vma-vm-active_list);
+   return i915_gem_object_move_to_active(vma-obj, ring);
+}
+
 static void
 i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 403309c..6a0f568 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -436,11 +436,11 @@ static int do_switch(struct i915_hw_context *to)
 * MI_SET_CONTEXT instead of when the next seqno has completed.
 */
if (from != NULL) {
-   struct drm_i915_private *dev_priv = 
from-obj-base.dev-dev_private;
-   struct i915_address_space *ggtt = dev_priv-gtt.base;
+   struct drm_i915_private *dev_priv = ring-dev-dev_private;
+   struct i915_vma *vma =
+   i915_gem_obj_to_vma(from-obj, dev_priv-gtt.base);
from-obj-base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
-   list_move_tail(i915_gem_obj_to_vma(from-obj, ggtt)-mm_list, 
ggtt-active_list);
-   i915_gem_object_move_to_active(from-obj, ring);
+   i915_vma_move_to_active(vma, ring);
/* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
 * whole damn pipeline, we don't need to explicitly mark the
 * object dirty. The only exception is that the context must be
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index f0a0bd7..928d37b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -869,8 +869,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
obj-base.read_domains = obj-base.pending_read_domains;
obj-fenced_gpu_access = obj-pending_fenced_gpu_access;
 
-   list_move_tail(vma-mm_list, vma-vm-active_list);
-   i915_gem_object_move_to_active(obj, ring);
+   i915_vma_move_to_active(vma, ring);
if (obj-base.write_domain) {
obj-dirty = 1;
obj-last_write_seqno = intel_ring_get_seqno(ring);
-- 
1.8.4

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


[Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM

2013-08-30 Thread Ben Widawsky
From: Ben Widawsky b...@bwidawsk.net

As we plumb the code with more VM information, it has become more
obvious that the easiest way to deal with bind and unbind is to simply
put the function pointers in the vm, and let those choose the correct
way to handle the page table updates. This change allows many places in
the code to simply be vm-bind, and not have to worry about
distinguishing PPGTT vs GGTT.

Notice that this patch has no impact on functionality. I've decided to
save the actual change until the next patch because I think it's easier
to review that way. I'm happy to squash the two, or let Daniel do it on
merge.

v2:
Make ggtt handle the quirky aliasing ppgtt
Add flags to bind object to support above
Don't ever call bind/unbind directly for PPGTT until we have real, full
PPGTT (use NULLs to assert this)
Make sure we rebind the ggtt if there already is a ggtt binding. This
happens on set cache levels
Use VMA for bind/unbind (Daniel, Ben)

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_drv.h |  69 +---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 101 
 2 files changed, 140 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9ed77a..377ca63 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -461,6 +461,36 @@ enum i915_cache_level {
 
 typedef uint32_t gen6_gtt_pte_t;
 
+/**
+ * A VMA represents a GEM BO that is bound into an address space. Therefore, a
+ * VMA's presence cannot be guaranteed before binding, or after unbinding the
+ * object into/from the address space.
+ *
+ * To make things as simple as possible (ie. no refcounting), a VMA's lifetime
+ * will always be = an objects lifetime. So object refcounting should cover 
us.
+ */
+struct i915_vma {
+   struct drm_mm_node node;
+   struct drm_i915_gem_object *obj;
+   struct i915_address_space *vm;
+
+   /** This object's place on the active/inactive lists */
+   struct list_head mm_list;
+
+   struct list_head vma_link; /* Link in the object's VMA list */
+
+   /** This vma's place in the batchbuffer or on the eviction list */
+   struct list_head exec_list;
+
+   /**
+* Used for performing relocations during execbuffer insertion.
+*/
+   struct hlist_node exec_node;
+   unsigned long exec_handle;
+   struct drm_i915_gem_exec_object2 *exec_entry;
+
+};
+
 struct i915_address_space {
struct drm_mm mm;
struct drm_device *dev;
@@ -499,9 +529,18 @@ struct i915_address_space {
/* FIXME: Need a more generic return type */
gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
 enum i915_cache_level level);
+
+   /** Unmap an object from an address space. This usually consists of
+* setting the valid PTE entries to a reserved scratch page. */
+   void (*unbind_vma)(struct i915_vma *vma);
void (*clear_range)(struct i915_address_space *vm,
unsigned int first_entry,
unsigned int num_entries);
+   /* Map an object into an address space with the given cache flags. */
+#define GLOBAL_BIND (10)
+   void (*bind_vma)(struct i915_vma *vma,
+enum i915_cache_level cache_level,
+u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
   struct sg_table *st,
   unsigned int first_entry,
@@ -548,36 +587,6 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
 };
 
-/**
- * A VMA represents a GEM BO that is bound into an address space. Therefore, a
- * VMA's presence cannot be guaranteed before binding, or after unbinding the
- * object into/from the address space.
- *
- * To make things as simple as possible (ie. no refcounting), a VMA's lifetime
- * will always be = an objects lifetime. So object refcounting should cover 
us.
- */
-struct i915_vma {
-   struct drm_mm_node node;
-   struct drm_i915_gem_object *obj;
-   struct i915_address_space *vm;
-
-   /** This object's place on the active/inactive lists */
-   struct list_head mm_list;
-
-   struct list_head vma_link; /* Link in the object's VMA list */
-
-   /** This vma's place in the batchbuffer or on the eviction list */
-   struct list_head exec_list;
-
-   /**
-* Used for performing relocations during execbuffer insertion.
-*/
-   struct hlist_node exec_node;
-   unsigned long exec_handle;
-   struct drm_i915_gem_exec_object2 *exec_entry;
-
-};
-
 struct i915_ctx_hang_stats {
/* This context had batch pending when hang was declared */
unsigned batch_pending;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 212f6d8..d5a4580 100644
--- 

[Intel-gfx] [PATCH 7/8] drm/i915: Use the new vm [un]bind functions

2013-08-30 Thread Ben Widawsky
From: Ben Widawsky b...@bwidawsk.net

Building on the last patch which created the new function pointers in
the VM for bind/unbind, here we actually put those new function pointers
to use.

Split out as a separate patch to aid in review. I'm fine with squashing
into the previous patch if people request it.

v2: Updated to address the smart ggtt which can do aliasing as needed
Make sure we bind to global gtt when mappable and fenceable. I thought
we could get away without this initialy, but we cannot.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_drv.h|  9 --
 drivers/gpu/drm/i915/i915_gem.c| 31 ---
 drivers/gpu/drm/i915/i915_gem_context.c|  8 +++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 29 --
 drivers/gpu/drm/i915/i915_gem_gtt.c| 48 ++
 5 files changed, 34 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 377ca63..94cbd0a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2065,17 +2065,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device 
*dev, void *data,
 
 /* i915_gem_gtt.c */
 void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
-void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
-   struct drm_i915_gem_object *obj,
-   enum i915_cache_level cache_level);
-void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
- struct drm_i915_gem_object *obj);
-
 void i915_gem_restore_gtt_mappings(struct drm_device *dev);
 int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
-void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
-   enum i915_cache_level cache_level);
-void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 void i915_gem_init_global_gtt(struct drm_device *dev);
 void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8547b97..0082c81 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2645,12 +2645,8 @@ int i915_vma_unbind(struct i915_vma *vma)
 
trace_i915_vma_unbind(vma);
 
-   if (obj-has_global_gtt_mapping)
-   i915_gem_gtt_unbind_object(obj);
-   if (obj-has_aliasing_ppgtt_mapping) {
-   i915_ppgtt_unbind_object(dev_priv-mm.aliasing_ppgtt, obj);
-   obj-has_aliasing_ppgtt_mapping = 0;
-   }
+   vma-vm-unbind_vma(vma);
+
i915_gem_gtt_finish_object(obj);
i915_gem_object_unpin_pages(obj);
 
@@ -3377,7 +3373,6 @@ int i915_gem_object_set_cache_level(struct 
drm_i915_gem_object *obj,
enum i915_cache_level cache_level)
 {
struct drm_device *dev = obj-base.dev;
-   drm_i915_private_t *dev_priv = dev-dev_private;
struct i915_vma *vma;
int ret;
 
@@ -3416,11 +3411,8 @@ int i915_gem_object_set_cache_level(struct 
drm_i915_gem_object *obj,
return ret;
}
 
-   if (obj-has_global_gtt_mapping)
-   i915_gem_gtt_bind_object(obj, cache_level);
-   if (obj-has_aliasing_ppgtt_mapping)
-   i915_ppgtt_bind_object(dev_priv-mm.aliasing_ppgtt,
-  obj, cache_level);
+   list_for_each_entry(vma, obj-vma_list, vma_link)
+   vma-vm-bind_vma(vma, cache_level, 0);
}
 
list_for_each_entry(vma, obj-vma_list, vma_link)
@@ -3748,6 +3740,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
bool map_and_fenceable,
bool nonblocking)
 {
+   const u32 flags = map_and_fenceable ? GLOBAL_BIND : 0;
struct i915_vma *vma;
int ret;
 
@@ -3776,20 +3769,22 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
}
 
if (!i915_gem_obj_bound(obj, vm)) {
-   struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
-
ret = i915_gem_object_bind_to_vm(obj, vm, alignment,
 map_and_fenceable,
 nonblocking);
if (ret)
return ret;
 
-   if (!dev_priv-mm.aliasing_ppgtt)
-   i915_gem_gtt_bind_object(obj, obj-cache_level);
-   }
+   vma = i915_gem_obj_to_vma(obj, vm);
+   vm-bind_vma(vma, obj-cache_level, flags);
+   } else
+   vma = i915_gem_obj_to_vma(obj, vm);
 
+   /* Objects are created map and fenceable. If we bind an object
+* the first time, and we had aliasing PPGTT 

[Intel-gfx] [PATCH 8/8] drm/i915: eliminate vm-insert_entries()

2013-08-30 Thread Ben Widawsky
From: Ben Widawsky b...@bwidawsk.net

With bind/unbind function pointers in place, we no longer need
insert_entries. We could, and want, to remove clear_range, however it's
not totally easy at this point. Since it's used in a couple of place
still that don't only deal in objects: setup, ppgtt init, and restore
gtt mappings.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 3c752ad..72c8b8a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -339,8 +339,8 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-base.unbind_vma = NULL;
ppgtt-base.clear_range = gen6_ppgtt_clear_range;
-   ppgtt-base.bind_vma = NULL;
ppgtt-base.insert_entries = gen6_ppgtt_insert_entries;
+   ppgtt-base.bind_vma = NULL;
ppgtt-base.cleanup = gen6_ppgtt_cleanup;
ppgtt-base.scratch = dev_priv-gtt.base.scratch;
ppgtt-pt_pages = kzalloc(sizeof(struct page *)*ppgtt-num_pd_entries,
@@ -591,19 +591,6 @@ static void gen6_ggtt_clear_range(struct 
i915_address_space *vm,
readl(gtt_base);
 }
 
-
-static void i915_ggtt_insert_entries(struct i915_address_space *vm,
-struct sg_table *st,
-unsigned int pg_start,
-enum i915_cache_level cache_level)
-{
-   unsigned int flags = (cache_level == I915_CACHE_NONE) ?
-   AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
-
-   intel_gtt_insert_sg_entries(st, pg_start, flags);
-
-}
-
 static void i915_ggtt_bind_vma(struct i915_vma *vma,
   enum i915_cache_level cache_level,
   u32 unused)
@@ -916,7 +903,6 @@ static int gen6_gmch_probe(struct drm_device *dev,
 
dev_priv-gtt.base.clear_range = gen6_ggtt_clear_range;
dev_priv-gtt.base.unbind_vma = gen6_ggtt_unbind_vma;
-   dev_priv-gtt.base.insert_entries = gen6_ggtt_insert_entries;
dev_priv-gtt.base.bind_vma = gen6_ggtt_bind_vma;
 
return ret;
@@ -950,7 +936,6 @@ static int i915_gmch_probe(struct drm_device *dev,
dev_priv-gtt.do_idle_maps = needs_idle_maps(dev_priv-dev);
dev_priv-gtt.base.clear_range = i915_ggtt_clear_range;
dev_priv-gtt.base.unbind_vma = i915_ggtt_unbind_vma;
-   dev_priv-gtt.base.insert_entries = i915_ggtt_insert_entries;
dev_priv-gtt.base.bind_vma = i915_ggtt_bind_vma;
 
return 0;
-- 
1.8.4

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


Re: [Intel-gfx] [PATCH 1/8] drm/i915: Synchronize pread/pwrite with wait_rendering

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 04:43:54PM -0700, Ben Widawsky wrote:
 lifted from Daniel:
 pread/pwrite isn't about the object's domain at all, but purely about
 synchronizing for outstanding rendering. Replacing the call to
 set_to_gtt_domain with a wait_rendering would imo improve code
 readability. Furthermore we could pimp pread to only block for
 outstanding writes and not for reads.
 
 Since you're not the first one to trip over this: Can I volunteer you
 for a follow-up patch to fix this?
 
 Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

This should fail i-g-t...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/8] drm/i915: Extract vm specific part of eviction

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 04:43:55PM -0700, Ben Widawsky wrote:
 As we'll see in the next patch, being able to evict for just 1 VM is
 handy.
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 ---
  drivers/gpu/drm/i915/i915_gem_evict.c | 28 ++--
  1 file changed, 22 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
 b/drivers/gpu/drm/i915/i915_gem_evict.c
 index cc8974f..e9033f0 100644
 --- a/drivers/gpu/drm/i915/i915_gem_evict.c
 +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
 @@ -155,12 +155,31 @@ found:
   return ret;
  }
  
 +static int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle)
 +{
 + struct i915_vma *vma, *next;
 + int ret;
 +
 + if (do_idle) {
 + ret = i915_gpu_idle(vm-dev);
 + if (ret)
 + return ret;
 +
 + i915_gem_retire_requests(vm-dev);
 + }
 +
 + list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
 + if (vma-obj-pin_count == 0)
 + WARN_ON(i915_vma_unbind(vma));
 +
 + return 0;
 +}
 +
  int
  i915_gem_evict_everything(struct drm_device *dev)
  {
   drm_i915_private_t *dev_priv = dev-dev_private;
   struct i915_address_space *vm;
 - struct i915_vma *vma, *next;
   bool lists_empty = true;
   int ret;
  
 @@ -187,11 +206,8 @@ i915_gem_evict_everything(struct drm_device *dev)
   i915_gem_retire_requests(dev);
  
   /* Having flushed everything, unbind() should never raise an error */
 - list_for_each_entry(vm, dev_priv-vm_list, global_link) {
 - list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
 - if (vma-obj-pin_count == 0)
 - WARN_ON(i915_vma_unbind(vma));
 - }
 + list_for_each_entry(vm, dev_priv-vm_list, global_link)
 + WARN_ON(i915_gem_evict_vm(vm, false));

Wny not use do_idle here?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/8] drm/i915: evict VM instead of everything

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 04:43:56PM -0700, Ben Widawsky wrote:
 When reserving objects during execbuf, it is possible to come across an
 object which will not fit given the current fragmentation of the address
 space. We do not have any defragment in drm_mm, so the strategy is to
 instead evict everything, and reallocate objects.
 
 With the upcoming addition of multiple VMs, there is no point to evict
 everything since doing so is overkill for the specific case mentioned
 above.
 
 Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

This loses the ENOSPC notification, and the tracepoints. I've just seen you
add tracepoints later..

I'm pretty certain this will be caught by an extra pass through reserve,
and probably worth it for the simplification. Fortunately, we do have
i-g-t coverage for this.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/8] drm/i915: trace vm eviction instead of everything

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 04:43:57PM -0700, Ben Widawsky wrote:
 Tracing vm eviction is really the event we care about. For the cases we
 evict everything, we still will get the trace.

Keep both until you retire evict_everything for good. Sometimes you need
flashing neon lights to explain things in a trace.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/8] drm/i915: Convert active API to VMA

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 04:43:58PM -0700, Ben Widawsky wrote:
 From: Ben Widawsky b...@bwidawsk.net
 
 Even though we track object activity and not VMA, because we have the
 active_list be based on the VM, it makes the most sense to use VMAs in the
 APIs.
 
 NOTE: Daniel intends to eventually rip out active/inactive LRUs, but for
 now, leave them be.
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 
 Conflicts:
   drivers/gpu/drm/i915/i915_gem_execbuffer.c

There's a silly chunk here, but otherwise ok.

 ---
  drivers/gpu/drm/i915/i915_drv.h|  5 ++---
  drivers/gpu/drm/i915/i915_gem.c| 11 +--
  drivers/gpu/drm/i915/i915_gem_context.c|  8 
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +--
  4 files changed, 16 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 6e8ade0..c9ed77a 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1866,9 +1866,8 @@ static inline void i915_gem_object_unpin_pages(struct 
 drm_i915_gem_object *obj)
  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
  int i915_gem_object_sync(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *to);
 -void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
 - struct intel_ring_buffer *ring);
 -
 +void i915_vma_move_to_active(struct i915_vma *vma,
 +  struct intel_ring_buffer *ring);
  int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index a839bcb..8547b97 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -1881,11 +1881,11 @@ i915_gem_object_get_pages(struct drm_i915_gem_object 
 *obj)
   return 0;
  }
  
 -void
 +static void
  i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
  struct intel_ring_buffer *ring)
  {
 - struct drm_device *dev = obj-base.dev;
 + struct drm_device *dev = ring-dev;

? Are you trying to anger the static checkers?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 6/8] drm/i915: Add bind/unbind object functions to VM

2013-08-30 Thread Chris Wilson
On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote:
 From: Ben Widawsky b...@bwidawsk.net
 
 As we plumb the code with more VM information, it has become more
 obvious that the easiest way to deal with bind and unbind is to simply
 put the function pointers in the vm, and let those choose the correct
 way to handle the page table updates. This change allows many places in
 the code to simply be vm-bind, and not have to worry about
 distinguishing PPGTT vs GGTT.
 
 Notice that this patch has no impact on functionality. I've decided to
 save the actual change until the next patch because I think it's easier
 to review that way. I'm happy to squash the two, or let Daniel do it on
 merge.
 
 v2:
 Make ggtt handle the quirky aliasing ppgtt
 Add flags to bind object to support above
 Don't ever call bind/unbind directly for PPGTT until we have real, full
 PPGTT (use NULLs to assert this)
 Make sure we rebind the ggtt if there already is a ggtt binding. This
 happens on set cache levels
 Use VMA for bind/unbind (Daniel, Ben)
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

You like pokkadot paint for its inconsistency? Other than interesting
alternation of styles, I see nothing wrong with the logic.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] Avoid i915 flip unpin/HPD event handler deadlock.

2013-08-30 Thread Stuart Abercrombie
Both of these were taking the mode_config mutex but executed from the
same work queue.  If intel_crtc_page_flip happened to flush a work queue
containing an HPD event handler work item, deadlock resulted, since the
mutex required by the HPD handler was taken before the flush.  Instead
use a separate work queue for the flip unpin work.

Signed-off-by: sabercrom...@chromium.org
Reviewed-by: marc...@chromium.org

---
 drivers/gpu/drm/i915/i915_dma.c  | 21 -
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4f129bb..9215360 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1558,6 +1558,22 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
goto out_mtrrfree;
}
 
+   /* intel_crtc_page_flip runs with the mode_config mutex having been
+* taken in the DRM layer.  It synchronously waits for pending unpin
+* work items while holding this mutex.  Therefore this queue cannot
+* contain work items that take this mutex, such as HPD event
+* handling, or we deadlock.  There is also no reason for flipping to
+* wait on such events.  Therefore put flip unpinning in its own
+* work queue.
+*/
+   dev_priv-flip_unpin_wq = alloc_ordered_workqueue(i915, 0);
+   if (dev_priv-flip_unpin_wq == NULL) {
+   DRM_ERROR(Failed to create flip unpin workqueue.\n);
+   destroy_workqueue(dev_priv-wq);
+   ret = -ENOMEM;
+   goto out_mtrrfree;
+   }
+
/* This must be called before any calls to HAS_PCH_* */
intel_detect_pch(dev);
 
@@ -1628,6 +1644,7 @@ out_gem_unload:
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
destroy_workqueue(dev_priv-wq);
+   destroy_workqueue(dev_priv-flip_unpin_wq);
 out_mtrrfree:
if (dev_priv-mm.gtt_mtrr = 0) {
mtrr_del(dev_priv-mm.gtt_mtrr,
@@ -1709,7 +1726,8 @@ int i915_driver_unload(struct drm_device *dev)
intel_opregion_fini(dev);
 
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-   /* Flush any outstanding unpin_work. */
+   /* Flush any outstanding unpin, HPD, etc. work. */
+   flush_workqueue(dev_priv-flip_unpin_wq);
flush_workqueue(dev_priv-wq);
 
mutex_lock(dev-struct_mutex);
@@ -1734,6 +1752,7 @@ int i915_driver_unload(struct drm_device *dev)
intel_teardown_mchbar(dev);
 
destroy_workqueue(dev_priv-wq);
+   destroy_workqueue(dev_priv-flip_unpin_wq);
 
pci_dev_put(dev_priv-bridge_dev);
kfree(dev-dev_private);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4130e6d..7eb4619 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -766,6 +766,7 @@ typedef struct drm_i915_private {
struct work_struct error_work;
struct completion error_completion;
struct workqueue_struct *wq;
+   struct workqueue_struct *flip_unpin_wq;
 
/* Display functions */
struct drm_i915_display_funcs display;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e204913..acd07dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6937,7 +6937,7 @@ static void do_intel_finish_page_flip(struct drm_device 
*dev,
  obj-pending_flip.counter);
wake_up(dev_priv-pending_flip_queue);
 
-   queue_work(dev_priv-wq, work-work);
+   queue_work(dev_priv-flip_unpin_wq, work-work);
 
trace_i915_flip_complete(intel_crtc-plane, work-pending_flip_obj);
 }
@@ -7305,7 +7305,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
spin_unlock_irqrestore(dev-event_lock, flags);
 
if (atomic_read(intel_crtc-unpin_work_count) = 2)
-   flush_workqueue(dev_priv-wq);
+   flush_workqueue(dev_priv-flip_unpin_wq);
 
ret = i915_mutex_lock_interruptible(dev);
if (ret)
-- 
1.8.4

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


Re: [Intel-gfx] [PATCH 1/8] drm/i915: Synchronize pread/pwrite with wait_rendering

2013-08-30 Thread Ben Widawsky
On Sat, Aug 31, 2013 at 12:50:30AM +0100, Chris Wilson wrote:
 On Fri, Aug 30, 2013 at 04:43:54PM -0700, Ben Widawsky wrote:
  lifted from Daniel:
  pread/pwrite isn't about the object's domain at all, but purely about
  synchronizing for outstanding rendering. Replacing the call to
  set_to_gtt_domain with a wait_rendering would imo improve code
  readability. Furthermore we could pimp pread to only block for
  outstanding writes and not for reads.
  
  Since you're not the first one to trip over this: Can I volunteer you
  for a follow-up patch to fix this?
  
  Recommended-by: Daniel Vetter daniel.vet...@ffwll.ch
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
 
 This should fail i-g-t...
 -Chris
 

Daniel, how have I failed your plan?

-- 
Ben Widawsky, 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/8] drm/i915: Extract vm specific part of eviction

2013-08-30 Thread Ben Widawsky
On Sat, Aug 31, 2013 at 12:52:03AM +0100, Chris Wilson wrote:
 On Fri, Aug 30, 2013 at 04:43:55PM -0700, Ben Widawsky wrote:
  As we'll see in the next patch, being able to evict for just 1 VM is
  handy.
  
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
  ---
   drivers/gpu/drm/i915/i915_gem_evict.c | 28 ++--
   1 file changed, 22 insertions(+), 6 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
  b/drivers/gpu/drm/i915/i915_gem_evict.c
  index cc8974f..e9033f0 100644
  --- a/drivers/gpu/drm/i915/i915_gem_evict.c
  +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
  @@ -155,12 +155,31 @@ found:
  return ret;
   }
   
  +static int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle)
  +{
  +   struct i915_vma *vma, *next;
  +   int ret;
  +
  +   if (do_idle) {
  +   ret = i915_gpu_idle(vm-dev);
  +   if (ret)
  +   return ret;
  +
  +   i915_gem_retire_requests(vm-dev);
  +   }
  +
  +   list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
  +   if (vma-obj-pin_count == 0)
  +   WARN_ON(i915_vma_unbind(vma));
  +
  +   return 0;
  +}
  +
   int
   i915_gem_evict_everything(struct drm_device *dev)
   {
  drm_i915_private_t *dev_priv = dev-dev_private;
  struct i915_address_space *vm;
  -   struct i915_vma *vma, *next;
  bool lists_empty = true;
  int ret;
   
  @@ -187,11 +206,8 @@ i915_gem_evict_everything(struct drm_device *dev)
  i915_gem_retire_requests(dev);
   
  /* Having flushed everything, unbind() should never raise an error */
  -   list_for_each_entry(vm, dev_priv-vm_list, global_link) {
  -   list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
  -   if (vma-obj-pin_count == 0)
  -   WARN_ON(i915_vma_unbind(vma));
  -   }
  +   list_for_each_entry(vm, dev_priv-vm_list, global_link)
  +   WARN_ON(i915_gem_evict_vm(vm, false));
 
 Wny not use do_idle here?
 -Chris
 

The point of the extraction was to exactly avoid repeated call to idle +
retire, although in looking back at it, I  suppose the cost wouldn't be
much since the first idle would make subsequent calls to idle + retire
just about free).

Just dropping the do_idle parameter, and always idling is what you
probably want.

-- 
Ben Widawsky, 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 4/8] drm/i915: trace vm eviction instead of everything

2013-08-30 Thread Ben Widawsky
On Sat, Aug 31, 2013 at 01:06:02AM +0100, Chris Wilson wrote:
 On Fri, Aug 30, 2013 at 04:43:57PM -0700, Ben Widawsky wrote:
  Tracing vm eviction is really the event we care about. For the cases we
  evict everything, we still will get the trace.
 
 Keep both until you retire evict_everything for good. Sometimes you need
 flashing neon lights to explain things in a trace.
 -Chris
 

Got it. Will resend after Daniel has a chance to look. Do you want this
squashed in with the previous patch to keep the trace events as well?

-- 
Ben Widawsky, 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 6/8] drm/i915: Add bind/unbind object functions to VM

2013-08-30 Thread Ben Widawsky
On Sat, Aug 31, 2013 at 01:12:55AM +0100, Chris Wilson wrote:
 On Fri, Aug 30, 2013 at 04:43:59PM -0700, Ben Widawsky wrote:
  From: Ben Widawsky b...@bwidawsk.net
  
  As we plumb the code with more VM information, it has become more
  obvious that the easiest way to deal with bind and unbind is to simply
  put the function pointers in the vm, and let those choose the correct
  way to handle the page table updates. This change allows many places in
  the code to simply be vm-bind, and not have to worry about
  distinguishing PPGTT vs GGTT.
  
  Notice that this patch has no impact on functionality. I've decided to
  save the actual change until the next patch because I think it's easier
  to review that way. I'm happy to squash the two, or let Daniel do it on
  merge.
  
  v2:
  Make ggtt handle the quirky aliasing ppgtt
  Add flags to bind object to support above
  Don't ever call bind/unbind directly for PPGTT until we have real, full
  PPGTT (use NULLs to assert this)
  Make sure we rebind the ggtt if there already is a ggtt binding. This
  happens on set cache levels
  Use VMA for bind/unbind (Daniel, Ben)
  
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
 
 You like pokkadot paint for its inconsistency? Other than interesting
 alternation of styles, I see nothing wrong with the logic.
 -Chris
 

To what are you referring? I'm probably more than willing to change
whatever displeases you.

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