[Intel-gfx] Delayed mail delivery problem

[Tian, Kevin]

I had some replies to this mailing list yesterday, but received below 
notification:

---
Delivery is delayed to these recipients or groups:

intel-gfx@lists.freedesktop.org

Subject: RE: [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context requirement

This message hasn't been delivered yet. Delivery will continue to be attempted.
---

Do I need to wait for some time or better to resend my replies?

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

Re: [Intel-gfx] [4.4-rc1][Regression] drm/i915: Check live status before reading edid

[Jindal, Sonika]

Hi Joe,

Yes, first thing to try is to increase the tries.
Can you please point me to the bug and provide more details like platform, 
monitor, cable.
Are you referring to the same issue as Oleksandr reported where a single link 
dvi/hdmi cable didn’t work and dual link worked?

Regards,
Sonika

-Original Message-
From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] 
Sent: Thursday, February 25, 2016 3:09 AM
To: Jindal, Sonika 
Cc: Sharma, Shashank ; Vivi, Rodrigo 
; Daniel Vetter ; Jani Nikula 
; David Airlie ; intel-gfx 
; dri-devel ; 
LKML 
Subject: [4.4-rc1][Regression] drm/i915: Check live status before reading edid

Hi Sonika,

A kernel bug report was opened against Ubuntu [0].  After a kernel bisect, it 
was found that reverting the following commit resolved this bug:

commit 237ed86c693d8a8e4db476976aeb30df4deac74b
Author: Sonika Jindal 
Date:   Tue Sep 15 09:44:20 2015 +0530

drm/i915: Check live status before reading edid



The regression was introduced as of v4.4-rc1.

I was hoping to get your feedback, since you are the patch author.  Do think 
increasing the number of tries in intel_hdmi_detect() is worth trying?  Do you 
think gathering any additional data will help diagnose this issue, or would it 
be best to submit a revert request?


Thanks,

Joe

[0] http://pad.lv/lp1543683

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

[Intel-gfx] [4.4-rc1][Regression] drm/i915: Check live status before reading edid

[Joseph Salisbury]

Hi Sonika,

A kernel bug report was opened against Ubuntu [0].  After a kernel
bisect, it was found that reverting the following commit resolved this bug:

commit 237ed86c693d8a8e4db476976aeb30df4deac74b
Author: Sonika Jindal 
Date:   Tue Sep 15 09:44:20 2015 +0530

drm/i915: Check live status before reading edid



The regression was introduced as of v4.4-rc1.

I was hoping to get your feedback, since you are the patch author.  Do
think increasing the number of tries in intel_hdmi_detect() is worth
trying?  Do you think gathering any additional data will help diagnose
this issue, or would it be best to submit a revert request?


Thanks,

Joe

[0] http://pad.lv/lp1543683

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

Re: [Intel-gfx] [BUG] HDMI 12bpc causing occasional flickering and blanking

[Ville Syrjälä]

On Wed, Feb 24, 2016 at 06:48:35PM +0100, Tore Anderson wrote:
> Hi Ville,
> 
> > Well, just to check the details of your particular cable/dongle,
> > maybe you can post the dmesg with drm.debug=0xe with my branch?
> > Or at least the parts that refer to DP dual mode adaptors.
> 
> Here: http://filebin.net/gd91wnltky/dmesg.4.5.0-rc4-g32fa589.txt
> 
> I'm assuming the most important parts are:
> 
> [drm:intel_hdmi_dp_dual_mode_detect] DP dual mode adaptor (type 1 HDMI) 
> detected (max TMDS clock: 165000 kHz, TMDS OE# control: no)
> [drm:intel_hdmi_compute_config] picking bpc to 8 for HDMI output
> 
> For the record, I'd call my adapter a «cable» and not a «dongle». I had
> thought it was completely passive; if there's any active circuitry
> inside it it must be really tiny - it'd have to fit inside the rather
> standard-sized HDMI/DP connectors in the ends.

Yeah, the chips used for this stuff are pretty small. I had one dongle
fry itself a while back, so I pulled it apart and found a PS8121E
chip inside it (with a number of other mostly passive components).

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

[Intel-gfx] [PATCH 3/4] drm/i915/gen9: Disable DC states if power well support is disabled

[Imre Deak]

If power well support is disabled via the i915.disable_power_well module
option we should never enable DC states. Currently we would enable DC
states even in this case during system suspend, where we need to disable
all power wells regardless of the disable_power_well option.

CC: Patrik Jakobsson 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 88df99e..7f65d5f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2030,6 +2030,9 @@ static uint32_t get_allowed_dc_mask(const struct 
drm_i915_private *dev_priv,
if (!HAS_CSR(dev_priv))
return mask;
 
+   if (!i915.disable_power_well)
+   return mask;
+
mask |= DC_STATE_EN_UPTO_DC5;
if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
mask |= DC_STATE_EN_UPTO_DC6;
-- 
2.5.0

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

[Intel-gfx] [PATCH 1/4] drm/i915/skl: Fix power domain suspend sequence

[Imre Deak]

During system suspend we need to first disable power wells then
unitialize the display core. In case power well support is disabled we
did this in the wrong order, so fix this up.

Fixes: d314cd43 ("drm/i915: fix handling of the disable_power_well module 
option")
CC: sta...@vger.kernel.org
CC: Patrik Jakobsson 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e232976..8276dc2 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2319,15 +2319,15 @@ void intel_power_domains_init_hw(struct 
drm_i915_private *dev_priv, bool resume)
  */
 void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
 {
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
-   skl_display_core_uninit(dev_priv);
-
/*
 * Even if power well support was disabled we still want to disable
 * power wells while we are system suspended.
 */
if (!i915.disable_power_well)
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
+   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+   skl_display_core_uninit(dev_priv);
 }
 
 /**
-- 
2.5.0

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

[Intel-gfx] [PATCH 2/4] drm/i915/gen9: Sanitize handling of allowed DC states

[Imre Deak]

We can simplify the conditions selecting the target DC state during
runtime by calculating the allowed DC states in advance during driver
loading. This also makes it easier to disable DC states depending on the
i915.disable_power_well module option, added in the next patch.

CC: Patrik Jakobsson 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 74 +++--
 2 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9e76bfc..b563de5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -750,6 +750,7 @@ struct intel_csr {
i915_reg_t mmioaddr[8];
uint32_t mmiodata[8];
uint32_t dc_state;
+   uint32_t allowed_dc_mask;
 };
 
 #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8276dc2..88df99e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -538,12 +538,8 @@ static void gen9_set_dc_state(struct drm_i915_private 
*dev_priv, uint32_t state)
else
mask |= DC_STATE_EN_UPTO_DC6;
 
-   WARN_ON_ONCE(state & ~mask);
-
-   if (i915.enable_dc == 0)
-   state = DC_STATE_DISABLE;
-   else if (i915.enable_dc == 1 && state > DC_STATE_EN_UPTO_DC5)
-   state = DC_STATE_EN_UPTO_DC5;
+   if (WARN_ON_ONCE(state & ~dev_priv->csr.allowed_dc_mask))
+   state &= dev_priv->csr.allowed_dc_mask;
 
val = I915_READ(DC_STATE_EN);
DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n",
@@ -659,8 +655,7 @@ static void gen9_disable_dc5_dc6(struct drm_i915_private 
*dev_priv)
 {
assert_can_disable_dc5(dev_priv);
 
-   if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
-   i915.enable_dc != 0 && i915.enable_dc != 1)
+   if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
assert_can_disable_dc6(dev_priv);
 
gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
@@ -839,26 +834,19 @@ static void gen9_dc_off_power_well_enable(struct 
drm_i915_private *dev_priv,
 static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
   struct i915_power_well *power_well)
 {
-   if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
-   i915.enable_dc != 0 && i915.enable_dc != 1)
+   if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
skl_enable_dc6(dev_priv);
-   else
+   else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
gen9_enable_dc5(dev_priv);
 }
 
 static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
   struct i915_power_well *power_well)
 {
-   if (power_well->count > 0) {
-   gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
-   } else {
-   if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
-   i915.enable_dc != 0 &&
-   i915.enable_dc != 1)
-   gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
-   else
-   gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
-   }
+   if (power_well->count > 0)
+   gen9_dc_off_power_well_enable(dev_priv, power_well);
+   else
+   gen9_dc_off_power_well_disable(dev_priv, power_well);
 }
 
 static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
@@ -2023,6 +2011,48 @@ sanitize_disable_power_well_option(const struct 
drm_i915_private *dev_priv,
return 1;
 }
 
+static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
+   int enable_dc)
+{
+   uint32_t mask = 0;
+
+   /*
+* DC9 has a separate HW flow from the rest of the DC states, not
+* depending on the DMC firmware. It's needed by system
+* suspend/resume, so allow it unconditionally.
+*/
+   if (IS_BROXTON(dev_priv))
+   mask |= DC_STATE_EN_DC9;
+
+   if (!enable_dc)
+   return mask;
+
+   if (!HAS_CSR(dev_priv))
+   return mask;
+
+   mask |= DC_STATE_EN_UPTO_DC5;
+   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+   mask |= DC_STATE_EN_UPTO_DC6;
+   } else if (!IS_BROXTON(dev_priv)) {
+   MISSING_CASE(INTEL_DEVID(dev_priv));
+   mask = 0;
+   }
+
+   switch (enable_dc) {
+   case 1:
+   mask &= ~DC_STATE_EN_UPTO_DC6;
+   break;
+   case 2:
+   case -1:
+   break;
+   default:
+   DRM_ERROR("Unexpected value for enable_dc (%d)\n", enable_dc);
+   break;
+ 

[Intel-gfx] [PATCH 4/4] drm/i915/gen9: Remove state asserts when disabling DC states

[Imre Deak]

Disabling the DC states when it's already disabled is a valid scenario,
for example during HW state sanitization during driver loading and
resuming or when DC states are disabled via the i915.enable_dc or
disable_power_well option.

CC: Patrik Jakobsson 
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 41 +
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 7f65d5f..1661c2a 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -458,8 +458,6 @@ static void assert_can_enable_dc9(struct drm_i915_private 
*dev_priv)
 static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
 {
WARN(intel_irqs_enabled(dev_priv), "Interrupts not disabled yet.\n");
-   WARN(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_DC9),
-   "DC9 already programmed to be disabled.\n");
WARN(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5,
"DC5 still not disabled.\n");
 
@@ -602,18 +600,6 @@ static void assert_can_enable_dc5(struct drm_i915_private 
*dev_priv)
assert_csr_loaded(dev_priv);
 }
 
-static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
-{
-   /*
-* During initialization, the firmware may not be loaded yet.
-* We still want to make sure that the DC enabling flag is cleared.
-*/
-   if (dev_priv->power_domains.initializing)
-   return;
-
-   assert_rpm_wakelock_held(dev_priv);
-}
-
 static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 {
assert_can_enable_dc5(dev_priv);
@@ -638,29 +624,6 @@ static void assert_can_enable_dc6(struct drm_i915_private 
*dev_priv)
assert_csr_loaded(dev_priv);
 }
 
-static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
-{
-   /*
-* During initialization, the firmware may not be loaded yet.
-* We still want to make sure that the DC enabling flag is cleared.
-*/
-   if (dev_priv->power_domains.initializing)
-   return;
-
-   WARN_ONCE(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6),
- "DC6 already programmed to be disabled.\n");
-}
-
-static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
-{
-   assert_can_disable_dc5(dev_priv);
-
-   if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
-   assert_can_disable_dc6(dev_priv);
-
-   gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
-}
-
 void skl_enable_dc6(struct drm_i915_private *dev_priv)
 {
assert_can_enable_dc6(dev_priv);
@@ -673,8 +636,6 @@ void skl_enable_dc6(struct drm_i915_private *dev_priv)
 
 void skl_disable_dc6(struct drm_i915_private *dev_priv)
 {
-   assert_can_disable_dc6(dev_priv);
-
DRM_DEBUG_KMS("Disabling DC6\n");
 
gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
@@ -828,7 +789,7 @@ static bool gen9_dc_off_power_well_enabled(struct 
drm_i915_private *dev_priv,
 static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
  struct i915_power_well *power_well)
 {
-   gen9_disable_dc5_dc6(dev_priv);
+   gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 }
 
 static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
-- 
2.5.0

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

[Intel-gfx] [PATCH 0/4] drm/915: Sanitize DC state handling

[Imre Deak]

This patchset simplifies the way how we select the target DC state based
on the platform and module options. It also fixes one ordering problem I
noticed while going through the code, see the first patch.

Imre Deak (4):
  drm/i915/skl: Fix power domain suspend sequence
  drm/i915/gen9: Sanitize handling of allowed DC states
  drm/i915/gen9: Disable DC states if power well support is disabled
  drm/i915/gen9: Remove state asserts when disabling DC states

 drivers/gpu/drm/i915/i915_drv.h |   1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 122 +++-
 2 files changed, 60 insertions(+), 63 deletions(-)

-- 
2.5.0

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

Re: [Intel-gfx] [BUG] HDMI 12bpc causing occasional flickering and blanking

[Tore Anderson]

Hi Ville,

> Well, just to check the details of your particular cable/dongle,
> maybe you can post the dmesg with drm.debug=0xe with my branch?
> Or at least the parts that refer to DP dual mode adaptors.

Here: http://filebin.net/gd91wnltky/dmesg.4.5.0-rc4-g32fa589.txt

I'm assuming the most important parts are:

[drm:intel_hdmi_dp_dual_mode_detect] DP dual mode adaptor (type 1 HDMI) 
detected (max TMDS clock: 165000 kHz, TMDS OE# control: no)
[drm:intel_hdmi_compute_config] picking bpc to 8 for HDMI output

For the record, I'd call my adapter a «cable» and not a «dongle». I had
thought it was completely passive; if there's any active circuitry
inside it it must be really tiny - it'd have to fit inside the rather
standard-sized HDMI/DP connectors in the ends.

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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect DP++ adaptor TMDS clock limit

[Ville Syrjälä]

On Tue, Feb 23, 2016 at 06:46:26PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Try to detect the max TMDS clock limit for the DP++ adaptor (if any)
> and take it into account when checking the port clock.
> 
> Note that as with the sink (HDMI vs. DVI) TMDS clock limit we'll ignore
> the adaptor TMDS clock limit in the modeset path, in case users are
> already "overclocking" their TMDS links. One subtle change here is that
> we'll have to respect the adaptor TMDS clock limit when we decide whether
> to do 12bpc or 8bpc, otherwise we might end up picking 12bpc and
> accidentally driving the TMDS link out of spec even when the user chose
> a mode that fits wihting the limits at 8bpc. This means you can't
> "overclock" your DP++ dongle at 12bpc anymore, but you can continue to
> do so at 8bpc.
> 
> Note that for simplicity we'll use the I2C access method for all dual
> mode adaptors including type 2. Otherwise we'd have to start mixing
> DP AUX and HDMI together. In the future we may need to do that if we
> come across any board designs that don't hook up the DDC pins to the
> DP++ connectors. Such boards would obviously only work with type 2
> dual mode adaptors, and not type 1.
> 
> Signed-off-by: Ville Syrjälä 

Reported-by: Tore Anderson 
Tested-by: Tore Anderson 
Fixes: 7a0baa623446 ("Revert "drm/i915: Disable 12bpc hdmi for now"")
References: 
https://lists.freedesktop.org/archives/intel-gfx/2016-February/088336.html

> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  3 ++
>  drivers/gpu/drm/i915/intel_hdmi.c | 65 
> ++-
>  2 files changed, 60 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 4852049c9ab3..944eacbfb6a9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -704,6 +704,9 @@ struct cxsr_latency {
>  struct intel_hdmi {
>   i915_reg_t hdmi_reg;
>   int ddc_bus;
> + struct {
> + int max_tmds_clock;
> + } dp_dual_mode;
>   bool limited_color_range;
>   bool color_range_auto;
>   bool has_hdmi_sink;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 80b44c054087..1e8cfdb18dc7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "intel_drv.h"
>  #include 
>  #include "i915_drv.h"
> @@ -1168,27 +1169,42 @@ static void pch_post_disable_hdmi(struct 
> intel_encoder *encoder)
>   intel_disable_hdmi(encoder);
>  }
>  
> -static int hdmi_port_clock_limit(struct intel_hdmi *hdmi, bool 
> respect_dvi_limit)
> +static int intel_hdmi_source_max_tmds_clock(struct drm_i915_private 
> *dev_priv)
>  {
> - struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> -
> - if ((respect_dvi_limit && !hdmi->has_hdmi_sink) || IS_G4X(dev))
> + if (IS_G4X(dev_priv))
>   return 165000;
> - else if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8)
> + else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
>   return 30;
>   else
>   return 225000;
>  }
>  
> +static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
> +  bool respect_downstream_limits)
> +{
> + struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> + int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
> +
> + if (respect_downstream_limits) {
> + if (hdmi->dp_dual_mode.max_tmds_clock)
> + max_tmds_clock = min(max_tmds_clock,
> +  hdmi->dp_dual_mode.max_tmds_clock);
> + if (!hdmi->has_hdmi_sink)
> + max_tmds_clock = min(max_tmds_clock, 165000);
> + }
> +
> + return max_tmds_clock;
> +}
> +
>  static enum drm_mode_status
>  hdmi_port_clock_valid(struct intel_hdmi *hdmi,
> -   int clock, bool respect_dvi_limit)
> +   int clock, bool respect_downstream_limits)
>  {
>   struct drm_device *dev = intel_hdmi_to_dev(hdmi);
>  
>   if (clock < 25000)
>   return MODE_CLOCK_LOW;
> - if (clock > hdmi_port_clock_limit(hdmi, respect_dvi_limit))
> + if (clock > hdmi_port_clock_limit(hdmi, respect_downstream_limits))
>   return MODE_CLOCK_HIGH;
>  
>   /* BXT DPLL can't generate 223-240 MHz */
> @@ -1312,7 +1328,7 @@ bool intel_hdmi_compute_config(struct intel_encoder 
> *encoder,
>* within limits.
>*/
>   if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink &&
> - hdmi_port_clock_valid(intel_hdmi, clock_12bpc, false) == MODE_OK &&
> + hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true) == MODE_OK &&
>   hdmi_12bpc_possible(pipe_config)) {
>   

[Intel-gfx] [PATCH] drm/i915/guc: Support GuC SKL v6.1

[yu . dai]

From: Alex Dai 

This version of GuC firmware fixes the engine reset issue where golden
context LRC address is treated as page index by mistake. It also fixes
the problem that scheduler stops submiting to one engine when the other
engine work queue is full.

Signed-off-by: Alex Dai 
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index e0093a9..e329a8a 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -59,7 +59,7 @@
  *
  */
 
-#define I915_SKL_GUC_UCODE "i915/skl_guc_ver4.bin"
+#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6.bin"
 MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
 
 /* User-friendly representation of an enum */
@@ -611,8 +611,8 @@ void intel_guc_ucode_init(struct drm_device *dev)
fw_path = NULL;
} else if (IS_SKYLAKE(dev)) {
fw_path = I915_SKL_GUC_UCODE;
-   guc_fw->guc_fw_major_wanted = 4;
-   guc_fw->guc_fw_minor_wanted = 3;
+   guc_fw->guc_fw_major_wanted = 6;
+   guc_fw->guc_fw_minor_wanted = 1;
} else {
fw_path = "";   /* unknown device */
}
-- 
2.5.0

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

Re: [Intel-gfx] [PATCH v6 3/8] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v6.

[Zanoni, Paulo R]

Em Qua, 2016-02-24 às 11:24 +0100, Maarten Lankhorst escreveu:
> Currently we perform our own wait in post_plane_update,
> but the atomic core performs another one in wait_for_vblanks.
> This means that 2 vblanks are done when a fb is changed,
> which is a bit overkill.
> 
> Merge them by creating a helper function that takes a crtc mask
> for the planes to wait on.
> 
> The broadwell vblank workaround may look gone entirely but this is
> not the case. pipe_config->wm_changed is set to true
> when any plane is turned on, which forces a vblank wait.
> 
> Changes since v1:
> - Removing the double vblank wait on broadwell moved to its own
> commit.
> Changes since v2:
> - Move out POWER_DOMAIN_MODESET handling to its own commit.
> Changes since v3:
> - Do not wait for vblank on legacy cursor updates. (Ville)
> - Move broadwell vblank workaround comment to page_flip_finished.
> (Ville)
> Changes since v4:
> - Compile fix, legacy_cursor_flip -> *_update.
> Changes since v5:
> - Kill brackets.
> - Add WARN_ON when wait_for_vblanks fails.
> - Remove extra newlines.
> - Split the checks whether vblank is needed to a separate function,
>   with comments why a vblank is needed.
> Signed-off-by: Maarten Lankhorst 

Reviewed-by: Paulo Zanoni 

> ---
> v5 was skipped, previous version was v5 because it had the compile
> fix.
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 4625f8a9ba12..8e579a8505ac 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -97,6 +97,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>   crtc_state->disable_lp_wm = false;
>   crtc_state->disable_cxsr = false;
>   crtc_state->wm_changed = false;
> + crtc_state->fb_changed = false;
>  
>   return _state->base;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2107e324cd9e..9f32cb0bf978 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4792,9 +4792,6 @@ static void intel_post_plane_update(struct
> intel_crtc *crtc)
>   to_intel_crtc_state(crtc->base.state);
>   struct drm_device *dev = crtc->base.dev;
>  
> - if (atomic->wait_vblank)
> - intel_wait_for_vblank(dev, crtc->pipe);
> -
>   intel_frontbuffer_flip(dev, atomic->fb_bits);
>  
>   crtc->wm.cxsr_allowed = true;
> @@ -10957,6 +10954,12 @@ static bool page_flip_finished(struct
> intel_crtc *crtc)
>   return true;
>  
>   /*
> +  * BDW signals flip done immediately if the plane
> +  * is disabled, even if the plane enable is already
> +  * armed to occur at the next vblank :(
> +  */
> +
> + /*
>    * A DSPSURFLIVE check isn't enough in case the mmio and CS
> flips
>    * used the same base address. In that case the mmio flip
> might
>    * have completed, but the CS hasn't even executed the flip
> yet.
> @@ -11833,6 +11836,9 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>   if (!was_visible && !visible)
>   return 0;
>  
> + if (fb != old_plane_state->base.fb)
> + pipe_config->fb_changed = true;
> +
>   turn_off = was_visible && (!visible || mode_changed);
>   turn_on = visible && (!was_visible || mode_changed);
>  
> @@ -11847,11 +11853,8 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>   pipe_config->wm_changed = true;
>  
>   /* must disable cxsr around plane enable/disable */
> - if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> - if (is_crtc_enabled)
> - intel_crtc->atomic.wait_vblank =
> true;
> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
>   pipe_config->disable_cxsr = true;
> - }
>   } else if (intel_wm_need_update(plane, plane_state)) {
>   pipe_config->wm_changed = true;
>   }
> @@ -11865,14 +11868,6 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>   intel_crtc->atomic.post_enable_primary = turn_on;
>   intel_crtc->atomic.update_fbc = true;
>  
> - /*
> -  * BDW signals flip done immediately if the plane
> -  * is disabled, even if the plane enable is already
> -  * armed to occur at the next vblank :(
> -  */
> - if (turn_on && IS_BROADWELL(dev))
> - intel_crtc->atomic.wait_vblank = true;
> -
>   break;
>   case DRM_PLANE_TYPE_CURSOR:
>   break;
> @@ -11885,13 +11880,11 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>    */
>   if (IS_IVYBRIDGE(dev) &&
>   needs_scaling(to_intel_plane_state(plane_state))
> &&
> - 

Re: [Intel-gfx] [PATCH v2] drm/i915: Avoid selecting unavailable BSD2 ring

[Gabriel Feceoru]

On 23.02.2016 21:36, Dave Gordon wrote:

On 23/02/16 14:39, Tvrtko Ursulin wrote:


On 23/02/16 14:03, Chris Wilson wrote:

On Tue, Feb 23, 2016 at 01:31:17PM +, Tvrtko Ursulin wrote:


On 23/02/16 13:06, Gabriel Feceoru wrote:



On 23.02.2016 13:05, Tvrtko Ursulin wrote:


Hi,

On 23/02/16 10:52, Gabriel Feceoru wrote:

Return error when I915_EXEC_BSD_RING2 flag is set but BSD2 ring
is not available in the HW.


What is the reasoning behind this? So far kernel was allowing
userspace
to select these bits and execute on the first engine. With this
patch it
would start failing potentially breaking userspace. Is it not too
late
to make such change?


I noticed some inconsistencies in igt with regards to bsd and bsd1.
For instance, if bsd2 is not available, gem_sync@basic-bsd1 is
skipped,
but it's skipped because of the 2nd check gem_has_bsd2 (see
gem_require_ring). Surprisingly gem_has_ring() didn't complain about
bsd1.

This fix will make gem_has_ring() return false.

I'm not aware about legacy/compatibility issue - if that's the case,
please disregard this.


Hmmm.. Chris, what is the reasoning behind:

commit eaa03678b00179da89f194113c0740c033857c1c
Author: Chris Wilson 
Date:   Thu Jan 28 13:44:19 2016 +

 lib: Hide BSD1/BSD2 rings on hardware without BSD2

 The kernel happily lets us run on I915_EXEC_BSD2 even with such
hardware
 existing. Sigh.

 Signed-off-by: Chris Wilson 

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 9dfa9b2603ce..fa44080e5902 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1341,6 +1341,12 @@ static int gem_has_ring(int fd, int ring)
  void gem_require_ring(int fd, int ring_id)
  {
 igt_require(gem_has_ring(fd, ring_id));
+
+   /* silly ABI, the kernel thinks everyone who has BSD also has
BSD2 */
+   if ((ring_id & ~(3<<13)) == I915_EXEC_BSD) {
+   if (ring_id & (3 << 13))
+   igt_require(gem_has_bsd2(fd));
+   }
  }

  /* prime */

ABI was (and still is) that specifying BSD1 or BSD2 explicitly is
silently ignored by the kernel when BSD2 is not preset, defaulting
to BSD1.


Thereby pretending that BSD, BSD1, BSD2 exist.


This patch makes tests requesting BSD1 skip when there is no BSD2
which I think is wrong in any case.


BSD 1/2 selection only makes sense when we have multiple BSD rings.
Running tests on BSD default, BSD1 and BSD2 is pointless if they all
are equivalent. Using the BSD ping-pong when we have BSD1 and BSD2 is
questionable as the ping-pong nature is uncontrolled, but nevertheless
the code path needs to be tested.


If we want to (and can) change the ABI it should only reject the
non-existent ring and not limit the selection mechanism to
hardware which has BSD2.


I disagree, we have a ring selection mechanism. If the extension doesn't
exist, trying to use it should be an error. The extension was not only
an ABI mistake but undesired (it is now defunct).


Not sure that I understand what you meant here. Nothing as far as I can
tell is now defunct. Neither the selection mechanism, or the existence
of BSD2.

To be absolutely clear, you are, or you are not, in favour of Gabriel's
patch to start failing execbuf with fine grained BSD selection flags
when BSD2 is not present?

Regards,
Tvrtko


Currently:

#define I915_EXEC_BSD (2<<0)

/** Used for switching BSD rings on the platforms with two BSD rings */
#define I915_EXEC_BSD_SHIFT   (13)
#define I915_EXEC_BSD_MASK(3 << I915_EXEC_BSD_SHIFT) /* default
ping-pong mode */
#define I915_EXEC_BSD_DEFAULT (0 << I915_EXEC_BSD_SHIFT)
#define I915_EXEC_BSD_RING1   (1 << I915_EXEC_BSD_SHIFT)
#define I915_EXEC_BSD_RING2   (2 << I915_EXEC_BSD_SHIFT)

It makes sense to have the original "BSD" flag mean "the default BSD",
and use different flags to mean specifically "BSD1" or "BSD2". Which
appears to be what we've done; naive clients that don't set any of the
new BSD bits will get default behaviour (execute on *any* BSD ring) with
no control over the ping-pong mechanism (if any). Clients that specify a
specific ring should get that one, and only that one; if it doesn't
exist then it's an error.

Any program that's going to set these bits should first ask whether (or
which) engines exist and select appropriately. So I think I'm with Chris
here.

On the other hand, I think what Tvrtko said was "it should not be an
error to select a specific ring [that exists] just because there are no
other rings". Which I also agree with.

So the ring-select-checking code should:
1. reject the I915_EXEC_BSD_RING2 case if BSD2 does not exist


This is one of the things the patch does, I guess everybody agrees on 
rejecting BSD2.



2. reject the I915_EXEC_BSD_RING1 case if BSD1 does not exist
  (for some future bizarre numbering scheme? or a
   partitioning system that reserves BSD1 for someone else?)
3. never reject the I915_EXEC_BSD_DEFAULT case
  (although it will 

Re: [Intel-gfx] [BUG] HDMI 12bpc causing occasional flickering and blanking

[Ville Syrjälä]

On Tue, Feb 23, 2016 at 08:44:49PM +0100, Tore Anderson wrote:
> Hi Ville,
> 
> > "The monitor is connected with a DP+-to-HDMI cable"
> > This and some reading of the DP dual mode spec gave me another idea;
> > The DP->HDMI adaptor may simply be degrading the signal quality too
> > much. According to the DP dual mode spec we're supposed to limit the
> > TMDS clock based on the type of adapter used, but currently we have
> > no code to do that. I've cooked up a few patches that should do what
> > we want:
> > git://github.com/vsyrjala/linux.git dp_dual_mode
> > 
> > I've quickly tested it locally, and it seemed to do the right thing
> > with a few different types of adaptors.
> 
> I've run 32fa589 for a few hours now and it have not seen a single
> blank or flicker. So it seems you've nailed it - thanks a lot!

Excellent.

> 
> Let me know if you want me to test more patches, post debug logs, or
> anything else.

Well, just to check the details of your particular cable/dongle,
maybe you can post the dmesg with drm.debug=0xe with my branch?
Or at least the parts that refer to DP dual mode adaptors.

> 
> BTW, also discovered right before you sent that e-mail that downgrading
> to a 1920x1080i mode (rather than the monitor's native 1920x1080) would
> also stop the flickering. I'd assume that also fits well with your
> diagnosis (less bandwidth needed => better tolerance for degraded signal
> quality), but I thought I'd let you know in case not.

Yeah, interlaced requires half the bandwidth of progressive, so it
should then fit comfortably within the 165MHz limit of the adaptor.

> 
> > > By the way: Is it possible to disable HDMI 12bpc in a way that
> > > doesn't require me to patch and rebuild the kernel drivers, such as
> > > a kernel module parameter or sysfs setting? (I prefer to simply use
> > > the upstream Fedora kernel RPMs, but this issue is currently making
> > > that impossible.)  
> > 
> > We don't have any knob to control this.
> 
> I don't need it anymore, so no worries. ;-)
> 
> Tore

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

[Intel-gfx] [PATCH v2 2/6] drm/atomic: Pass connector and state to update_connector_routing.

[Maarten Lankhorst]

Minor cleanup, connector and connector_state are always non-NULL here.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic_helper.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index a0226d4b949a..3d1f97a832fc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -215,22 +215,16 @@ steal_encoder(struct drm_atomic_state *state,
 }
 
 static int
-update_connector_routing(struct drm_atomic_state *state, int conn_idx)
+update_connector_routing(struct drm_atomic_state *state,
+struct drm_connector *connector,
+struct drm_connector_state *connector_state)
 {
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
struct drm_crtc *encoder_crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *connector_state;
struct drm_crtc_state *crtc_state;
int idx, ret;
 
-   connector = state->connectors[conn_idx];
-   connector_state = state->connector_states[conn_idx];
-
-   if (!connector)
-   return 0;
-
DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
 connector->base.id,
 connector->name);
@@ -494,7 +488,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 * drivers must set crtc->mode_changed themselves when connector
 * properties need to be updated.
 */
-   ret = update_connector_routing(state, i);
+   ret = update_connector_routing(state, connector,
+  connector_state);
if (ret)
return ret;
}
-- 
2.1.0

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

Re: [Intel-gfx] [RFCv2 04/14] drm/i915: factor out alloc_context_idr() and __i915_gem_create_context()

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> 
> For flexible GEM context creation, we factor out __i915_gem_create_context
> as the core logic of creation a GEM context. After the refactor, it more
> likesa context creation servcie, which is able to create context by
> explicit requirement of upper level components, not by the assumptions of
> incoming parameters.
> 
> For the assumptions in original implementation, we keep them in the upper
> level wrapper: i915_gem_create_context().

IMO combining 5/6/7 is clearer to give a full picture how create_context
is refactored, but will leave to i915 guys to comment the preferred style.

> 
> alloc_context_idr() is another function factored out to setup a IDR for
> ordinary GEM context. Some context, e.g. GVT context, maybe more than one
> kernel context in furture (currently there is only one kernel context: the
> default context) doesn't need a IDR. So we make it an option in context
> creation.
> 

Could you elaborate why IDR cannot be used for GVT context. Even
if it is not used, keeping it can reduce the code divergence as long as
no function is impacted...

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

Re: [Intel-gfx] [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context requirement

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> 
> This patchset is used to discuss and finalize the i915 changes required by
> GVT context. Previously we have discussed about how to hack i915 to meet
> GVT context requirement, and thanks for the idea and comments.
> 
> In this patchset, mostly it refactors the existing i915 APIs, spliting the
> hard-coded assumptions from its core logic, keep these assumptions in the
> high level wrapper and make the core logic much more flexible and config-
> urable, which is able to be used by GVT context creation and submission.

It would be good to note that this patch series is not the only change required
for GVT context management. It addresses creation/submission. We also 
need some specific change in context switch time. Do you want to include them
together in next version to compose a full picture?

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

Re: [Intel-gfx] [PATCH] drm/i915: Update state before setting watermarks.

[Tvrtko Ursulin]

On 24/02/16 13:54, Maarten Lankhorst wrote:

When intel_update_watermarks is called on skylake it inspects
crtc->state, which should show as disabled. This wasn't the case,
and this resulted in a divide-by-zero in
skl_ddb_get_pipe_allocation_limits when intel_update_watermarks was called.

  [ cut here ]
  WARNING: CPU: 1 PID: 295 at drivers/gpu/drm/i915/intel_pm.c:2834 
skl_update_pipe_wm+0x102/0x8c0 [i915]()
  WARN_ON(!config->num_pipes_active)
  Modules linked in: coretemp i915(+) 
x
  CPU: 1 PID: 295 Comm: systemd-udevd Tainted: G U  W   
4.5.0-rc4-xx #25
  Hardware name: x
    88003777f5a8 813485c2 88003777f5f0
   a0236240 88003777f5e0 81050fce 8800aa42
   8800aba18000 8800aba18000 880037304c00 8800aa42
  Call Trace:
   [] dump_stack+0x67/0x95
   [] warn_slowpath_common+0x9e/0xc0
   [] warn_slowpath_fmt+0x4c/0x50
   [] ? flush_work+0x8e/0x280
   [] ? flush_work+0x5/0x280
   [] skl_update_pipe_wm+0x102/0x8c0 [i915]
   [] skl_update_wm+0xff/0x5f0 [i915]
   [] ? trace_hardirqs_on_caller+0x15e/0x1d0
   [] ? trace_hardirqs_on+0xd/0x10
   [] intel_update_watermarks+0x1e/0x30 [i915]
   [] intel_crtc_disable_noatomic+0xd2/0x150 [i915]
   [] intel_modeset_setup_hw_state+0xdd2/0xde0 [i915]
   [] intel_modeset_init+0x15a3/0x1950 [i915]
   [] i915_driver_load+0x13c6/0x1720 [i915]
   [] ? add_sysfs_fw_map_entry+0x9b/0x9b
   [] drm_dev_register+0x6f/0xb0 [drm]
   [] drm_get_pci_dev+0x10a/0x1d0 [drm]
   [] i915_pci_probe+0x49/0x50 [i915]
   [] pci_device_probe+0x80/0xf0
   [] driver_probe_device+0x1bc/0x3d0
   [] __driver_attach+0x66/0x90
   [] ? driver_probe_device+0x3d0/0x3d0
   [] bus_for_each_dev+0x5b/0xa0
   [] driver_attach+0x1e/0x20
   [] bus_add_driver+0x151/0x270
   [] driver_register+0x8c/0xd0
   [] __pci_register_driver+0x5d/0x60
   [] drm_pci_init+0x58/0xf0 [drm]
   [] ? trace_hardirqs_on+0xd/0x10
   [] ? 0xa02aa000
   [] i915_init+0x94/0x9b [i915]
   [] do_one_initcall+0x113/0x1f0
   [] ? rcu_read_lock_sched_held+0x61/0x90
   [] ? kmem_cache_alloc_trace+0x1cc/0x280
   [] do_init_module+0x60/0x1c8
   [] load_module+0x1ceb/0x2410
   [] ? store_uevent+0x40/0x40
   [] ? kernel_read+0x41/0x60
   [] SYSC_finit_module+0x8d/0xa0
   [] SyS_finit_module+0xe/0x10
   [] entry_SYSCALL_64_fastpath+0x12/0x6f
  ---[ end trace 1149e9ab3695a423 ]---
  [ cut here ]

Reported-by: Tvrtko Ursulin 
Signed-off-by: Maarten Lankhorst 


Thanks for looking into this! Also,

Tested-by: Tvrtko Ursulin 

Regards,

Tvrtko


---
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index deee56010eee..d9e0470419a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6358,6 +6358,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)

  static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
  {
+   struct intel_encoder *encoder;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
enum intel_display_power_domain domain;
@@ -6378,6 +6379,19 @@ static void intel_crtc_disable_noatomic(struct drm_crtc 
*crtc)
dev_priv->display.crtc_disable(crtc);
intel_crtc->active = false;
intel_fbc_disable(intel_crtc);
+
+   DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was enabled, now 
disabled\n",
+ crtc->base.id);
+
+   WARN_ON(drm_atomic_set_mode_for_crtc(crtc->state, NULL) < 0);
+   crtc->state->active = false;
+   crtc->enabled = false;
+   crtc->state->connector_mask = 0;
+   crtc->state->encoder_mask = 0;
+
+   for_each_encoder_on_crtc(crtc->dev, crtc, encoder)
+   encoder->base.crtc = NULL;
+
intel_update_watermarks(crtc);
intel_disable_shared_dpll(intel_crtc);

@@ -15526,38 +15540,9 @@ static void intel_sanitize_crtc(struct intel_crtc 
*crtc)

/* Adjust the state of the output pipe according to whether we
 * have active connectors/encoders. */
-   if (!intel_crtc_has_encoders(crtc))
+   if (crtc->active && !intel_crtc_has_encoders(crtc))
intel_crtc_disable_noatomic(>base);

-   if (crtc->active != crtc->base.state->active) {
-   struct intel_encoder *encoder;
-
-   /* This can happen either due to bugs in the get_hw_state
-* functions or because of calls to intel_crtc_disable_noatomic,
-* or because the pipe is force-enabled due to the
-* pipe A quirk. */
-   DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n",
- crtc->base.base.id,
- 

[Intel-gfx] [PATCH] drm/i915: Fix bogus dig_port_map[] assignment for pre-HSW

[Takashi Iwai]

The recent commit [0bdf5a05647a: drm/i915: Add reverse mapping between
port and intel_encoder] introduced a reverse mapping to retrieve
intel_dig_port object from the port number.  The code assumed that the
port vs intel_dig_port are 1:1 mapping.  But in reality, this was a
too naive assumption.

As Martin reported about the missing HDMI audio on his SNB machine,
pre-HSW chips may have multiple intel_dig_port objects corresponding
to the same port.  Since we assign the mapping statically at the init
time and the multiple objects override the map, it may not match with
the actually enabled output.

This patch tries to address the regression above.  The reverse mapping
is provided basically only for the audio callbacks, so now we set /
clear the mapping dynamically at enabling and disabling HDMI/DP audio,
so that we can always track the latest and correct object
corresponding to the given port.

Fixes: 0bdf5a05647a ('drm/i915: Add reverse mapping between port and 
intel_encoder')
Reported-and-tested-by: Martin Kepplinger 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/i915/intel_audio.c | 3 +++
 drivers/gpu/drm/i915/intel_ddi.c   | 1 -
 drivers/gpu/drm/i915/intel_dp.c| 1 -
 drivers/gpu/drm/i915/intel_hdmi.c  | 2 --
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 31f6d212fb1b..30f921421b0c 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -527,6 +527,8 @@ void intel_audio_codec_enable(struct intel_encoder 
*intel_encoder)
 
mutex_lock(_priv->av_mutex);
intel_dig_port->audio_connector = connector;
+   /* referred in audio callbacks */
+   dev_priv->dig_port_map[port] = intel_encoder;
mutex_unlock(_priv->av_mutex);
 
if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
@@ -554,6 +556,7 @@ void intel_audio_codec_disable(struct intel_encoder 
*intel_encoder)
 
mutex_lock(_priv->av_mutex);
intel_dig_port->audio_connector = NULL;
+   dev_priv->dig_port_map[port] = NULL;
mutex_unlock(_priv->av_mutex);
 
if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 54a165b9c92d..a50fc452d5f1 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3312,7 +3312,6 @@ void intel_ddi_init(struct drm_device *dev, enum port 
port)
intel_encoder->get_config = intel_ddi_get_config;
 
intel_dig_port->port = port;
-   dev_priv->dig_port_map[port] = intel_encoder;
intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
  (DDI_BUF_PORT_REVERSAL |
   DDI_A_4_LANES);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1bbd67b046da..acf918728492 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6035,7 +6035,6 @@ intel_dp_init(struct drm_device *dev,
}
 
intel_dig_port->port = port;
-   dev_priv->dig_port_map[port] = intel_encoder;
intel_dig_port->dp.output_reg = output_reg;
 
intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 4a77639a489d..23ee48dc765f 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2146,7 +2146,6 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
 void intel_hdmi_init(struct drm_device *dev,
 i915_reg_t hdmi_reg, enum port port)
 {
-   struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_digital_port *intel_dig_port;
struct intel_encoder *intel_encoder;
struct intel_connector *intel_connector;
@@ -2215,7 +2214,6 @@ void intel_hdmi_init(struct drm_device *dev,
intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
 
intel_dig_port->port = port;
-   dev_priv->dig_port_map[port] = intel_encoder;
intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 
-- 
2.7.2

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

[Intel-gfx] [PATCH v2 5/6] drm/atomic: Handle encoder assignment conflicts in a separate check.

[Maarten Lankhorst]

The current check doesn't handle the case where we don't steal an
encoder, but keep it on the current connector. If we repurpose
disable_conflicting_encoders to do the checking, we just have
to reject the ones that conflict.

Signed-off-by: Maarten Lankhorst 
Testcase: kms_setmode.invalid-clone-single-crtc-stealing
---
 drivers/gpu/drm/drm_atomic_helper.c | 58 +++--
 1 file changed, 24 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 3543c7fcd072..32bd5bebef0b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -86,7 +86,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
}
 }
 
-static int disable_conflicting_connectors(struct drm_atomic_state *state)
+static int handle_conflicting_encoders(struct drm_atomic_state *state,
+  bool disable_conflicting_encoders)
 {
struct drm_connector_state *conn_state;
struct drm_connector *connector;
@@ -106,8 +107,17 @@ static int disable_conflicting_connectors(struct 
drm_atomic_state *state)
else
new_encoder = funcs->best_encoder(connector);
 
-   if (new_encoder)
+   if (new_encoder) {
+   if (encoder_mask & (1 << 
drm_encoder_index(new_encoder))) {
+   DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on 
[CONNECTOR:%d:%s] already assigned\n",
+   new_encoder->base.id, new_encoder->name,
+   connector->base.id, connector->name);
+
+   return -EINVAL;
+   }
+
encoder_mask |= 1 << drm_encoder_index(new_encoder);
+   }
}
 
drm_for_each_connector(connector, state->dev) {
@@ -120,6 +130,15 @@ static int disable_conflicting_connectors(struct 
drm_atomic_state *state)
if (!encoder || !(encoder_mask & (1 << 
drm_encoder_index(encoder
continue;
 
+   if (!disable_conflicting_encoders) {
+   DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on 
[CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
+encoder->base.id, encoder->name,
+connector->state->crtc->base.id,
+connector->state->crtc->name,
+connector->base.id, connector->name);
+   return -EINVAL;
+   }
+
conn_state = drm_atomic_get_connector_state(state, connector);
if (IS_ERR(conn_state))
return PTR_ERR(conn_state);
@@ -148,26 +167,6 @@ static int disable_conflicting_connectors(struct 
drm_atomic_state *state)
return 0;
 }
 
-static bool
-check_pending_encoder_assignment(struct drm_atomic_state *state,
-struct drm_encoder *new_encoder)
-{
-   struct drm_connector *connector;
-   struct drm_connector_state *conn_state;
-   int i;
-
-   for_each_connector_in_state(state, connector, conn_state, i) {
-   if (conn_state->best_encoder != new_encoder)
-   continue;
-
-   /* encoder already assigned and we're trying to re-steal it! */
-   if (connector->state->best_encoder != conn_state->best_encoder)
-   return false;
-   }
-
-   return true;
-}
-
 static void
 set_best_encoder(struct drm_atomic_state *state,
 struct drm_connector_state *conn_state,
@@ -326,13 +325,6 @@ update_connector_routing(struct drm_atomic_state *state,
return 0;
}
 
-   if (!check_pending_encoder_assignment(state, new_encoder)) {
-   DRM_DEBUG_ATOMIC("Encoder for [CONNECTOR:%d:%s] already 
assigned\n",
-connector->base.id,
-connector->name);
-   return -EINVAL;
-   }
-
ret = steal_encoder(state, new_encoder);
if (ret) {
DRM_DEBUG_ATOMIC("Encoder stealing failed for 
[CONNECTOR:%d:%s]\n",
@@ -511,11 +503,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
}
}
 
-   if (state->legacy_set_config) {
-   ret = disable_conflicting_connectors(state);
-   if (ret)
-   return ret;
-   }
+   ret = handle_conflicting_encoders(state, state->legacy_set_config);
+   if (ret)
+   return ret;
 
for_each_connector_in_state(state, connector, connector_state, i) {
/*
-- 
2.1.0

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

[Intel-gfx] [PATCH v2 6/6] drm/atomic: Clean up steal_encoder

[Maarten Lankhorst]

Now that only encoders can be stolen that are part of the state
steal_encoder no longer needs to inspect all connectors,
just those that are part of the atomic state.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 32bd5bebef0b..0fc56339001d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -215,18 +215,11 @@ steal_encoder(struct drm_atomic_state *state,
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
+   int i;
 
-   drm_for_each_connector(connector, state->dev) {
+   for_each_connector_in_state(state, connector, connector_state, i) {
struct drm_crtc *encoder_crtc;
 
-   if (connector->state->best_encoder != encoder)
-   continue;
-
-   connector_state = drm_atomic_get_connector_state(state,
-connector);
-   if (IS_ERR(connector_state))
-   return PTR_ERR(connector_state);
-
if (connector_state->best_encoder != encoder ||
WARN_ON(!connector_state->crtc))
continue;
-- 
2.1.0

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

[Intel-gfx] [PATCH v2 0/6] drm/atomic: Fix encoder stealing, v2.

[Maarten Lankhorst]

After trying out various ways to handle encoder stealing better
I came up with a cleaner way.

The first patch cleans up update_output_state and only adds affected connectors.
This is required to determine which connectors are not part of the atomic state,
and allow disabling them if they have conflicting encoders.

After that 2 cleanups, then a fix to make encoder stealing explicit,
followed by using the newly created function to prevent encoder duplication
and finally another cleanup.

Maarten Lankhorst (6):
  drm/atomic: Clean up update_output_state.
  drm/atomic: Pass connector and state to update_connector_routing.
  drm/atomic: Always call steal_encoder.
  drm/atomic: Handle encoder stealing from set_config better.
  drm/atomic: Handle encoder assignment conflicts in a separate check.
  drm/atomic: Clean up steal_encoder

 drivers/gpu/drm/drm_atomic_helper.c | 215 +++-
 include/drm/drm_crtc.h  |   2 +
 2 files changed, 113 insertions(+), 104 deletions(-)

-- 
2.1.0

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

[Intel-gfx] [PATCH v2 1/6] drm/atomic: Clean up update_output_state.

[Maarten Lankhorst]

With the addition of crtc_state->connector_mask other connectors from
different crtc's aren't needed any more to determine if a crtc has
connectors, so only call add_affected_connectors on the target crtc.
This allows a cleanup to first remove all current connectors, then
add all set->connectors to the target crtc.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic_helper.c | 41 +++--
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 4da4f2a49078..a0226d4b949a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1761,28 +1761,18 @@ static int update_output_state(struct drm_atomic_state 
*state,
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *conn_state;
-   int ret, i, j;
+   int ret, i;
 
ret = drm_modeset_lock(>mode_config.connection_mutex,
   state->acquire_ctx);
if (ret)
return ret;
 
-   /* First grab all affected connector/crtc states. */
-   for (i = 0; i < set->num_connectors; i++) {
-   conn_state = drm_atomic_get_connector_state(state,
-   set->connectors[i]);
-   if (IS_ERR(conn_state))
-   return PTR_ERR(conn_state);
-   }
-
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   return ret;
-   }
+   /* First disable all connectors on the target crtc. */
+   ret = drm_atomic_add_affected_connectors(state, set->crtc);
+   if (ret)
+   return ret;
 
-   /* Then recompute connector->crtc links and crtc enabling state. */
for_each_connector_in_state(state, connector, conn_state, i) {
if (conn_state->crtc == set->crtc) {
ret = drm_atomic_set_crtc_for_connector(conn_state,
@@ -1790,16 +1780,19 @@ static int update_output_state(struct drm_atomic_state 
*state,
if (ret)
return ret;
}
+   }
 
-   for (j = 0; j < set->num_connectors; j++) {
-   if (set->connectors[j] == connector) {
-   ret = 
drm_atomic_set_crtc_for_connector(conn_state,
-   
set->crtc);
-   if (ret)
-   return ret;
-   break;
-   }
-   }
+   /* Then set all connectors from set->connectors on the target crtc */
+   for (i = 0; i < set->num_connectors; i++) {
+   conn_state = drm_atomic_get_connector_state(state,
+   set->connectors[i]);
+   if (IS_ERR(conn_state))
+   return PTR_ERR(conn_state);
+
+   ret = drm_atomic_set_crtc_for_connector(conn_state,
+   set->crtc);
+   if (ret)
+   return ret;
}
 
for_each_crtc_in_state(state, crtc, crtc_state, i) {
-- 
2.1.0

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

Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

[Takashi Iwai]

On Tue, 23 Feb 2016 20:09:02 +0100,
Martin Kepplinger wrote:
> 
> Am 2016-02-23 um 18:14 schrieb Ville Syrjälä:
> > On Tue, Feb 23, 2016 at 05:57:40PM +0100, Takashi Iwai wrote:
> >> On Mon, 22 Feb 2016 22:37:28 +0100,
> >> Martin Kepplinger wrote:
> >>>
> >>> Am 2016-02-22 um 20:10 schrieb Takashi Iwai:
>  On Mon, 22 Feb 2016 19:58:18 +0100,
>  Martin Kepplinger wrote:
> >
> > Am 2016-02-22 um 15:12 schrieb Takashi Iwai:
> >> On Mon, 22 Feb 2016 15:02:56 +0100,
> >> Martin Kepplinger wrote:
>  And how about my questions in the previous mail?  Does
>  i915_audio_component_get_eld() is called and returns 0?
>  And is monitor_present set true or false?
> >>>
> >>> i915_audio_component_get_eld() returns 0 and monitor_present is 0.
> 
>  If i915_audio_component_get_eld() is called but returns zero, track
>  the code flow there.  It means either intel_encoder is NULL or
>  intel_dig_port->audio_connector is NULL.
> >>>
> >>> intel_dig_port->audio_connector is NULL!
> >>>
> >>> (when called during boot and during HDMI plugin)
> >>
> >> Interesting.  The relevant code flow should be like:
> >>
> >>   intel_audio_codec_enable()
> >>   -> acomp->audio_ops->pin_eld_notify()
> >> -> intel_pin_eld_notify()
> >>   -> check_presence_and_report()
> >> -> hdmi_present_sense()
> >>  -> sync_eld_via_acomp()
> >>-> snd_hdac_acomp_get_eld()
> >>  -> i915_audio_component_get_eld()
> >>
> >> So, at first, check whether intel_dig_port in both
> >> intel_audio_codec_enable() and i915_audio_component_get_eld() points
> >> to the same object address.  The audio_connector must be set in
> >> intel_audio_codec_enable(), thus basically it must be non-NULL at
> >> i915_audio_component_get_eld(), too.
> >>
> >
> > intel_dig_port is *not* the same object in these 2 places. During
> > plugin, see:
> >
> > [  146.934091] in intel_audio_codec_enable: intel_dig_port is
> > 8800a1f54000
> > [  146.934121] in i915_audio_component_get_eld: intel_dig_port is
> > 880244f7d000
> >
> > sorry for the slow responses. I'll try to go back that direction unless
> > again someone comes up with other suggestions.
> 
>  Thanks, this makes sense.  It implies that the digital port mapping is
>  somehow wrong.  There are three places setting dig_port_map[], one in
>  intel_ddi_init(), one in intel_dp_init() and another in
>  intel_hdmi_init().  Try to check which function creates which object
>  assigned to which port number, together with drm.debug=0x0e debug
>  messages.
> 
> >>> without using drm.debug=0x0e, but by printing the kmalloc'ed objects in
> >>> those 3 functions with ports, I found:
> >>>
> >>> 2 of them are running, only during boot:
> >>>
> >>> [2.322865] intel_hdmi_init: intel_dig_port is 880242564000  port 1
> >>> [2.322999] intel_dp_init: intel_dig_port is 880242f3  port 1
> >>>
> >>> is is correct for them to have both port 1? Any more ideas?
> >>
> >> Adding intel-gfx ML to Cc.
> >>
> >> Martin, is the machine SandyBridge or IvyBridge?
> >>
> >> In anyway it's PCH_SPLIT and there can call both intel_hdmi_init() and 
> >> intel_dp_init() for the same port although both functions map
> >> intel_dig_port[].  The assumption of intel_dig_port[] reverse mapping
> >> is that there is only a single intel_dig_port assigned to a port, but
> >> this doesn't look correct...
> > 
> > On pre-HSW there can indeed be two encoders for the same port.
> > And I'm planning to change HSW+ to follow that model as well,
> > but I've been busy with other stuff to finish off that work [1]
> > 
> > [1] 
> > https://lists.freedesktop.org/archives/intel-gfx/2015-December/082384.html
> > 
> 
> So your work wouldn't fix hdmi-audio pre-HSW here?

The only question is how to pass intel_dig_port.  The current code is
a kind of optimization suggested after my initial patch.

Since dig_port_map[] is used only for the audio callback, we can
assign it dynamically just before the callbacks.

Could you try the patch below?  (It's totally untested.)

> Anyways, a quick fix would be good, and if not that, remember marking
> future changes that fix this, for stable.
> 
> What implications would something like the following, that Takashi
> suggested, have on other systems?

We'd take that action as a last resort, but it should be limited to
pre-HSW.  So if any, we'd need the check of codec ID.


thanks,

Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 31f6d212fb1b..e2bee6957cc0 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -521,6 +521,9 @@ void intel_audio_codec_enable(struct intel_encoder 
*intel_encoder)
 
connector->eld[6] = 

Re: [Intel-gfx] [RFCv2 10/14] drm/i915: update PDPs by condition when submit the LRC context

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> 
> Previously the PDPs inside the ring context are updated at:
> 
> - When populate a LRC context
> - Before submitting a LRC context (only for 32 bit PPGTT, as the amount
> of used PDPs may change)
> 
> This patch postpones the PDPs upgrade to submission time, and will update
> it by condition if the PPGTT is 48b. Under GVT-g, one GVT context will be
> used by different guest, the PPGTT instance related to the context might
> be changed before the submission time. And this patch gives GVT context
> a chance to load the new PPGTT instance into an initialized context.

Could you elaborate why we share one GVT context across different guest?
A natural thought is that we'll create one GVT context per every guest
context...

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

Re: [Intel-gfx] [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context requirement

[Wang, Zhi A]

Hi Kevin:
Now our context switch is covered by context status change notification 
handler. In the status change handler we will save render registers. As GVT 
context will be a single submission context we will restore the render 
registers when the GVT context is scheduled-out by HW.

> -Original Message-
> From: Tian, Kevin
> Sent: Wednesday, February 24, 2016 4:56 PM
> To: Wang, Zhi A; intel-gfx@lists.freedesktop.org; igv...@lists.01.org
> Cc: Lv, Zhiyuan; Niu, Bing; Song, Jike; daniel.vet...@ffwll.ch; Cowperthwaite,
> David J; ch...@chris-wilson.co.uk; joonas.lahti...@linux.intel.com
> Subject: RE: [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context
> requirement
> 
> > From: Wang, Zhi A
> > Sent: Thursday, February 18, 2016 7:42 PM
> >
> > This patchset is used to discuss and finalize the i915 changes
> > required by GVT context. Previously we have discussed about how to
> > hack i915 to meet GVT context requirement, and thanks for the idea and
> comments.
> >
> > In this patchset, mostly it refactors the existing i915 APIs, spliting
> > the hard-coded assumptions from its core logic, keep these assumptions
> > in the high level wrapper and make the core logic much more flexible
> > and config- urable, which is able to be used by GVT context creation and
> submission.
> 
> It would be good to note that this patch series is not the only change 
> required
> for GVT context management. It addresses creation/submission. We also need
> some specific change in context switch time. Do you want to include them
> together in next version to compose a full picture?
> 
> Thanks
> Kevin
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915/dsi: Added the generic gpio sequence support and gpio table

[Deepak M]

The generic gpio is sequence is parsed from the VBT and the
GPIO table is updated with the North core, South core and
SUS core elements.

v2: Move changes in sideband.c file to new patch(Jani), rebase
v3: Moved the Macro`s to intel_dsi_panel_vbt.c (Jani)

v3 by Jani
- rebase on previous patches
- don't return null on errors

v4 by Deepak
- rebase
- prefixed the VLV_ to all the GPIO macros

v5 by deepak
- readded the checks which were removed in the
  earlier patchset (Jani)

Cc: Ville Syrjälä 
Signed-off-by: Deepak M 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_reg.h|   6 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 592 ++---
 2 files changed, 555 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3774870..606dc71 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -620,10 +620,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   IOSF_PORT_FLISDSI0x1b
 #define   IOSF_PORT_GPIO_SC0x48
 #define   IOSF_PORT_GPIO_SUS   0xa8
+#define   IOSF_MAX_GPIO_NUM_NC 26
+#define   IOSF_MAX_GPIO_NUM_SC 128
+#define   IOSF_MAX_GPIO_NUM172
 #define   IOSF_PORT_CCU0xa9
 #define VLV_IOSF_DATA  _MMIO(VLV_DISPLAY_BASE + 0x2104)
 #define VLV_IOSF_ADDR  _MMIO(VLV_DISPLAY_BASE + 0x2108)
 
+#define VLV_GPIO_CFG   0x2000CC00
+#define VLV_GPIO_INPUT_DIS 0x04
+
 /* See configdb bunit SB addr map */
 #define BUNIT_REG_BISOC0x11
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 787f01c..794bd1f 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -58,30 +58,356 @@ static inline struct vbt_panel *to_vbt_panel(struct 
drm_panel *panel)
 
 #define NS_KHZ_RATIO 100
 
-#define GPI0_NC_0_HV_DDI0_HPD   0x4130
-#define GPIO_NC_0_HV_DDI0_PAD   0x4138
-#define GPIO_NC_1_HV_DDI0_DDC_SDA   0x4120
-#define GPIO_NC_1_HV_DDI0_DDC_SDA_PAD   0x4128
-#define GPIO_NC_2_HV_DDI0_DDC_SCL   0x4110
-#define GPIO_NC_2_HV_DDI0_DDC_SCL_PAD   0x4118
-#define GPIO_NC_3_PANEL0_VDDEN  0x4140
-#define GPIO_NC_3_PANEL0_VDDEN_PAD  0x4148
-#define GPIO_NC_4_PANEL0_BLKEN  0x4150
-#define GPIO_NC_4_PANEL0_BLKEN_PAD  0x4158
-#define GPIO_NC_5_PANEL0_BLKCTL 0x4160
-#define GPIO_NC_5_PANEL0_BLKCTL_PAD 0x4168
-#define GPIO_NC_6_PCONF00x4180
-#define GPIO_NC_6_PAD   0x4188
-#define GPIO_NC_7_PCONF00x4190
-#define GPIO_NC_7_PAD   0x4198
-#define GPIO_NC_8_PCONF00x4170
-#define GPIO_NC_8_PAD   0x4178
-#define GPIO_NC_9_PCONF00x4100
-#define GPIO_NC_9_PAD   0x4108
-#define GPIO_NC_10_PCONF0   0x40E0
-#define GPIO_NC_10_PAD  0x40E8
-#define GPIO_NC_11_PCONF0   0x40F0
-#define GPIO_NC_11_PAD  0x40F8
+#define VLV_HV_DDI0_HPD_GPIONC_0_PCONF0 0x4130
+#define VLV_HV_DDI0_HPD_GPIONC_0_PAD0x4138
+#define VLV_HV_DDI0_DDC_SDA_GPIONC_1_PCONF0 0x4120
+#define VLV_HV_DDI0_DDC_SDA_GPIONC_1_PAD0x4128
+#define VLV_HV_DDI0_DDC_SCL_GPIONC_2_PCONF0 0x4110
+#define VLV_HV_DDI0_DDC_SCL_GPIONC_2_PAD0x4118
+#define VLV_PANEL0_VDDEN_GPIONC_3_PCONF00x4140
+#define VLV_PANEL0_VDDEN_GPIONC_3_PAD   0x4148
+#define VLV_PANEL0_BKLTEN_GPIONC_4_PCONF0   0x4150
+#define VLV_PANEL0_BKLTEN_GPIONC_4_PAD  0x4158
+#define VLV_PANEL0_BKLTCTL_GPIONC_5_PCONF0  0x4160
+#define VLV_PANEL0_BKLTCTL_GPIONC_5_PAD 0x4168
+#define VLV_HV_DDI1_HPD_GPIONC_6_PCONF0 0x4180
+#define VLV_HV_DDI1_HPD_GPIONC_6_PAD0x4188
+#define VLV_HV_DDI1_DDC_SDA_GPIONC_7_PCONF0 0x4190
+#define VLV_HV_DDI1_DDC_SDA_GPIONC_7_PAD0x4198
+#define VLV_HV_DDI1_DDC_SCL_GPIONC_8_PCONF0 0x4170
+#define VLV_HV_DDI1_DDC_SCL_GPIONC_8_PAD0x4178
+#define VLV_PANEL1_VDDEN_GPIONC_9_PCONF00x4100
+#define VLV_PANEL1_VDDEN_GPIONC_9_PAD   0x4108
+#define VLV_PANEL1_BKLTEN_GPIONC_10_PCONF0  0x40E0
+#define VLV_PANEL1_BKLTEN_GPIONC_10_PAD 0x40E8
+#define VLV_PANEL1_BKLTCTL_GPIONC_11_PCONF0 0x40F0
+#define VLV_PANEL1_BKLTCTL_GPIONC_11_PAD0x40F8
+#define VLV_GP_INTD_DSI_TE1_GPIONC_12_PCONF00x40C0
+#define VLV_GP_INTD_DSI_TE1_GPIONC_12_PAD   0x40C8
+#define VLV_HV_DDI2_DDC_SDA_GPIONC_13_PCONF00x41A0
+#define 

Re: [Intel-gfx] [PATCH] drm/i915: Add two-stage ILK-style watermark programming (v11)

[Maarten Lankhorst]

Op 24-02-16 om 02:24 schreef Matt Roper:
> On Tue, Feb 23, 2016 at 05:20:13PM -0800, Matt Roper wrote:
>> In addition to calculating final watermarks, let's also pre-calculate a
>> set of intermediate watermark values at atomic check time.  These
>> intermediate watermarks are a combination of the watermarks for the old
>> state and the new state; they should satisfy the requirements of both
>> states which means they can be programmed immediately when we commit the
>> atomic state (without waiting for a vblank).  Once the vblank does
>> happen, we can then re-program watermarks to the more optimal final
>> value.
>>
>> v2: Significant rebasing/rewriting.
>>
>> v3:
>>  - Move 'need_postvbl_update' flag to CRTC state (Daniel)
>>  - Don't forget to check intermediate watermark values for validity
>>(Maarten)
>>  - Don't due async watermark optimization; just do it at the end of the
>>atomic transaction, after waiting for vblanks.  We do want it to be
>>async eventually, but adding that now will cause more trouble for
>>Maarten's in-progress work.  (Maarten)
>>  - Don't allocate space in crtc_state for intermediate watermarks on
>>platforms that don't need it (gen9+).
>>  - Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit
>>now that ilk_update_wm is gone.
>>
>> v4:
>>  - Add a wm_mutex to cover updates to intel_crtc->active and the
>>need_postvbl_update flag.  Since we don't have async yet it isn't
>>terribly important yet, but might as well add it now.
>>  - Change interface to program watermarks.  Platforms will now expose
>>.initial_watermarks() and .optimize_watermarks() functions to do
>>watermark programming.  These should lock wm_mutex, copy the
>>appropriate state values into intel_crtc->active, and then call
>>the internal program watermarks function.
>>
>> v5:
>>  - Skip intermediate watermark calculation/check during initial hardware
>>readout since we don't trust the existing HW values (and don't have
>>valid values of our own yet).
>>  - Don't try to call .optimize_watermarks() on platforms that don't have
>>atomic watermarks yet.  (Maarten)
>>
>> v6:
>>  - Rebase
>>
>> v7:
>>  - Further rebase
>>
>> v8:
>>  - A few minor indentation and line length fixes
>>
>> v9:
>>  - Yet another rebase since Maarten's patches reworked a bunch of the
>>code (wm_pre, wm_post, etc.) that this was previously based on.
>>
>> v10:
>>  - Move wm_mutex to dev_priv to protect against racing commits against
>>disjoint CRTC sets. (Maarten)
>>  - Drop unnecessary clearing of cstate->wm.need_postvbl_update (Maarten)
>>
>> v11:
>>  - Now that we've moved to atomic watermark updates, make sure we call
>>the proper function to program watermarks in
>>{ironlake,haswell}_crtc_enable(); the failure to do so on the
>>previous patch iteration led to us not actually programming the
>>watermarks before turning on the CRTC, which was the cause of the
>>underruns that the CI system was seeing.
>>  - Fix inverted logic for determining when to optimize watermarks.  We
>>were needlessly optimizing when the intermediate/optimal values were
>>the same (harmless), but not actually optimizing when they differed
>>(also harmless, but wasteful from a power/bandwidth perspective).
>>
>> Cc: Maarten Lankhorst 
>> Signed-off-by: Matt Roper 
>> ---
> To assist with review, the non-rebasing changes in this iteration vs the
> last one are:
>
>> @@ -4925,7 +4960,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>>   */
>>  intel_crtc_load_lut(crtc);
>>  
>> -intel_update_watermarks(crtc);
>> +dev_priv->display.initial_watermarks(intel_crtc->config);
>>  intel_enable_pipe(intel_crtc);
>>  
>>  if (intel_crtc->config->has_pch_encoder)
>> @@ -5024,7 +5059,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>>  if (!intel_crtc->config->has_dsi_encoder)
>>  intel_ddi_enable_transcoder_func(crtc);
>>  
>> -intel_update_watermarks(crtc);
>> +dev_priv->display.initial_watermarks(pipe_config);
>>  intel_enable_pipe(intel_crtc);
>>  
>>  if (intel_crtc->config->has_pch_encoder)
Are the intermediate watermarks identical to optimal watermarks during modeset?
> (both new additions to the patch)
>
> and:
>
>> +/*
>> + * If our intermediate WM are identical to the final WM, then we can
>> + * omit the post-vblank programming; only update if it's different.
>> + */
>> +if (memcmp(a, >wm.optimal.ilk, sizeof(*a)) == 0)
>> +newstate->wm.need_postvbl_update = false;
> (replacement of a "!=" with "==")
Ah, good catch!

Lets just wait for CI before applying again
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFCv2 03/14] drm/i915: Introduce host graphics memory/fence partition for GVT-g

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Tuesday, February 23, 2016 9:23 PM
> >> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> >> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> >> @@ -348,6 +348,10 @@ void *gvt_create_pgt_device(struct drm_i915_private
> *dev_priv)
> >>goto err;
> >>}
> >>
> >> +  dev_priv->gvt.host_fence_sz = gvt.host_fence_sz;
> >> +  dev_priv->gvt.host_low_gm_sz_in_mb = gvt.host_low_gm_sz;
> >> +  dev_priv->gvt.host_high_gm_sz_in_mb = gvt.host_high_gm_sz;
> >
> > I'm thinking, could we expose the pgt_device struct (at least
> > partially, and then have a PIMPL pattern), to avoid this kind of
> > duplication of declarations and unnecessary copies between i915 and
> > i915_gvt modules?
> >
> > It's little rough that the gvt driver writes to i915_private struct.
> > I'd rather see that gvt.host_fence_sz and other variables get sanitized
> > and then written to pgt_device (maybe the public part would be
> > i915_pgt_device) and used by gvt and i915 code.
> >
> > Was this ever considered?
> >
> The previous version do something similar like that, both i915 and gvt
> reads GVT module kernel parameter but considered that GVT modules could
> be configured as "n" in kernel configuration, probably we should let
> i915 to store this information and GVT to configure it if GVT is active?

Agree with Joonas. We don't need another gvt wrap. Let's just expose
pgt_device directly. I believe all other information can be encapsulated
under pgt_device.

btw to match other description in the code, is it clear to rename pgt_device
to gvt_device?

Another minor thing needs Joonas' feedback. Is it usual to capture
all module parameters belonging to one feature structurized together
(like 'gvt' in this patch), or just to leave them directly exposed?

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

Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4

[Sharma, Shashank]

Hi Ville, 
We will look into this in sometime. Right now team is slightly loaded due to 
project milestone. 
Last time I looked into this, we dint have this HW to reproduce this issue.

Regards
Shashank
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] 
Sent: Tuesday, February 23, 2016 8:38 PM
To: Oleksandr Natalenko
Cc: Vetter, Daniel; intel-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org; linux-ker...@vger.kernel.org; Daniel Vetter; 
Jindal, Sonika; Sharma, Shashank; Wang, Gary C
Subject: Re: [REGRESSION] i915: No HDMI output with 4.4

On Mon, Feb 22, 2016 at 02:32:32PM +0200, Oleksandr Natalenko wrote:
> Ville, Daniel,
> 
> any additional info I could provide? I have to return dual-link DVI 
> cable back, so let me know if I could reveal more details if necessary.

Unfortunately I'm out of ideas for now. Daniel is on vacation.
Anyone else? VPG folks should take the ball here since they broke it.

In the meantime I think as a workaround I think you could use something like 
video=HDMI-A-1:e on the kernel command line (not sure I got the connector name 
right for your system). I think that should result in the live status check to 
be skipped, at least when populating the mode list.

Might be a good idea to collect all the information here and put in a bug 
report (https://bugs.freedesktop.org/ -> DRI -> DRM/Intel) so that all the logs 
and such would be in one place.

> 
> Regards,
>Oleksandr
> 
> 16.02.2016 14:54, Daniel Vetter написав:
> > On Tue, Feb 16, 2016 at 12:58:56PM +0200, Oleksandr Natalenko wrote:
> >> Ville, Daniel,
> >> 
> >> I've just got another monitor and another DVI-HDMI cable, and here 
> >> what I've got.
> >> 
> >> ===Single Link DVI-D cable with 3 different monitors===
> >> 
> >> Computer DVI ——— DVI-D (Single Link)/HDMI cable ——— HDMI LG 
> >> 23MP65HQ-P === not working
> > 
> > I presume the above LG screen is what you've called previously "old 
> > monitor"?
> > 
> >> Computer DVI ——— DVI-D (Single Link)/HDMI cable ——— HDMI LG 
> >> 23MP67HQ-P === not working Computer DVI ——— DVI-D (Single 
> >> Link)/HDMI cable ——— HDMI LG 23MP55HQ-P === works!
> >> 
> >> ===Dual Link DVI-D cable with monitor that doesn't work with Single 
> >> Link cable===
> >> 
> >> Computer DVI ——— DVI-D (Dual Link)/HDMI cable ——— HDMI LG 
> >> 23MP65HQ-P === works!
> > 
> > Funky. Can you pls grab the debug logs (with the special patches 
> > from
> > Ville) for this case? I wonder why suddenly different cable and it 
> > works.
> > 
> > Also: Is this one of these older-ish screens where you must have a 
> > dual-link cable to drive it at full resolution rate?
> > -Daniel
> > 
> > 
> >> ===Laptop with HDMI output===
> >> 
> >> Laptop HDMI ——— HDMI/HDMI cable ——— HDMI LG 23MP65HQ-P === works!
> >> 
> >> I'd say that single link DVI cables are broken with new kernel, but 
> >> one of monitors could work with such a cable. So I have no idea :(.
> >> 
> >> Regards,
> >>   Oleksandr.
> >> 
> >> 15.02.2016 17:42, Daniel Vetter wrote:
> >> >The other downside is that it'll make us non-compliant, which was 
> >> >the point of this entire ordeal: HDMI spec forbids us from 
> >> >starting any i2c transactions when the hpd isn't signalling a present 
> >> >screen.
> >> >
> >> >So maybe we need to buy one of these broken screens.
> >> >
> >> >Oleksandr, what exact model are you using? And any chance that you 
> >> >could test this on some other machine with intel gfx and latest 
> >> >kernel, just to make sure this really is some issue with the sink 
> >> >and not with the machine itself? And I guess you've tested with 
> >> >some other hdmi sink, and that works?

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

Re: [Intel-gfx] [RFCv2 02/14] drm/i915/gvt: Introduce the basic architecture of GVT-g

[Tian, Kevin]

> From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com]
> Sent: Tuesday, February 23, 2016 8:42 PM
> 
> Hi,
> 
> Code is looking a lot better.
> 
> A general question still; why you seem to be preferring multi-stage
> alloc and destroy?

One reason for multi-stage, IMO, is that GVT needs to do a snapshot
of initial MMIO state before i915 driver does actual initialization. That
snapshot will be presented to each VM which can then observe same
state as it would observe on a bare metal. Then the majority of
initialization needs to wait after i915 driver completes initialization. 
Zhi can correct me here.

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

Re: [Intel-gfx] [PATCH v6 3/7] drm/i915: Add per context timelines to fence object

[Maarten Lankhorst]

Op 22-02-16 om 15:33 schreef John Harrison:
> On 18/02/2016 14:49, Chris Wilson wrote:
>> On Thu, Feb 18, 2016 at 02:24:06PM +, john.c.harri...@intel.com wrote:
>>> From: John Harrison 
>>>
>>> The fence object used inside the request structure requires a sequence
>>> number. Although this is not used by the i915 driver itself, it could
>>> potentially be used by non-i915 code if the fence is passed outside of
>>> the driver. This is the intention as it allows external kernel drivers
>>> and user applications to wait on batch buffer completion
>>> asynchronously via the dma-buff fence API.
>>>
>>> To ensure that such external users are not confused by strange things
>>> happening with the seqno, this patch adds in a per context timeline
>>> that can provide a guaranteed in-order seqno value for the fence. This
>>> is safe because the scheduler will not re-order batch buffers within a
>>> context - they are considered to be mutually dependent.
>> This is still nonsense. Just implement per-context seqno.
> If you already have a set of patches to implement per-context seqno then 
> let's get them merged. Otherwise, that is follow up work to be done once the 
> scheduler has landed. There has already been too much churn and delay. So the 
> decision is to get the scheduler in as soon as possible and any 'could do 
> better' issues should be logged for follow up work.
Seems to me that per context seqno would be cleaner than this hack..

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

Re: [Intel-gfx] [RFCv2 06/14] drm/i915: let __i915_gem_context_create() takes context creation params

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> 
> Let the core logic of context creation service creats the GEM context by
> context creation params.
> 
> Now it provides following options for context creation:
> - Need to create legacy context for this GEM context?
> - Need to create PPGTT instance for this GEM context?
> - Need to treat this context as the global default context?
> 
> And for all the assumptions in the original implementation, we keep them
> in the upper-level wrapper.
> 
> Signed-off-by: Zhi Wang 
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 71
> -
>  1 file changed, 43 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 5516346..cda09f7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -223,6 +223,13 @@ static int __create_legacy_hw_context(struct drm_device 
> *dev,
>   return 0;
>  }
> 
> +struct i915_gem_context_create_params {
> + struct drm_i915_file_private *file_priv;
> + bool has_legacy_ctx;

'has_legacy_ctx" means slightly different from 'is_legacy_ctx' in
original place. :-)

> + bool has_ppgtt;

similarly 'has_ppgtt' has different meaning from original 
USE_FULL_PPGTT... from your purpose looks it is for indicating
whether a new ppgtt should be created here, then a clearer
name is required.

> + bool is_default_ctx;

You abandoned 'global' from original name. Any reason?

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

Re: [Intel-gfx] [RFCv2 07/14] drm/i915: factor out __intel_lr_context_deferred_alloc()

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
> b/drivers/gpu/drm/i915/intel_lrc.h
> index e6cda3e..528c4fb 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -97,10 +97,18 @@ static inline void intel_logical_ring_emit_reg(struct 
> intel_ringbuffer
> *ringbuf,
>  #define LRC_PPHWSP_PN(LRC_GUCSHR_PN + 1)
>  #define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
> 
> +struct intel_lr_context_alloc_params {
> + struct intel_engine_cs *ring;
> + u32 ringbuffer_size;
> + bool ctx_needs_init;

'ctx_initialized'?

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

[Intel-gfx] [PATCH 2/2] drm/i915: GPIO for CHT generic MIPI

[Deepak M]

From: Yogesh Mohan Marimuthu 

The GPIO configuration and register offsets are different from
baytrail for cherrytrail. Port the gpio programming accordingly
for cherrytrail in this patch.

v2: Removing the duplication of parsing

v3: Moved the macro def to panel_vbt.c file

Cc: Ville Syrjälä 
Cc: Jani Nikula 
Signed-off-by: Yogesh Mohan Marimuthu 
Signed-off-by: Deepak M 
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 123 +++--
 1 file changed, 98 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 794bd1f..6b9a1f7 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -58,6 +58,28 @@ static inline struct vbt_panel *to_vbt_panel(struct 
drm_panel *panel)
 
 #define NS_KHZ_RATIO 100
 
+#define CHV_IOSF_PORT_GPIO_N 0x13
+#define CHV_IOSF_PORT_GPIO_SE0x48
+#define CHV_IOSF_PORT_GPIO_SW0xB2
+#define CHV_IOSF_PORT_GPIO_E 0xA8
+#define CHV_MAX_GPIO_NUM_N   72
+#define CHV_MAX_GPIO_NUM_SE  99
+#define CHV_MAX_GPIO_NUM_SW  197
+#define CHV_MIN_GPIO_NUM_SE  73
+#define CHV_MIN_GPIO_NUM_SW  100
+#define CHV_MIN_GPIO_NUM_E   198
+
+#define CHV_PAD_FMLY_BASE0x4400
+#define CHV_PAD_FMLY_SIZE0x400
+#define CHV_PAD_CFG_0_1_REG_SIZE 0x8
+#define CHV_PAD_CFG_REG_SIZE 0x4
+#define CHV_VBT_MAX_PINS_PER_FMLY15
+
+#define CHV_GPIO_CFG_UNLOCK0x
+#define CHV_GPIO_CFG_HIZ   0x8100
+#define CHV_GPIO_CFG_TX_STATE_SHIFT1
+
+
 #define VLV_HV_DDI0_HPD_GPIONC_0_PCONF0 0x4130
 #define VLV_HV_DDI0_HPD_GPIONC_0_PAD0x4138
 #define VLV_HV_DDI0_DDC_SDA_GPIONC_1_PCONF0 0x4120
@@ -685,34 +707,13 @@ static const u8 *mipi_exec_delay(struct intel_dsi 
*intel_dsi, const u8 *data)
return data;
 }
 
-static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
+void vlv_program_gpio(struct intel_dsi *intel_dsi, u8 gpio, u8 action)
 {
-   u8 gpio, action;
+   struct drm_device *dev = intel_dsi->base.base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
u16 function, pad;
u32 val;
u8 port;
-   struct drm_device *dev = intel_dsi->base.base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-
-   DRM_DEBUG_DRIVER("MIPI: executing gpio element\n");
-
-   if (dev_priv->vbt.dsi.seq_version >= 3)
-   data++;
-
-   gpio = *data++;
-
-   /* pull up/down */
-   action = *data++ & 1;
-
-   if (gpio >= ARRAY_SIZE(gtable)) {
-   DRM_DEBUG_KMS("unknown gpio %u\n", gpio);
-   goto out;
-   }
-
-   if (!IS_VALLEYVIEW(dev_priv)) {
-   DRM_DEBUG_KMS("GPIO element not supported on this platform\n");
-   goto out;
-   }
 
if (dev_priv->vbt.dsi.seq_version >= 3) {
if (gpio <= IOSF_MAX_GPIO_NUM_NC) {
@@ -728,7 +729,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi 
*intel_dsi, const u8 *data)
port = IOSF_PORT_GPIO_SUS;
} else {
DRM_ERROR("GPIO number is not present in the table\n");
-   goto out;
+   return;
}
} else {
port = IOSF_PORT_GPIO_NC;
@@ -750,6 +751,78 @@ static const u8 *mipi_exec_gpio(struct intel_dsi 
*intel_dsi, const u8 *data)
/* pull up/down */
vlv_iosf_sb_write(dev_priv, port, pad, val);
mutex_unlock(_priv->sb_lock);
+}
+
+void chv_program_gpio(struct intel_dsi *intel_dsi, u8 gpio, u8 action)
+{
+   struct drm_device *dev = intel_dsi->base.base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u16 function, pad;
+   u16 family_num;
+   u8 block;
+
+   if (dev_priv->vbt.dsi.seq_version >= 3) {
+   if (gpio <= CHV_MAX_GPIO_NUM_N) {
+   block = CHV_IOSF_PORT_GPIO_N;
+   DRM_DEBUG_DRIVER("GPIO is in the north Block\n");
+   } else if (gpio <= CHV_MAX_GPIO_NUM_SE) {
+   block = CHV_IOSF_PORT_GPIO_SE;
+   gpio = gpio - CHV_MIN_GPIO_NUM_SE;
+   DRM_DEBUG_DRIVER("GPIO is in the south east Block\n");
+   } else if (gpio <= CHV_MAX_GPIO_NUM_SW) {
+   block = CHV_IOSF_PORT_GPIO_SW;
+   gpio = gpio - CHV_MIN_GPIO_NUM_SW;
+   DRM_DEBUG_DRIVER("GPIO is in the south west Block\n");
+   } else {
+ 

Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

[Takashi Iwai]

On Wed, 24 Feb 2016 08:51:32 +0100,
Takashi Iwai wrote:
> 
> Since dig_port_map[] is used only for the audio callback, we can
> assign it dynamically just before the callbacks.
> 
> Could you try the patch below?  (It's totally untested.)

The one below might be safer, it's setting in the mutex lock.
Please give this a try instead.


Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 31f6d212fb1b..30f921421b0c 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -527,6 +527,8 @@ void intel_audio_codec_enable(struct intel_encoder 
*intel_encoder)
 
mutex_lock(_priv->av_mutex);
intel_dig_port->audio_connector = connector;
+   /* referred in audio callbacks */
+   dev_priv->dig_port_map[port] = intel_encoder;
mutex_unlock(_priv->av_mutex);
 
if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
@@ -554,6 +556,7 @@ void intel_audio_codec_disable(struct intel_encoder 
*intel_encoder)
 
mutex_lock(_priv->av_mutex);
intel_dig_port->audio_connector = NULL;
+   dev_priv->dig_port_map[port] = NULL;
mutex_unlock(_priv->av_mutex);
 
if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5583d7..63ba42aa2985 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3311,7 +3311,6 @@ void intel_ddi_init(struct drm_device *dev, enum port 
port)
intel_encoder->get_config = intel_ddi_get_config;
 
intel_dig_port->port = port;
-   dev_priv->dig_port_map[port] = intel_encoder;
intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
  (DDI_BUF_PORT_REVERSAL |
   DDI_A_4_LANES);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d313cb9..ac6a37cbd323 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6035,7 +6035,6 @@ intel_dp_init(struct drm_device *dev,
}
 
intel_dig_port->port = port;
-   dev_priv->dig_port_map[port] = intel_encoder;
intel_dig_port->dp.output_reg = output_reg;
 
intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 4a77639a489d..23ee48dc765f 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2146,7 +2146,6 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
 void intel_hdmi_init(struct drm_device *dev,
 i915_reg_t hdmi_reg, enum port port)
 {
-   struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_digital_port *intel_dig_port;
struct intel_encoder *intel_encoder;
struct intel_connector *intel_connector;
@@ -2215,7 +2214,6 @@ void intel_hdmi_init(struct drm_device *dev,
intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
 
intel_dig_port->port = port;
-   dev_priv->dig_port_map[port] = intel_encoder;
intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4

[Oleksandr Natalenko]

24.02.2016 06:40, Jindal, Sonika написав:

Do you have logs for the failure with the single link hdmi cable and
the register dump which you have given for the working case?
If not, can you please capture the logs and register dump.
Also which platform is this?
If it is live status related issue, we need to see how long it is
taking to set the live status, or is it not setting it at all with the
single-link cable?


Emm, I've already posted all requested logs and dumps in this thread 
before.


Any additional logs/dumps needed?

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

Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4

[Oleksandr Natalenko]

23.02.2016 17:08, Ville Syrjälä написав:

In the meantime I think as a workaround I think you could use
something like video=HDMI-A-1:e on the kernel command line (not sure
I got the connector name right for your system). I think that should
result in the live status check to be skipped, at least when
populating the mode list.


This definitely did the trick for me:

===
[  +0.000354] [drm] forcing HDMI-A-1 connector ON
===

And single-link cable works now.


Might be a good idea to collect all the information here and put in a
bug report (https://bugs.freedesktop.org/ -> DRI -> DRM/Intel) so that
all the logs and such would be in one place.


OK.

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

[Intel-gfx] [PATCH v2 4/6] drm/atomic: Handle encoder stealing from set_config better.

[Maarten Lankhorst]

Instead of failing with -EINVAL when conflicting encoders are found,
the legacy set_config will disable other connectors when encoders
conflict.

With the cleanup to update_output_state this is a lot easier to
implement. set_config only adds connectors to the state that are
modified, and because of the previous commit that calls
add_affected_connectors only on set->crtc it means any connector not
part of the modeset can be stolen from. We disable the connector in
that case, and possibly the crtc if no connectors are left.

Atomic modeset itself still doesn't allow encoder stealing, the
results would be too unpredictable.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic_helper.c | 69 +
 include/drm/drm_crtc.h  |  2 ++
 2 files changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index e89a5da27463..3543c7fcd072 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -86,6 +86,68 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
}
 }
 
+static int disable_conflicting_connectors(struct drm_atomic_state *state)
+{
+   struct drm_connector_state *conn_state;
+   struct drm_connector *connector;
+   struct drm_encoder *encoder;
+   unsigned encoder_mask = 0;
+   int i, ret;
+
+   for_each_connector_in_state(state, connector, conn_state, i) {
+   const struct drm_connector_helper_funcs *funcs = 
connector->helper_private;
+   struct drm_encoder *new_encoder;
+
+   if (!conn_state->crtc)
+   continue;
+
+   if (funcs->atomic_best_encoder)
+   new_encoder = funcs->atomic_best_encoder(connector, 
conn_state);
+   else
+   new_encoder = funcs->best_encoder(connector);
+
+   if (new_encoder)
+   encoder_mask |= 1 << drm_encoder_index(new_encoder);
+   }
+
+   drm_for_each_connector(connector, state->dev) {
+   struct drm_crtc_state *crtc_state;
+
+   if (drm_atomic_get_existing_connector_state(state, connector))
+   continue;
+
+   encoder = connector->state->best_encoder;
+   if (!encoder || !(encoder_mask & (1 << 
drm_encoder_index(encoder
+   continue;
+
+   conn_state = drm_atomic_get_connector_state(state, connector);
+   if (IS_ERR(conn_state))
+   return PTR_ERR(conn_state);
+
+   DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], 
disabling [CONNECTOR:%d:%s]\n",
+encoder->base.id, encoder->name,
+conn_state->crtc->base.id, 
conn_state->crtc->name,
+connector->base.id, connector->name);
+
+   crtc_state = drm_atomic_get_existing_crtc_state(state, 
conn_state->crtc);
+
+   ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+   if (ret)
+   return ret;
+
+   if (!crtc_state->connector_mask) {
+   ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
+   NULL);
+   if (ret < 0)
+   return ret;
+
+   crtc_state->active = false;
+   }
+   }
+
+   return 0;
+}
+
 static bool
 check_pending_encoder_assignment(struct drm_atomic_state *state,
 struct drm_encoder *new_encoder)
@@ -449,6 +511,12 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
}
}
 
+   if (state->legacy_set_config) {
+   ret = disable_conflicting_connectors(state);
+   if (ret)
+   return ret;
+   }
+
for_each_connector_in_state(state, connector, connector_state, i) {
/*
 * This only sets crtc->mode_changed for routing changes,
@@ -1797,6 +1865,7 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set)
if (!state)
return -ENOMEM;
 
+   state->legacy_set_config = true;
state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
 retry:
ret = __drm_atomic_helper_set_config(set, state);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7fad193dc645..9a946df27f07 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1677,6 +1677,7 @@ struct drm_bridge {
  * @dev: parent DRM device
  * @allow_modeset: allow full modeset
  * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
+ * @legacy_set_config: Disable conflicting encoders instead of failing with 
-EINVAL.
  * @planes: pointer to array of plane pointers
  * 

Re: [Intel-gfx] [PATCH i-g-t] tests/gem_buffered_svm: Buffered SVM tests

[Tvrtko Ursulin]

Hi,

On 10/02/16 18:01, Vinay Belgaumkar wrote:

These tests were initially reviewed/merged under the gem_softpin title.
They use softpinning and userptr mechanism to share buffers between
CPU and GPU.

The userptr part was decoupled from them recently. Adding these tests
under a different name to ensure buffered SVM usage testing.

The only change made was to instantiate the drm fd in the main instead
of every subtest.

Cc: Michel Thierry 
Cc: Tvrtko Ursulin 
---
  tests/Makefile.sources   |1 +
  tests/gem_buffered_svm.c | 1051 ++
  2 files changed, 1052 insertions(+)
  create mode 100644 tests/gem_buffered_svm.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index df92586..e6ec6f8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -17,6 +17,7 @@ TESTS_progs_M = \
drv_hangman \
gem_bad_reloc \
gem_basic \
+   gem_buffered_svm \
gem_busy \
gem_caching \
gem_close_race \
diff --git a/tests/gem_buffered_svm.c b/tests/gem_buffered_svm.c
new file mode 100644
index 000..90e63c4
--- /dev/null
+++ b/tests/gem_buffered_svm.c
@@ -0,0 +1,1051 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Vinay Belgaumkar 
+ *Thomas Daniel 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "intel_chipset.h"
+#include "intel_io.h"
+#include "i915_drm.h"
+#include 
+#include 
+#include 
+#include 
+#include "igt_kms.h"
+#include 
+#include 
+#include 
+#include "igt.h"
+
+#define BO_SIZE 4096
+#define MULTIPAGE_BO_SIZE 2 * BO_SIZE
+#define STORE_BATCH_BUFFER_SIZE 4
+#define EXEC_OBJECT_PINNED (1<<4)
+#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
+#define SHARED_BUFFER_SIZE 4096
+
+typedef struct drm_i915_gem_userptr i915_gem_userptr;
+
+static uint32_t init_userptr(int fd, i915_gem_userptr *, void *ptr, uint64_t 
size);
+static void *create_mem_buffer(uint64_t size);
+static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr);
+static void gem_pin_userptr_test(int fd);
+static void gem_pin_bo_test(int fd);
+static void gem_pin_invalid_vma_test(int fd, bool test_decouple_flags, bool 
test_canonical_offset);
+static void gem_pin_overlap_test(int fd);
+static void gem_pin_high_address_test(int fd);
+
+#define NO_PPGTT 0
+#define ALIASING_PPGTT 1
+#define FULL_32_BIT_PPGTT 2
+#define FULL_48_BIT_PPGTT 3
+/* uses_full_ppgtt
+ * Finds supported PPGTT details.
+ * @fd DRM fd
+ * @min can be
+ * 0 - No PPGTT
+ * 1 - Aliasing PPGTT
+ * 2 - Full PPGTT (32b)
+ * 3 - Full PPGTT (48b)
+ * RETURNS true/false if min support is present
+*/
+static bool uses_full_ppgtt(int fd, int min)
+{
+   struct drm_i915_getparam gp;
+   int val = 0;
+
+   memset(, 0, sizeof(gp));
+   gp.param = 18; /* HAS_ALIASING_PPGTT */
+   gp.value = 
+
+   if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, ))
+   return 0;
+
+   errno = 0;
+   return val >= min;
+}


gem_gtt_type got added to igt in the meantime so this can be reduced to:

static bool uses_full_ppgtt(int fd, int min)
{
return gem_gtt_type(fd) >= min;
}


+
+/* gem_call_userptr_ioctl
+ * Helper to call ioctl - TODO: move to lib
+ * @fd - drm fd
+ * @userptr - pointer to initialised userptr
+ * RETURNS status of ioctl call
+*/
+static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr)
+{
+   int ret;
+
+   ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_USERPTR, userptr);
+
+   if (ret)
+   ret = errno;
+
+   return ret;
+}
+
+/* init_userptr
+ * Helper that inits userptr an returns 

[Intel-gfx] [PATCH] drm/i915: Update state before setting watermarks.

[Maarten Lankhorst]

When intel_update_watermarks is called on skylake it inspects
crtc->state, which should show as disabled. This wasn't the case,
and this resulted in a divide-by-zero in
skl_ddb_get_pipe_allocation_limits when intel_update_watermarks was called.

 [ cut here ]
 WARNING: CPU: 1 PID: 295 at drivers/gpu/drm/i915/intel_pm.c:2834 
skl_update_pipe_wm+0x102/0x8c0 [i915]()
 WARN_ON(!config->num_pipes_active)
 Modules linked in: coretemp i915(+) 
x
 CPU: 1 PID: 295 Comm: systemd-udevd Tainted: G U  W   4.5.0-rc4-xx 
#25
 Hardware name: x
   88003777f5a8 813485c2 88003777f5f0
  a0236240 88003777f5e0 81050fce 8800aa42
  8800aba18000 8800aba18000 880037304c00 8800aa42
 Call Trace:
  [] dump_stack+0x67/0x95
  [] warn_slowpath_common+0x9e/0xc0
  [] warn_slowpath_fmt+0x4c/0x50
  [] ? flush_work+0x8e/0x280
  [] ? flush_work+0x5/0x280
  [] skl_update_pipe_wm+0x102/0x8c0 [i915]
  [] skl_update_wm+0xff/0x5f0 [i915]
  [] ? trace_hardirqs_on_caller+0x15e/0x1d0
  [] ? trace_hardirqs_on+0xd/0x10
  [] intel_update_watermarks+0x1e/0x30 [i915]
  [] intel_crtc_disable_noatomic+0xd2/0x150 [i915]
  [] intel_modeset_setup_hw_state+0xdd2/0xde0 [i915]
  [] intel_modeset_init+0x15a3/0x1950 [i915]
  [] i915_driver_load+0x13c6/0x1720 [i915]
  [] ? add_sysfs_fw_map_entry+0x9b/0x9b
  [] drm_dev_register+0x6f/0xb0 [drm]
  [] drm_get_pci_dev+0x10a/0x1d0 [drm]
  [] i915_pci_probe+0x49/0x50 [i915]
  [] pci_device_probe+0x80/0xf0
  [] driver_probe_device+0x1bc/0x3d0
  [] __driver_attach+0x66/0x90
  [] ? driver_probe_device+0x3d0/0x3d0
  [] bus_for_each_dev+0x5b/0xa0
  [] driver_attach+0x1e/0x20
  [] bus_add_driver+0x151/0x270
  [] driver_register+0x8c/0xd0
  [] __pci_register_driver+0x5d/0x60
  [] drm_pci_init+0x58/0xf0 [drm]
  [] ? trace_hardirqs_on+0xd/0x10
  [] ? 0xa02aa000
  [] i915_init+0x94/0x9b [i915]
  [] do_one_initcall+0x113/0x1f0
  [] ? rcu_read_lock_sched_held+0x61/0x90
  [] ? kmem_cache_alloc_trace+0x1cc/0x280
  [] do_init_module+0x60/0x1c8
  [] load_module+0x1ceb/0x2410
  [] ? store_uevent+0x40/0x40
  [] ? kernel_read+0x41/0x60
  [] SYSC_finit_module+0x8d/0xa0
  [] SyS_finit_module+0xe/0x10
  [] entry_SYSCALL_64_fastpath+0x12/0x6f
 ---[ end trace 1149e9ab3695a423 ]---
 [ cut here ]

Reported-by: Tvrtko Ursulin 
Signed-off-by: Maarten Lankhorst 
---
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index deee56010eee..d9e0470419a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6358,6 +6358,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 
 static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
 {
+   struct intel_encoder *encoder;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
enum intel_display_power_domain domain;
@@ -6378,6 +6379,19 @@ static void intel_crtc_disable_noatomic(struct drm_crtc 
*crtc)
dev_priv->display.crtc_disable(crtc);
intel_crtc->active = false;
intel_fbc_disable(intel_crtc);
+
+   DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was enabled, now 
disabled\n",
+ crtc->base.id);
+
+   WARN_ON(drm_atomic_set_mode_for_crtc(crtc->state, NULL) < 0);
+   crtc->state->active = false;
+   crtc->enabled = false;
+   crtc->state->connector_mask = 0;
+   crtc->state->encoder_mask = 0;
+
+   for_each_encoder_on_crtc(crtc->dev, crtc, encoder)
+   encoder->base.crtc = NULL;
+
intel_update_watermarks(crtc);
intel_disable_shared_dpll(intel_crtc);
 
@@ -15526,38 +15540,9 @@ static void intel_sanitize_crtc(struct intel_crtc 
*crtc)
 
/* Adjust the state of the output pipe according to whether we
 * have active connectors/encoders. */
-   if (!intel_crtc_has_encoders(crtc))
+   if (crtc->active && !intel_crtc_has_encoders(crtc))
intel_crtc_disable_noatomic(>base);
 
-   if (crtc->active != crtc->base.state->active) {
-   struct intel_encoder *encoder;
-
-   /* This can happen either due to bugs in the get_hw_state
-* functions or because of calls to intel_crtc_disable_noatomic,
-* or because the pipe is force-enabled due to the
-* pipe A quirk. */
-   DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n",
- crtc->base.base.id,
- crtc->base.state->enable ? "enabled" : "disabled",
- crtc->active ? "enabled" : "disabled");
-
-   WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) < 
0);
-   

[Intel-gfx] [PATCH v2 3/6] drm/atomic: Always call steal_encoder.

[Maarten Lankhorst]

There's no need to have a separate function to get the crtc
which is stolen, this can already be found when actually
stealing the encoder.

drm_for_each_connector already checks for connection_mutex, so
use that macro now.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic_helper.c | 77 +++--
 1 file changed, 22 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 3d1f97a832fc..e89a5da27463 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -106,25 +106,6 @@ check_pending_encoder_assignment(struct drm_atomic_state 
*state,
return true;
 }
 
-static struct drm_crtc *
-get_current_crtc_for_encoder(struct drm_device *dev,
-struct drm_encoder *encoder)
-{
-   struct drm_mode_config *config = >mode_config;
-   struct drm_connector *connector;
-
-   WARN_ON(!drm_modeset_is_locked(>connection_mutex));
-
-   drm_for_each_connector(connector, dev) {
-   if (connector->state->best_encoder != encoder)
-   continue;
-
-   return connector->state->crtc;
-   }
-
-   return NULL;
-}
-
 static void
 set_best_encoder(struct drm_atomic_state *state,
 struct drm_connector_state *conn_state,
@@ -168,47 +149,39 @@ set_best_encoder(struct drm_atomic_state *state,
 
 static int
 steal_encoder(struct drm_atomic_state *state,
- struct drm_encoder *encoder,
- struct drm_crtc *encoder_crtc)
+ struct drm_encoder *encoder)
 {
-   struct drm_mode_config *config = >dev->mode_config;
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
 
-   /*
-* We can only steal an encoder coming from a connector, which means we
-* must already hold the connection_mutex.
-*/
-   WARN_ON(!drm_modeset_is_locked(>connection_mutex));
-
-   DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing 
it\n",
-encoder->base.id, encoder->name,
-encoder_crtc->base.id, encoder_crtc->name);
+   drm_for_each_connector(connector, state->dev) {
+   struct drm_crtc *encoder_crtc;
 
-   crtc_state = drm_atomic_get_crtc_state(state, encoder_crtc);
-   if (IS_ERR(crtc_state))
-   return PTR_ERR(crtc_state);
-
-   crtc_state->connectors_changed = true;
-
-   list_for_each_entry(connector, >connector_list, head) {
if (connector->state->best_encoder != encoder)
continue;
 
-   DRM_DEBUG_ATOMIC("Stealing encoder from [CONNECTOR:%d:%s]\n",
-connector->base.id,
-connector->name);
-
connector_state = drm_atomic_get_connector_state(state,
 connector);
if (IS_ERR(connector_state))
return PTR_ERR(connector_state);
 
-   if (connector_state->best_encoder != encoder)
+   if (connector_state->best_encoder != encoder ||
+   WARN_ON(!connector_state->crtc))
continue;
 
+   encoder_crtc = connector_state->crtc;
+
+   DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], 
stealing it\n",
+encoder->base.id, encoder->name,
+encoder_crtc->base.id, encoder_crtc->name);
+
set_best_encoder(state, connector_state, NULL);
+
+   crtc_state = drm_atomic_get_existing_crtc_state(state, 
encoder_crtc);
+   crtc_state->connectors_changed = true;
+
+   return 0;
}
 
return 0;
@@ -221,7 +194,6 @@ update_connector_routing(struct drm_atomic_state *state,
 {
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
-   struct drm_crtc *encoder_crtc;
struct drm_crtc_state *crtc_state;
int idx, ret;
 
@@ -299,17 +271,12 @@ update_connector_routing(struct drm_atomic_state *state,
return -EINVAL;
}
 
-   encoder_crtc = get_current_crtc_for_encoder(state->dev,
-   new_encoder);
-
-   if (encoder_crtc) {
-   ret = steal_encoder(state, new_encoder, encoder_crtc);
-   if (ret) {
-   DRM_DEBUG_ATOMIC("Encoder stealing failed for 
[CONNECTOR:%d:%s]\n",
-connector->base.id,
-connector->name);
-   return ret;
-   }
+   ret = steal_encoder(state, new_encoder);
+   if (ret) {
+   

Re: [Intel-gfx] [RFCv2 02/14] drm/i915/gvt: Introduce the basic architecture of GVT-g

[Tian, Kevin]

> From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com]
> Sent: Tuesday, February 23, 2016 8:54 PM
> > diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
> > b/drivers/gpu/drm/i915/gvt/Makefile
> > new file mode 100644
> > index 000..959305f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/Makefile
> > @@ -0,0 +1,5 @@
> > +GVT_SOURCE := gvt.o params.o
> > +
> > +ccflags-y  += -I$(src) -I$(src)/.. -Wall -Werror 
> > -Wno-unused-function
> > +i915_gvt-y := $(GVT_SOURCE)
> 
> (This name conflicts with upper level i915_gvt, which I suggested
> renaming to intel_gvt.c. A one more reason more so this can be kept as
> is).
> 

Curious what's the criteria whether a file should be called i915_xxx or 
intel_xxx?

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

Re: [Intel-gfx] [RFCv2 03/14] drm/i915: Introduce host graphics memory/fence partition for GVT-g

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 52cfa32..2099b7e 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -348,6 +348,10 @@ void *gvt_create_pgt_device(struct drm_i915_private
> *dev_priv)
>   goto err;
>   }
> 
> + dev_priv->gvt.host_fence_sz = gvt.host_fence_sz;
> + dev_priv->gvt.host_low_gm_sz_in_mb = gvt.host_low_gm_sz;
> + dev_priv->gvt.host_high_gm_sz_in_mb = gvt.host_high_gm_sz;

Do we need hard limiting fence number for host usage here? There is no
continuity requirement as seen for graphics memory, since we do translate
fence# between guest view and host view. So we could make it flexible
as an on-demand allocation when creating a vGPU. Daniel even mentioned
, iirc, that today i915 can dynamically grab a fence register away from 
an application, which could be useful even when host fence usage is high
(not a typical case in server virtualization which runs few applications in 
host).

> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 9127f8f..de09dd4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2734,7 +2734,7 @@ static int i915_gem_setup_global_gtt(struct drm_device 
> *dev,
>   i915_address_space_init(ggtt_vm, dev_priv);
>   ggtt_vm->total += PAGE_SIZE;
> 
> - if (intel_vgpu_active(dev)) {
> + if (intel_vgpu_active(dev) || intel_gvt_active(dev)) {

above two conditions are bit confusing for others not familiar with 
this technology. vgpu_active is for driver running in a VM, while
gvt_active is for driver running in host. Could we introduce a better 
name, or at least wrap them into a more meaningful macro like
intel_ballooning_required?

>   ret = intel_vgt_balloon(dev);

I saw several comments whether ballooning is a right term here,
since we only do static reservation so far. How about renaming it
to intel_reserve_gm_resource to be more clear? In the future even
when we want to add true dynamic ballooning feature, it will be
largely refactored anyway. :-)

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

[Intel-gfx] Q: The relationship between "BIOS GT RC6 option" and RC_CONTROL

[Zhi Wang]

Hi:
  Had a question about "BIOS GT RC6 option". If I disabled the BIOS GT 
RC6 option, does that mean RC6 will be disabled no matter how driver 
configure RC_CONTROL registers on core platform?


Looks currently on some SoC platform e.g. BXT, the status of "BIOS GT 
RC6 option" will be reflected in the default value of RC_CONTROL or 
RC_STATE.


How about the core platform? As I didn't see the similar logic, I guess 
there should be some other kinds of approach here? :)


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

Re: [Intel-gfx] [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context requirement

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Wednesday, February 24, 2016 5:19 PM
> 
> Hi Kevin:
>   Now our context switch is covered by context status change notification 
> handler. In the
> status change handler we will save render registers. As GVT context will be a 
> single
> submission context we will restore the render registers when the GVT context 
> is
> scheduled-out by HW.

Thanks for explanation. It's fine then.

> 
> > -Original Message-
> > From: Tian, Kevin
> > Sent: Wednesday, February 24, 2016 4:56 PM
> > To: Wang, Zhi A; intel-gfx@lists.freedesktop.org; igv...@lists.01.org
> > Cc: Lv, Zhiyuan; Niu, Bing; Song, Jike; daniel.vet...@ffwll.ch; 
> > Cowperthwaite,
> > David J; ch...@chris-wilson.co.uk; joonas.lahti...@linux.intel.com
> > Subject: RE: [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context
> > requirement
> >
> > > From: Wang, Zhi A
> > > Sent: Thursday, February 18, 2016 7:42 PM
> > >
> > > This patchset is used to discuss and finalize the i915 changes
> > > required by GVT context. Previously we have discussed about how to
> > > hack i915 to meet GVT context requirement, and thanks for the idea and
> > comments.
> > >
> > > In this patchset, mostly it refactors the existing i915 APIs, spliting
> > > the hard-coded assumptions from its core logic, keep these assumptions
> > > in the high level wrapper and make the core logic much more flexible
> > > and config- urable, which is able to be used by GVT context creation and
> > submission.
> >
> > It would be good to note that this patch series is not the only change 
> > required
> > for GVT context management. It addresses creation/submission. We also need
> > some specific change in context switch time. Do you want to include them
> > together in next version to compose a full picture?
> >
> > Thanks
> > Kevin
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/atomic: Refuse to steal encoders from connectors not part of the state.

[Maarten Lankhorst]

Hey,

Op 18-02-16 om 13:59 schreef Ville Syrjälä:
> On Thu, Feb 18, 2016 at 01:43:11PM +0100, Daniel Vetter wrote:
>> On Thu, Feb 18, 2016 at 12:18:53PM +0100, Maarten Lankhorst wrote:
>>> Op 18-02-16 om 12:07 schreef Daniel Vetter:
 On Thu, Feb 18, 2016 at 09:54:43AM +0100, Maarten Lankhorst wrote:
> Because encoder <-> connector mapping is fixed when not moving to
> another crtc we can just reject connectors trying to steal an encoder
> from a connector not part of the state. This won't break MST on i915
> because in that case connectors will be part of the state if you switch
> them between crtc's. If they're not they stay on the same crtc, and
> encoder stealing would have failed anyway.
 We must do this for backwards compat. setCrtc on a connector that needs an
 encoder already used on some other crtc is supposed to disable that
 encoder (and the entire pipe if it's all unused) if we need it.
 -Daniel

>>> Could this be done from the setcrtc helper? Seems with atomic that wouldn't 
>>> be desired behavior.
>> If you want to avoid stealing with atomic, supply _all_ the
>> connectors/crtcs when doing an atomic modeset. After all the point of
>> atomic is to do global updates. I don't think it makes sense to have a
>> special case just for setcrtc, since it makes compat/transition
>> unecesserily complicated.
> I disagree. Having properties change magically is just a bad idea IMO.
> As far as checking for conflicts, IIRC I did that with a few bitmasks
> in my original atomic code, and it was pretty trivial. The current
> stealing  code we have is way too complicated for what it does IMO.
>
>> And we do this kind of stealing in other places
>> too with public api objects, e.g. if you move a plane.
> Mm. What exactly do we steal with planes?
I've sent a v2 that works nicely with "[IGT PATCH] tests/kms_setmode: Add tests 
when not stealing encoders on same crtc."
For all other calls disabling connectors to steal its encoder is rejected, but 
the behavior is preserved for set_config only.

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

[Intel-gfx] [PATCH v6 3/8] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v6.

[Maarten Lankhorst]

Currently we perform our own wait in post_plane_update,
but the atomic core performs another one in wait_for_vblanks.
This means that 2 vblanks are done when a fb is changed,
which is a bit overkill.

Merge them by creating a helper function that takes a crtc mask
for the planes to wait on.

The broadwell vblank workaround may look gone entirely but this is
not the case. pipe_config->wm_changed is set to true
when any plane is turned on, which forces a vblank wait.

Changes since v1:
- Removing the double vblank wait on broadwell moved to its own commit.
Changes since v2:
- Move out POWER_DOMAIN_MODESET handling to its own commit.
Changes since v3:
- Do not wait for vblank on legacy cursor updates. (Ville)
- Move broadwell vblank workaround comment to page_flip_finished. (Ville)
Changes since v4:
- Compile fix, legacy_cursor_flip -> *_update.
Changes since v5:
- Kill brackets.
- Add WARN_ON when wait_for_vblanks fails.
- Remove extra newlines.
- Split the checks whether vblank is needed to a separate function,
  with comments why a vblank is needed.
Signed-off-by: Maarten Lankhorst 
---
v5 was skipped, previous version was v5 because it had the compile fix.

diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 4625f8a9ba12..8e579a8505ac 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -97,6 +97,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->disable_lp_wm = false;
crtc_state->disable_cxsr = false;
crtc_state->wm_changed = false;
+   crtc_state->fb_changed = false;
 
return _state->base;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2107e324cd9e..9f32cb0bf978 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4792,9 +4792,6 @@ static void intel_post_plane_update(struct intel_crtc 
*crtc)
to_intel_crtc_state(crtc->base.state);
struct drm_device *dev = crtc->base.dev;
 
-   if (atomic->wait_vblank)
-   intel_wait_for_vblank(dev, crtc->pipe);
-
intel_frontbuffer_flip(dev, atomic->fb_bits);
 
crtc->wm.cxsr_allowed = true;
@@ -10957,6 +10954,12 @@ static bool page_flip_finished(struct intel_crtc *crtc)
return true;
 
/*
+* BDW signals flip done immediately if the plane
+* is disabled, even if the plane enable is already
+* armed to occur at the next vblank :(
+*/
+
+   /*
 * A DSPSURFLIVE check isn't enough in case the mmio and CS flips
 * used the same base address. In that case the mmio flip might
 * have completed, but the CS hasn't even executed the flip yet.
@@ -11833,6 +11836,9 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
if (!was_visible && !visible)
return 0;
 
+   if (fb != old_plane_state->base.fb)
+   pipe_config->fb_changed = true;
+
turn_off = was_visible && (!visible || mode_changed);
turn_on = visible && (!was_visible || mode_changed);
 
@@ -11847,11 +11853,8 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
pipe_config->wm_changed = true;
 
/* must disable cxsr around plane enable/disable */
-   if (plane->type != DRM_PLANE_TYPE_CURSOR) {
-   if (is_crtc_enabled)
-   intel_crtc->atomic.wait_vblank = true;
+   if (plane->type != DRM_PLANE_TYPE_CURSOR)
pipe_config->disable_cxsr = true;
-   }
} else if (intel_wm_need_update(plane, plane_state)) {
pipe_config->wm_changed = true;
}
@@ -11865,14 +11868,6 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
intel_crtc->atomic.post_enable_primary = turn_on;
intel_crtc->atomic.update_fbc = true;
 
-   /*
-* BDW signals flip done immediately if the plane
-* is disabled, even if the plane enable is already
-* armed to occur at the next vblank :(
-*/
-   if (turn_on && IS_BROADWELL(dev))
-   intel_crtc->atomic.wait_vblank = true;
-
break;
case DRM_PLANE_TYPE_CURSOR:
break;
@@ -11885,13 +11880,11 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
 */
if (IS_IVYBRIDGE(dev) &&
needs_scaling(to_intel_plane_state(plane_state)) &&
-   !needs_scaling(old_plane_state)) {
-   to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
-   } else if (turn_off && !mode_changed) {
-   intel_crtc->atomic.wait_vblank = true;
+   

[Intel-gfx] [PATCH v5] drm/i915/execlists: Move WA_TAIL_DWORDS to callee

[Dave Gordon]

From: Chris Wilson 

Currently emit-request starts writing to the ring and reserves space
for a workaround to be emitted later whilst submitting the request. It
is easier to read if the caller only allocates sufficient space for
its access (then the reader can quickly verify that the ring begin
allocates the exact space for the number of dwords emitted) and closes
the access to the ring. During submit, if we need to add the workaround,
we can reacquire ring access, in the assurance that we reserved space
for ourselves when beginning the request.

v3:
Reinstated #define for WA_TAIL_DWORDS so we could accommodate GPUs
that required different amounts of padding, generalised NOOP fill
[Rodrigo Vivi], added W/A space to reserved amount and updated
code comments [Dave Gordon],

v4:
Made #define for WA_RAIL_DWORDS a function-type macro in case we
want different values on different platforms (or engines), to
address comment by [Rodrigo Vivi]. But the current value is still
the constant (2) on all platforms, so this doesn't add any overhead.

v5:
Eliminated local variable & multiple indirections. Added long essay
comment about WaIdleLiteRestore. [Dave Gordon]

Originally-by: Rodrigo Vivi 
Rewritten-by: Chris Wilson 
Further-tweaked-by: Dave Gordon 

Signed-off-by: Dave Gordon 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Cc: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_lrc.c | 111 +--
 1 file changed, 72 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3a03646..ac71b13 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -225,6 +225,17 @@ enum {
 #define GEN8_CTX_ID_SHIFT 32
 #define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT  0x17
 
+/*
+ * Reserve space for some NOOPs at the end of each request, to be used by
+ * a workaround for not being allowed to do lite restore with HEAD==TAIL
+ * (WaIdleLiteRestore).
+ *
+ * The number of NOOPs is the same constant on all current platforms that
+ * require this, but in theory could be a platform- or engine- specific
+ * value based on the request.
+ */
+#define WA_TAIL_DWORDS(request)(2)
+
 static int intel_lr_context_pin(struct intel_context *ctx,
struct intel_engine_cs *engine);
 static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
@@ -457,21 +468,31 @@ static void execlists_context_unqueue(struct 
intel_engine_cs *ring)
 
if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
/*
-* WaIdleLiteRestore: make sure we never cause a lite
-* restore with HEAD==TAIL
+* WaIdleLiteRestore: lite restore must not have HEAD==TAIL.
+*
+* If a request has previously been submitted (as req1) and
+* is now being /re/submitted (as req0), it may actually have 
+* completed (with HEAD==TAIL), but we don't know that yet.
+*
+* Unfortunately the hardware requires that we not submit
+* a context that is already idle with HEAD==TAIL; but we
+* cannot safely check this because there would always be
+* an opportunity for a race, where the context /becomes/
+* idle after we check and before resubmission.
+*
+* So instead we increment the request TAIL here to ensure
+* that it is different from the last value seen by the
+* hardware, in effect always adding extra work to be done
+* even if the context has already completed. That work
+* consists of NOOPs added by intel_logical_ring_submit()
+* after the end of each request. Advancing TAIL turns
+* those NOOPs into part of the current request; if not so
+* consumed, they remain in the ringbuffer as a harmless
+* prefix to the next request.
 */
if (req0->elsp_submitted) {
-   /*
-* Apply the wa NOOPS to prevent ring:HEAD == req:TAIL
-* as we resubmit the request. See gen8_emit_request()
-* for where we prepare the padding after the end of the
-* request.
-*/
-   struct intel_ringbuffer *ringbuf;
-
-   ringbuf = req0->ctx->engine[ring->id].ringbuf;
req0->tail += 8;
-   req0->tail &= ringbuf->size - 1;
+   req0->tail &= req0->ringbuf->size - 1;
}
}
 
@@ 

Re: [Intel-gfx] [RFCv2 08/14] drm/i915: Support per-PPGTT address space mode

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> -static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> +static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, int 
> address_space_mode)

address_space_mode -> address_bits?

>  {
>   int ret;
> 
> @@ -1518,7 +1524,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>   ppgtt->base.bind_vma = ppgtt_bind_vma;
>   ppgtt->debug_dump = gen8_dump_ppgtt;
> 
> - if (USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
> + if (address_space_mode == 48) {

IS_48BIT_PPGTT

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

Re: [Intel-gfx] [RFCv2 13/14] drm/i915: Support context single submission when GVT is active

[Tian, Kevin]

> From: Wang, Zhi A
> Sent: Thursday, February 18, 2016 7:42 PM
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 1c0366a..2a6d6fe 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -479,6 +479,40 @@ static void execlists_i915_pick_requests(struct 
> intel_engine_cs
> *ring,
>   }
>  }
> 
> +static void execlists_gvt_pick_requests(struct intel_engine_cs *ring,
> + struct drm_i915_gem_request **req0,
> + struct drm_i915_gem_request **req1)

not read carefully, but if single submission itself has nothing tied to GVT,
better to make it generic since there may be other users in the future...

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