[Intel-gfx] [PATCH v2 1/7] drm/i915: move ibx_digital_port_connected to intel_dp.c
The function can be made static there. No functional changes.
Reviewed-by: Durgadoss R
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_display.c | 45 --
drivers/gpu/drm/i915/intel_dp.c | 61 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 --
3 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f604ce1c528b..1a0670259cdf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1061,51 +1061,6 @@ static void intel_wait_for_pipe_off(struct intel_crtc
*crtc)
}
}
-/*
- * ibx_digital_port_connected - is the specified port connected?
- * @dev_priv: i915 private structure
- * @port: the port to test
- *
- * Returns true if @port is connected, false otherwise.
- */
-bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
- struct intel_digital_port *port)
-{
- u32 bit;
-
- if (HAS_PCH_IBX(dev_priv->dev)) {
- switch (port->port) {
- case PORT_B:
- bit = SDE_PORTB_HOTPLUG;
- break;
- case PORT_C:
- bit = SDE_PORTC_HOTPLUG;
- break;
- case PORT_D:
- bit = SDE_PORTD_HOTPLUG;
- break;
- default:
- return true;
- }
- } else {
- switch (port->port) {
- case PORT_B:
- bit = SDE_PORTB_HOTPLUG_CPT;
- break;
- case PORT_C:
- bit = SDE_PORTC_HOTPLUG_CPT;
- break;
- case PORT_D:
- bit = SDE_PORTD_HOTPLUG_CPT;
- break;
- default:
- return true;
- }
- }
-
- return I915_READ(SDEISR) & bit;
-}
-
static const char *state_string(bool enabled)
{
return enabled ? "on" : "off";
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d32ce4841654..4aa3d664765b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4473,17 +4473,49 @@ edp_detect(struct intel_dp *intel_dp)
return status;
}
-static enum drm_connector_status
-ironlake_dp_detect(struct intel_dp *intel_dp)
+/*
+ * ibx_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Returns true if @port is connected, false otherwise.
+ */
+static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
+ struct intel_digital_port *port)
{
- struct drm_device *dev = intel_dp_to_dev(intel_dp);
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ u32 bit;
- if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
- return connector_status_disconnected;
+ if (HAS_PCH_IBX(dev_priv->dev)) {
+ switch (port->port) {
+ case PORT_B:
+ bit = SDE_PORTB_HOTPLUG;
+ break;
+ case PORT_C:
+ bit = SDE_PORTC_HOTPLUG;
+ break;
+ case PORT_D:
+ bit = SDE_PORTD_HOTPLUG;
+ break;
+ default:
+ return true;
+ }
+ } else {
+ switch (port->port) {
+ case PORT_B:
+ bit = SDE_PORTB_HOTPLUG_CPT;
+ break;
+ case PORT_C:
+ bit = SDE_PORTC_HOTPLUG_CPT;
+ break;
+ case PORT_D:
+ bit = SDE_PORTD_HOTPLUG_CPT;
+ break;
+ default:
+ return true;
+ }
+ }
- return intel_dp_detect_dpcd(intel_dp);
+ return I915_READ(SDEISR) & bit;
}
static int g4x_digital_port_connected(struct drm_device *dev,
@@ -4528,6 +4560,19 @@ static int g4x_digital_port_connected(struct drm_device
*dev,
}
static enum drm_connector_status
+ironlake_dp_detect(struct intel_dp *intel_dp)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+ if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+ return connector_status_disconnected;
+
+ return intel_dp_detect_dpcd(intel_dp);
+}
+
+static enum drm_connector_status
g4x_dp_detect(struct intel_dp *intel_dp)
{
struct drm_device *dev =
[Intel-gfx] [PATCH v2 0/7] drm/i915: clean up *_digital_port_connected
v2 with missing cases handled and intel_digital_port_connected return value changed to bool. Mostly it's just the addition of patches 2 and 3, and rebase of the rest. BR, Jani. Jani Nikula (7): drm/i915: move ibx_digital_port_connected to intel_dp.c drm/i915: make g4x_digital_port_connected return boolean status drm/i915: add MISSING_CASE annotation to ibx_digital_port_connected drm/i915: add common intel_digital_port_connected function drm/i915: split ibx_digital_port_connected to ibx and cpt variants drm/i915: split g4x_digital_port_connected to g4x and vlv variants drm/i915/bxt: Use correct live status register for BXT platform drivers/gpu/drm/i915/intel_display.c | 45 drivers/gpu/drm/i915/intel_dp.c | 199 +-- drivers/gpu/drm/i915/intel_drv.h | 2 - 3 files changed, 145 insertions(+), 101 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/7] drm/i915: make g4x_digital_port_connected return boolean status
We should not be hitting any of the default cases in
g4x_digital_port_connected, so add MISSING_CASE annotation and return
boolean status. The current behaviour is just cargo culting from the
days of yonder when the display port support was added to i915.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 26 +++---
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4aa3d664765b..d35dc0711f9b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4518,14 +4518,14 @@ static bool ibx_digital_port_connected(struct
drm_i915_private *dev_priv,
return I915_READ(SDEISR) & bit;
}
-static int g4x_digital_port_connected(struct drm_device *dev,
- struct intel_digital_port
*intel_dig_port)
+static bool g4x_digital_port_connected(struct drm_device *dev,
+ struct intel_digital_port *port)
{
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t bit;
if (IS_VALLEYVIEW(dev)) {
- switch (intel_dig_port->port) {
+ switch (port->port) {
case PORT_B:
bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
break;
@@ -4536,10 +4536,11 @@ static int g4x_digital_port_connected(struct drm_device
*dev,
bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
break;
default:
- return -EINVAL;
+ MISSING_CASE(port->port);
+ return false;
}
} else {
- switch (intel_dig_port->port) {
+ switch (port->port) {
case PORT_B:
bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
break;
@@ -4550,13 +4551,12 @@ static int g4x_digital_port_connected(struct drm_device
*dev,
bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
break;
default:
- return -EINVAL;
+ MISSING_CASE(port->port);
+ return false;
}
}
- if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
- return 0;
- return 1;
+ return I915_READ(PORT_HOTPLUG_STAT) & bit;
}
static enum drm_connector_status
@@ -4577,7 +4577,6 @@ g4x_dp_detect(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- int ret;
/* Can't disconnect eDP, but you can close the lid... */
if (is_edp(intel_dp)) {
@@ -4589,10 +4588,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
return status;
}
- ret = g4x_digital_port_connected(dev, intel_dig_port);
- if (ret == -EINVAL)
- return connector_status_unknown;
- else if (ret == 0)
+ if (!g4x_digital_port_connected(dev, intel_dig_port))
return connector_status_disconnected;
return intel_dp_detect_dpcd(intel_dp);
@@ -5059,7 +5055,7 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
if (!ibx_digital_port_connected(dev_priv,
intel_dig_port))
goto mst_fail;
} else {
- if (g4x_digital_port_connected(dev, intel_dig_port) !=
1)
+ if (!g4x_digital_port_connected(dev, intel_dig_port))
goto mst_fail;
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/7] drm/i915: add MISSING_CASE annotation to ibx_digital_port_connected
With the case added for eDP on port A (always connected from this
function's point of view), we should not be hitting any of the default
cases in ibx_digital_port_connected, so add MISSING_CASE annotation.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d35dc0711f9b..f08859471841 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4487,6 +4487,8 @@ static bool ibx_digital_port_connected(struct
drm_i915_private *dev_priv,
if (HAS_PCH_IBX(dev_priv->dev)) {
switch (port->port) {
+ case PORT_A:
+ return true;
case PORT_B:
bit = SDE_PORTB_HOTPLUG;
break;
@@ -4497,10 +4499,13 @@ static bool ibx_digital_port_connected(struct
drm_i915_private *dev_priv,
bit = SDE_PORTD_HOTPLUG;
break;
default:
- return true;
+ MISSING_CASE(port->port);
+ return false;
}
} else {
switch (port->port) {
+ case PORT_A:
+ return true;
case PORT_B:
bit = SDE_PORTB_HOTPLUG_CPT;
break;
@@ -4511,7 +4516,8 @@ static bool ibx_digital_port_connected(struct
drm_i915_private *dev_priv,
bit = SDE_PORTD_HOTPLUG_CPT;
break;
default:
- return true;
+ MISSING_CASE(port->port);
+ return false;
}
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 7/7] drm/i915/bxt: Use correct live status register for BXT platform
BXT platform uses live status bits from 0x0 register to obtain DP
status on hotplug. The existing g4x_digital_port_connected() uses a
different register and hence misses DP hotplug events on BXT
platform. This patch fixes it by using the appropriate register(0x0)
and live status bits(3:5).
Based on a patch by Durgadoss R , from whom the
commit message is shamelessly copy pasted.
Reported-by: Durgadoss R
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 36291838409b..508156cc750d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4569,6 +4569,29 @@ static bool vlv_digital_port_connected(struct
drm_i915_private *dev_priv,
return I915_READ(PORT_HOTPLUG_STAT) & bit;
}
+static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
+ struct intel_digital_port *port)
+{
+ u32 bit;
+
+ switch (port->port) {
+ case PORT_A:
+ bit = BXT_DE_PORT_HP_DDIA;
+ break;
+ case PORT_B:
+ bit = BXT_DE_PORT_HP_DDIB;
+ break;
+ case PORT_C:
+ bit = BXT_DE_PORT_HP_DDIC;
+ break;
+ default:
+ MISSING_CASE(port->port);
+ return false;
+ }
+
+ return I915_READ(GEN8_DE_PORT_ISR) & bit;
+}
+
/*
* intel_digital_port_connected - is the specified port connected?
* @dev_priv: i915 private structure
@@ -4583,6 +4606,8 @@ static bool intel_digital_port_connected(struct
drm_i915_private *dev_priv,
return ibx_digital_port_connected(dev_priv, port);
if (HAS_PCH_SPLIT(dev_priv))
return cpt_digital_port_connected(dev_priv, port);
+ else if (IS_BROXTON(dev_priv))
+ return bxt_digital_port_connected(dev_priv, port);
else if (IS_VALLEYVIEW(dev_priv))
return vlv_digital_port_connected(dev_priv, port);
else
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 6/7] drm/i915: split g4x_digital_port_connected to g4x and vlv variants
Choose the right function at the intel_digital_port_connected level.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 70 +++--
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 204d4376be59..36291838409b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4526,38 +4526,44 @@ static bool cpt_digital_port_connected(struct
drm_i915_private *dev_priv,
static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
struct intel_digital_port *port)
{
- uint32_t bit;
+ u32 bit;
- if (IS_VALLEYVIEW(dev_priv)) {
- switch (port->port) {
- case PORT_B:
- bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
- break;
- case PORT_C:
- bit = PORTC_HOTPLUG_LIVE_STATUS_VLV;
- break;
- case PORT_D:
- bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
- break;
- default:
- MISSING_CASE(port->port);
- return false;
- }
- } else {
- switch (port->port) {
- case PORT_B:
- bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
- break;
- case PORT_C:
- bit = PORTC_HOTPLUG_LIVE_STATUS_G4X;
- break;
- case PORT_D:
- bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
- break;
- default:
- MISSING_CASE(port->port);
- return false;
- }
+ switch (port->port) {
+ case PORT_B:
+ bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
+ break;
+ case PORT_C:
+ bit = PORTC_HOTPLUG_LIVE_STATUS_G4X;
+ break;
+ case PORT_D:
+ bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
+ break;
+ default:
+ MISSING_CASE(port->port);
+ return false;
+ }
+
+ return I915_READ(PORT_HOTPLUG_STAT) & bit;
+}
+
+static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv,
+ struct intel_digital_port *port)
+{
+ u32 bit;
+
+ switch (port->port) {
+ case PORT_B:
+ bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
+ break;
+ case PORT_C:
+ bit = PORTC_HOTPLUG_LIVE_STATUS_VLV;
+ break;
+ case PORT_D:
+ bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
+ break;
+ default:
+ MISSING_CASE(port->port);
+ return false;
}
return I915_READ(PORT_HOTPLUG_STAT) & bit;
@@ -4577,6 +4583,8 @@ static bool intel_digital_port_connected(struct
drm_i915_private *dev_priv,
return ibx_digital_port_connected(dev_priv, port);
if (HAS_PCH_SPLIT(dev_priv))
return cpt_digital_port_connected(dev_priv, port);
+ else if (IS_VALLEYVIEW(dev_priv))
+ return vlv_digital_port_connected(dev_priv, port);
else
return g4x_digital_port_connected(dev_priv, port);
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/7] drm/i915: split ibx_digital_port_connected to ibx and cpt variants
Choose the right function at the intel_digital_port_connected level.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 78 +++--
1 file changed, 43 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f947951a01d4..204d4376be59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4478,40 +4478,46 @@ static bool ibx_digital_port_connected(struct
drm_i915_private *dev_priv,
{
u32 bit;
- if (HAS_PCH_IBX(dev_priv->dev)) {
- switch (port->port) {
- case PORT_A:
- return true;
- case PORT_B:
- bit = SDE_PORTB_HOTPLUG;
- break;
- case PORT_C:
- bit = SDE_PORTC_HOTPLUG;
- break;
- case PORT_D:
- bit = SDE_PORTD_HOTPLUG;
- break;
- default:
- MISSING_CASE(port->port);
- return false;
- }
- } else {
- switch (port->port) {
- case PORT_A:
- return true;
- case PORT_B:
- bit = SDE_PORTB_HOTPLUG_CPT;
- break;
- case PORT_C:
- bit = SDE_PORTC_HOTPLUG_CPT;
- break;
- case PORT_D:
- bit = SDE_PORTD_HOTPLUG_CPT;
- break;
- default:
- MISSING_CASE(port->port);
- return false;
- }
+ switch (port->port) {
+ case PORT_A:
+ return true;
+ case PORT_B:
+ bit = SDE_PORTB_HOTPLUG;
+ break;
+ case PORT_C:
+ bit = SDE_PORTC_HOTPLUG;
+ break;
+ case PORT_D:
+ bit = SDE_PORTD_HOTPLUG;
+ break;
+ default:
+ MISSING_CASE(port->port);
+ return false;
+ }
+
+ return I915_READ(SDEISR) & bit;
+}
+
+static bool cpt_digital_port_connected(struct drm_i915_private *dev_priv,
+ struct intel_digital_port *port)
+{
+ u32 bit;
+
+ switch (port->port) {
+ case PORT_A:
+ return true;
+ case PORT_B:
+ bit = SDE_PORTB_HOTPLUG_CPT;
+ break;
+ case PORT_C:
+ bit = SDE_PORTC_HOTPLUG_CPT;
+ break;
+ case PORT_D:
+ bit = SDE_PORTD_HOTPLUG_CPT;
+ break;
+ default:
+ MISSING_CASE(port->port);
+ return false;
}
return I915_READ(SDEISR) & bit;
@@ -4567,8 +4573,10 @@ static bool g4x_digital_port_connected(struct
drm_i915_private *dev_priv,
static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
struct intel_digital_port *port)
{
- if (HAS_PCH_SPLIT(dev_priv))
+ if (HAS_PCH_IBX(dev_priv))
return ibx_digital_port_connected(dev_priv, port);
+ if (HAS_PCH_SPLIT(dev_priv))
+ return cpt_digital_port_connected(dev_priv, port);
else
return g4x_digital_port_connected(dev_priv, port);
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/7] drm/i915: add common intel_digital_port_connected function
Add a common intel_digital_port_connected() that splits out to functions
for different platforms. No functional changes.
v2: make the function return a boolean
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 41 ++---
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f08859471841..f947951a01d4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4473,13 +4473,6 @@ edp_detect(struct intel_dp *intel_dp)
return status;
}
-/*
- * ibx_digital_port_connected - is the specified port connected?
- * @dev_priv: i915 private structure
- * @port: the port to test
- *
- * Returns true if @port is connected, false otherwise.
- */
static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
struct intel_digital_port *port)
{
@@ -4524,13 +4517,12 @@ static bool ibx_digital_port_connected(struct
drm_i915_private *dev_priv,
return I915_READ(SDEISR) & bit;
}
-static bool g4x_digital_port_connected(struct drm_device *dev,
+static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
struct intel_digital_port *port)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t bit;
- if (IS_VALLEYVIEW(dev)) {
+ if (IS_VALLEYVIEW(dev_priv)) {
switch (port->port) {
case PORT_B:
bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
@@ -4565,6 +4557,22 @@ static bool g4x_digital_port_connected(struct drm_device
*dev,
return I915_READ(PORT_HOTPLUG_STAT) & bit;
}
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Return %true if @port is connected, %false otherwise.
+ */
+static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
+struct intel_digital_port *port)
+{
+ if (HAS_PCH_SPLIT(dev_priv))
+ return ibx_digital_port_connected(dev_priv, port);
+ else
+ return g4x_digital_port_connected(dev_priv, port);
+}
+
static enum drm_connector_status
ironlake_dp_detect(struct intel_dp *intel_dp)
{
@@ -4572,7 +4580,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+ if (!intel_digital_port_connected(dev_priv, intel_dig_port))
return connector_status_disconnected;
return intel_dp_detect_dpcd(intel_dp);
@@ -4594,7 +4602,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
return status;
}
- if (!g4x_digital_port_connected(dev, intel_dig_port))
+ if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
return connector_status_disconnected;
return intel_dp_detect_dpcd(intel_dp);
@@ -5057,13 +5065,8 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
/* indicate that we need to restart link training */
intel_dp->train_set_valid = false;
- if (HAS_PCH_SPLIT(dev)) {
- if (!ibx_digital_port_connected(dev_priv,
intel_dig_port))
- goto mst_fail;
- } else {
- if (!g4x_digital_port_connected(dev, intel_dig_port))
- goto mst_fail;
- }
+ if (!intel_digital_port_connected(dev_priv, intel_dig_port))
+ goto mst_fail;
if (!intel_dp_get_dpcd(intel_dp)) {
goto mst_fail;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Intel drm "Atomic update failure"
I've been experiencing system hangs with 4.2 release candidate kernels on Asus X553MA laptop. 4.1.0 seems stable. I've set serial console via ttyUSB0 so that I might be able to get some clue what might cause the hangs. With 4.2-rc7 (SHA 1b647a166f07dcf08709c8606470f4b17a4aa11d) I got the following: [ 4102.375874] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic update failure on pipe A (start=246190 end=246191) [ 7657.317892] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic update failure on pipe A (start=459302 end=459303) But the system didn't actually hang during the test when those messages appeared. Sometimes the hang might happen few minutes after booting and sometimes it might take few hours. I've now tried few times running with serial console and so far got only those two error messages. And there was also few hangs but I didn't get anything on console with them. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915: Enable full ppgtt for vgpu
The full ppgtt is supported in Intel GVT-g device model. So the restriction can be removed. Signed-off-by: Zhiyuan Lv Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/i915_gem_gtt.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ed10e77..823005c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6; has_full_ppgtt = INTEL_INFO(dev)->gen >= 7; - if (intel_vgpu_active(dev)) - has_full_ppgtt = false; /* emulation is too hard */ - /* * We don't allow disabling PPGTT for gen9+ as it's a requirement for * execlists, the sole mechanism available to submit work. -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/7] drm/i915: preallocate pdps for 32 bit vgpu
This is based on Mika Kuoppala's patch below:
http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/61104/match=workaround+hw+preload
The patch will preallocate the page directories for 32-bit PPGTT when
i915 runs inside a virtual machine with Intel GVT-g. With this change,
the root pointers in EXECLIST context will always keep the same.
The change is needed for vGPU because Intel GVT-g will do page table
shadowing, and needs to track all the page table changes from guest
i915 driver. However, if guest PPGTT is modified through GPU commands
like LRI, it is not possible to trap the operations in the right time,
so it will be hard to make shadow PPGTT to work correctly.
Shadow PPGTT could be much simpler with this change. Meanwhile
hypervisor could simply prohibit any attempt of PPGTT modification
through GPU command for security.
The function gen8_preallocate_top_level_pdps() in the patch is from
Mika, with only one change to set "used_pdpes" to avoid duplicated
allocation later.
Cc: Mika Kuoppala
Cc: Dave Gordon
Cc: Michel Thierry
Signed-off-by: Zhiyuan Lv
Signed-off-by: Zhi Wang
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 33 +
drivers/gpu/drm/i915/intel_lrc.c| 3 ++-
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4a76807..ed10e77 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1441,6 +1441,33 @@ static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt,
struct seq_file *m)
}
}
+static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
+{
+ unsigned long *new_page_dirs, **new_page_tables;
+ uint32_t pdpes = I915_PDPES_PER_PDP(dev);
+ int ret;
+
+ /* We allocate temp bitmap for page tables for no gain
+* but as this is for init only, lets keep the things simple
+*/
+ ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
+ if (ret)
+ return ret;
+
+ /* Allocate for all pdps regardless of how the ppgtt
+* was defined.
+*/
+ ret = gen8_ppgtt_alloc_page_directories(&ppgtt->base, &ppgtt->pdp,
+ 0, 1ULL << 32,
+ new_page_dirs);
+ if (!ret)
+ *ppgtt->pdp.used_pdpes = *new_page_dirs;
+
+ free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+
+ return ret;
+}
+
/*
* GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers
* with a net effect resembling a 2-level page table in normal x86 terms. Each
@@ -1484,6 +1511,12 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
trace_i915_page_directory_pointer_entry_alloc(&ppgtt->base,
0, 0,
GEN8_PML4E_SHIFT);
+
+ if (intel_vgpu_active(ppgtt->base.dev)) {
+ ret = gen8_preallocate_top_level_pdps(ppgtt);
+ if (ret)
+ goto free_scratch;
+ }
}
return 0;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e77b6b0..2dc8709 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1540,7 +1540,8 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request
*req,
* not needed in 48-bit.*/
if (req->ctx->ppgtt &&
(intel_ring_flag(req->ring) & req->ctx->ppgtt->pd_dirty_rings)) {
- if (!USES_FULL_48BIT_PPGTT(req->i915)) {
+ if (!USES_FULL_48BIT_PPGTT(req->i915) &&
+ !intel_vgpu_active(req->i915->dev)) {
ret = intel_logical_ring_emit_pdps(req);
if (ret)
return ret;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/7] drm/intel: guest i915 changes for Broadwell to run inside VM with Intel GVT-g
I915 kernel driver can now work inside a virtual machine on Haswell with Intel GVT-g. In order to do the same thing on Broadwell, there are some extra changes needed. The two main things are to support the more complicated PPGTT page table structure and EXECLIST contexts. GVT-g will perform shadow PPGTT and shadow context, which requires guest driver to explicitly notify host device model the life cycle of PPGTT and EXECLIST contexts. The first and the forth patches added some restrictions to drivers in virtualization scenario to make the shadow work easier. The first patch is based on Mika's earlier one, but we use it for vgpu only. The sixth patch is the implementation of the notification for shadowing. Zhiyuan Lv (7): drm/i915: preallocate pdps for 32 bit vgpu drm/i915: Enable full ppgtt for vgpu drm/i915: Always enable execlists on BDW for vgpu drm/i915: always pin lrc context for vgpu with Intel GVT-g drm/i915: Update PV INFO page definition for Intel GVT-g drm/i915: guest i915 notification for Intel-GVTg drm/i915: Allow Broadwell guest with Intel GVT-g drivers/gpu/drm/i915/i915_gem_gtt.c | 77 +++-- drivers/gpu/drm/i915/i915_vgpu.c| 2 +- drivers/gpu/drm/i915/i915_vgpu.h| 34 +++- drivers/gpu/drm/i915/intel_lrc.c| 44 ++--- 4 files changed, 145 insertions(+), 12 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/i915: Update PV INFO page definition for Intel GVT-g
Some more definitions in the PV info page are added. They are mainly
for the guest notification to Intel GVT-g device model. They are used
for Broadwell enabling.
Signed-off-by: Zhiyuan Lv
Signed-off-by: Zhi Wang
---
drivers/gpu/drm/i915/i915_vgpu.h | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 97a88b5..21c97f4 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -40,6 +40,19 @@
#define INTEL_VGT_IF_VERSION \
INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
+/*
+ * notifications from guest to vgpu device model
+ */
+enum vgt_g2v_type {
+ VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
+ VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
+ VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
+ VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
+ VGT_G2V_EXECLIST_CONTEXT_CREATE,
+ VGT_G2V_EXECLIST_CONTEXT_DESTROY,
+ VGT_G2V_MAX,
+};
+
struct vgt_if {
uint64_t magic; /* VGT_MAGIC */
uint16_t version_major;
@@ -70,11 +83,28 @@ struct vgt_if {
uint32_t rsv3[0x200 - 24]; /* pad to half page */
/*
* The bottom half page is for response from Gfx driver to hypervisor.
-* Set to reserved fields temporarily by now.
*/
uint32_t rsv4;
uint32_t display_ready; /* ready for display owner switch */
- uint32_t rsv5[0x200 - 2]; /* pad to one page */
+
+ uint32_t rsv5[4];
+
+ uint32_t g2v_notify;
+ uint32_t rsv6[7];
+
+ uint32_t pdp0_lo;
+ uint32_t pdp0_hi;
+ uint32_t pdp1_lo;
+ uint32_t pdp1_hi;
+ uint32_t pdp2_lo;
+ uint32_t pdp2_hi;
+ uint32_t pdp3_lo;
+ uint32_t pdp3_hi;
+
+ uint32_t execlist_context_descriptor_lo;
+ uint32_t execlist_context_descriptor_hi;
+
+ uint32_t rsv7[0x200 - 24];/* pad to one page */
} __packed;
#define vgtif_reg(x) \
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g
Intel GVT-g will perform EXECLIST context shadowing and ring buffer
shadowing. The shadow copy is created when guest creates a context.
If a context changes its LRCA address, the hypervisor is hard to know
whether it is a new context or not. We always pin context objects to
global GTT to make life easier.
Signed-off-by: Zhiyuan Lv
Signed-off-by: Zhi Wang
---
drivers/gpu/drm/i915/intel_lrc.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 39df304..4b2ac37 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2282,7 +2282,8 @@ void intel_lr_context_free(struct intel_context *ctx)
ctx->engine[i].ringbuf;
struct intel_engine_cs *ring = ringbuf->ring;
- if (ctx == ring->default_context) {
+ if ((ctx == ring->default_context) ||
+ (intel_vgpu_active(ring->dev))) {
intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
}
@@ -2353,6 +2354,8 @@ int intel_lr_context_deferred_create(struct intel_context
*ctx,
struct intel_engine_cs *ring)
{
const bool is_global_default_ctx = (ctx == ring->default_context);
+ const bool need_to_pin_ctx = (is_global_default_ctx ||
+ (intel_vgpu_active(ring->dev)));
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj;
@@ -2374,7 +2377,7 @@ int intel_lr_context_deferred_create(struct intel_context
*ctx,
return -ENOMEM;
}
- if (is_global_default_ctx) {
+ if (need_to_pin_ctx) {
ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
if (ret) {
@@ -2415,7 +2418,7 @@ int intel_lr_context_deferred_create(struct intel_context
*ctx,
goto error_free_rbuf;
}
- if (is_global_default_ctx) {
+ if (need_to_pin_ctx) {
ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
if (ret) {
DRM_ERROR(
@@ -2464,14 +2467,14 @@ int intel_lr_context_deferred_create(struct
intel_context *ctx,
return 0;
error:
- if (is_global_default_ctx)
+ if (need_to_pin_ctx)
intel_unpin_ringbuffer_obj(ringbuf);
error_destroy_rbuf:
intel_destroy_ringbuffer_obj(ringbuf);
error_free_rbuf:
kfree(ringbuf);
error_unpin_ctx:
- if (is_global_default_ctx)
+ if (need_to_pin_ctx)
i915_gem_object_ggtt_unpin(ctx_obj);
drm_gem_object_unreference(&ctx_obj->base);
return ret;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/7] drm/i915: Allow Broadwell guest with Intel GVT-g
I915 Broadwell guest driver is now supported to run inside a VM with Intel GVT-g Signed-off-by: Zhiyuan Lv Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/i915_vgpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c index 5eee75b..fdeb461 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.c +++ b/drivers/gpu/drm/i915/i915_vgpu.c @@ -66,7 +66,7 @@ void i915_check_vgpu(struct drm_device *dev) BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE); - if (!IS_HASWELL(dev)) + if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) return; magic = readq(dev_priv->regs + vgtif_reg(magic)); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
Broadwell hardware supports both ring buffer mode and execlist mode. When i915 runs inside a VM with Intel GVT-g, we allow execlist mode only. The reason is that GVT-g does not support the dynamic mode switch between ring buffer mode and execlist mode when running multiple virtual machines. Consider that ring buffer mode is legacy mode, it makes sense to drop it inside virtual machines. Signed-off-by: Zhiyuan Lv Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/intel_lrc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 2dc8709..39df304 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -239,6 +239,9 @@ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists if (INTEL_INFO(dev)->gen >= 9) return 1; + if (HAS_LOGICAL_RING_CONTEXTS(dev) && intel_vgpu_active(dev)) + return 1; + if (enable_execlists == 0) return 0; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/7] drm/i915: guest i915 notification for Intel-GVTg
When i915 drivers run inside a VM with Intel-GVTg, some explicit
notifications are needed from guest to host device model through PV
INFO page write. The notifications include:
PPGTT create/destroy
EXECLIST create/destroy
They are used for the shadow implementation of PPGTT and EXECLIST
context. Intel GVT-g needs to write-protect the guest pages of PPGTT
and contexts, and clear the write protection when they end their life
cycle.
Signed-off-by: Zhiyuan Lv
Signed-off-by: Zhi Wang
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 41 +
drivers/gpu/drm/i915/intel_lrc.c| 25 ++
2 files changed, 66 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 823005c..00dafb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -896,6 +896,41 @@ static int gen8_init_scratch(struct i915_address_space *vm)
return 0;
}
+static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
+{
+ enum vgt_g2v_type msg;
+ struct drm_device *dev = ppgtt->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned int offset = vgtif_reg(pdp0_lo);
+ int i;
+
+ if (USES_FULL_48BIT_PPGTT(dev)) {
+ u64 daddr = px_dma(&ppgtt->pml4);
+
+ I915_WRITE(offset, daddr & 0x);
+ I915_WRITE(offset + 4, daddr >> 32);
+
+ msg = (create ? VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE :
+ VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY);
+ } else {
+ for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
+ u64 daddr = i915_page_dir_dma_addr(ppgtt, i);
+
+ I915_WRITE(offset, daddr & 0x);
+ I915_WRITE(offset + 4, daddr >> 32);
+
+ offset += 8;
+ }
+
+ msg = (create ? VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE :
+ VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY);
+ }
+
+ I915_WRITE(vgtif_reg(g2v_notify), msg);
+
+ return 0;
+}
+
static void gen8_free_scratch(struct i915_address_space *vm)
{
struct drm_device *dev = vm->dev;
@@ -942,6 +977,9 @@ static void gen8_ppgtt_cleanup(struct i915_address_space
*vm)
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
+ if (intel_vgpu_active(vm->dev))
+ gen8_ppgtt_notify_vgt(ppgtt, false);
+
if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev))
gen8_ppgtt_cleanup_3lvl(ppgtt->base.dev, &ppgtt->pdp);
else
@@ -1516,6 +1554,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
}
}
+ if (intel_vgpu_active(ppgtt->base.dev))
+ gen8_ppgtt_notify_vgt(ppgtt, true);
+
return 0;
free_scratch:
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4b2ac37..80d424b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -136,6 +136,7 @@
#include
#include "i915_drv.h"
#include "intel_mocs.h"
+#include "i915_vgpu.h"
#define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
#define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
@@ -2122,6 +2123,22 @@ make_rpcs(struct drm_device *dev)
return rpcs;
}
+static void intel_lr_context_notify_vgt(struct intel_context *ctx,
+ struct intel_engine_cs *ring,
+ int msg)
+{
+ struct drm_device *dev = ring->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u64 tmp = intel_lr_context_descriptor(ctx, ring);
+
+ I915_WRITE(vgtif_reg(execlist_context_descriptor_lo),
+ tmp & 0x);
+ I915_WRITE(vgtif_reg(execlist_context_descriptor_hi),
+ tmp >> 32);
+
+ I915_WRITE(vgtif_reg(g2v_notify), msg);
+}
+
static int
populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object
*ctx_obj,
struct intel_engine_cs *ring, struct intel_ringbuffer
*ringbuf)
@@ -2282,6 +2299,10 @@ void intel_lr_context_free(struct intel_context *ctx)
ctx->engine[i].ringbuf;
struct intel_engine_cs *ring = ringbuf->ring;
+ if (intel_vgpu_active(ringbuf->ring->dev))
+ intel_lr_context_notify_vgt(ctx, ring,
+ VGT_G2V_EXECLIST_CONTEXT_DESTROY);
+
if ((ctx == ring->default_context) ||
(intel_vgpu_active(ring->dev))) {
intel_unpin_ringbuffer_obj(ringbuf);
@@ -2439,6 +2460,10 @@ int intel_lr_context_deferred_create(struct
intel_context *ctx,
ctx->engine[ring->id].ringbuf = ringbuf;
Re: [Intel-gfx] [PATCH 0/7] drm/intel: guest i915 changes for Broadwell to run inside VM with Intel GVT-g
Hi Jani, On Thu, Aug 20, 2015 at 09:44:08AM +0300, Jani Nikula wrote: > On Thu, 20 Aug 2015, Zhiyuan Lv wrote: > > I915 kernel driver can now work inside a virtual machine on Haswell > > with Intel GVT-g. In order to do the same thing on Broadwell, there > > are some extra changes needed. The two main things are to support the > > more complicated PPGTT page table structure and EXECLIST contexts. > > GVT-g will perform shadow PPGTT and shadow context, which requires > > guest driver to explicitly notify host device model the life cycle of > > PPGTT and EXECLIST contexts. > > > > The first and the forth patches added some restrictions to drivers in > > virtualization scenario to make the shadow work easier. The first > > patch is based on Mika's earlier one, but we use it for vgpu only. > > The sixth patch is the implementation of the notification for > > shadowing. > > Please send your patches with git send-email, or otherwise ensure the > patches are in-reply-to the cover letter to keep review in one thread. Thanks for the comments! I just re-sent the patchset, and these ones can be ignored. Sorry for the inconvenience caused! Regards, -Zhiyuan > > BR, > Jani. > > > > > > Zhiyuan Lv (7): > > drm/i915: preallocate pdps for 32 bit vgpu > > drm/i915: Enable full ppgtt for vgpu > > drm/i915: Always enable execlists on BDW for vgpu > > drm/i915: always pin lrc context for vgpu with Intel GVT-g > > drm/i915: Update PV INFO page definition for Intel GVT-g > > drm/i915: guest i915 notification for Intel-GVTg > > drm/i915: Allow Broadwell guest with Intel GVT-g > > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 77 > > +++-- > > drivers/gpu/drm/i915/i915_vgpu.c| 2 +- > > drivers/gpu/drm/i915/i915_vgpu.h| 34 +++- > > drivers/gpu/drm/i915/intel_lrc.c| 44 ++--- > > 4 files changed, 145 insertions(+), 12 deletions(-) > > > > -- > > 1.9.1 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/7] drm/i915: make g4x_digital_port_connected return boolean status
>-Original Message-
>From: Nikula, Jani
>Sent: Thursday, August 20, 2015 1:18 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: R, Durgadoss; Ville Syrjälä; Nikula, Jani
>Subject: [PATCH v2 2/7] drm/i915: make g4x_digital_port_connected return
>boolean status
>
>We should not be hitting any of the default cases in
>g4x_digital_port_connected, so add MISSING_CASE annotation and return
>boolean status. The current behaviour is just cargo culting from the
>days of yonder when the display port support was added to i915.
>
Reviewed-by: Durgadoss R
Thanks,
Durga
>Signed-off-by: Jani Nikula
>---
> drivers/gpu/drm/i915/intel_dp.c | 26 +++---
> 1 file changed, 11 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index 4aa3d664765b..d35dc0711f9b 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -4518,14 +4518,14 @@ static bool ibx_digital_port_connected(struct
>drm_i915_private *dev_priv,
> return I915_READ(SDEISR) & bit;
> }
>
>-static int g4x_digital_port_connected(struct drm_device *dev,
>- struct intel_digital_port
>*intel_dig_port)
>+static bool g4x_digital_port_connected(struct drm_device *dev,
>+ struct intel_digital_port *port)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> uint32_t bit;
>
> if (IS_VALLEYVIEW(dev)) {
>- switch (intel_dig_port->port) {
>+ switch (port->port) {
> case PORT_B:
> bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
> break;
>@@ -4536,10 +4536,11 @@ static int g4x_digital_port_connected(struct
>drm_device *dev,
> bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
> break;
> default:
>- return -EINVAL;
>+ MISSING_CASE(port->port);
>+ return false;
> }
> } else {
>- switch (intel_dig_port->port) {
>+ switch (port->port) {
> case PORT_B:
> bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
> break;
>@@ -4550,13 +4551,12 @@ static int g4x_digital_port_connected(struct
>drm_device *dev,
> bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
> break;
> default:
>- return -EINVAL;
>+ MISSING_CASE(port->port);
>+ return false;
> }
> }
>
>- if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
>- return 0;
>- return 1;
>+ return I915_READ(PORT_HOTPLUG_STAT) & bit;
> }
>
> static enum drm_connector_status
>@@ -4577,7 +4577,6 @@ g4x_dp_detect(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>- int ret;
>
> /* Can't disconnect eDP, but you can close the lid... */
> if (is_edp(intel_dp)) {
>@@ -4589,10 +4588,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
> return status;
> }
>
>- ret = g4x_digital_port_connected(dev, intel_dig_port);
>- if (ret == -EINVAL)
>- return connector_status_unknown;
>- else if (ret == 0)
>+ if (!g4x_digital_port_connected(dev, intel_dig_port))
> return connector_status_disconnected;
>
> return intel_dp_detect_dpcd(intel_dp);
>@@ -5059,7 +5055,7 @@ intel_dp_hpd_pulse(struct intel_digital_port
>*intel_dig_port, bool long_hpd)
> if (!ibx_digital_port_connected(dev_priv,
> intel_dig_port))
> goto mst_fail;
> } else {
>- if (g4x_digital_port_connected(dev, intel_dig_port) !=
>1)
>+ if (!g4x_digital_port_connected(dev, intel_dig_port))
> goto mst_fail;
> }
>
>--
>2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, one more batch of i915 fixes for v4.2. Revert of a VBT parsing commit that should've been queued for drm-next, not v4.2. The revert unbreaks Braswell among other things. Also on Braswell removal of DP HBR2/TP3 and intermediate eDP frequency support. The code was optimistically added based on incorrect documentation; the platform does not support them. These are cc: stable. Finally a gpu state fix from Chris, also cc: stable. BR, Jani. The following changes since commit d2944cf21305c754fa8b2d6c1eea05ad5dad7944: drm/i915: Commit planes on each crtc separately. (2015-08-13 12:09:18 +0300) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-08-20 for you to fetch changes up to ed63baaf849e91c84ac3e042b1fd6a0af07c16f3: drm/i915: Avoid TP3 on CHV (2015-08-19 11:13:59 +0300) Chris Wilson (1): drm/i915: Flag the execlists context object as dirty after every use Jani Nikula (1): Revert "drm/i915: Allow parsing of variable size child device entries from VBT" Thulasimani,Sivakumar (3): Revert "drm/i915: Add eDP intermediate frequencies for CHV" drm/i915: remove HBR2 from chv supported list drm/i915: Avoid TP3 on CHV drivers/gpu/drm/i915/intel_bios.c | 27 --- drivers/gpu/drm/i915/intel_dp.c | 35 ++- drivers/gpu/drm/i915/intel_lrc.c | 2 ++ 3 files changed, 28 insertions(+), 36 deletions(-) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: > Broadwell hardware supports both ring buffer mode and execlist mode. > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode > only. The reason is that GVT-g does not support the dynamic mode > switch between ring buffer mode and execlist mode when running > multiple virtual machines. Consider that ring buffer mode is legacy > mode, it makes sense to drop it inside virtual machines. If that is the case, you should query the host as to what mode it is running. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g
On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: > Intel GVT-g will perform EXECLIST context shadowing and ring buffer > shadowing. The shadow copy is created when guest creates a context. > If a context changes its LRCA address, the hypervisor is hard to know > whether it is a new context or not. We always pin context objects to > global GTT to make life easier. Nak. Please explain why we need to workaround a bug in the host. We cannot pin the context as that breaks userspace (e.g. synmark) who can and will try to use more contexts than we have room. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
Hi Chris, On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote: > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: > > Broadwell hardware supports both ring buffer mode and execlist mode. > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode > > only. The reason is that GVT-g does not support the dynamic mode > > switch between ring buffer mode and execlist mode when running > > multiple virtual machines. Consider that ring buffer mode is legacy > > mode, it makes sense to drop it inside virtual machines. > > If that is the case, you should query the host as to what mode it is > running. Thanks for the reply! You mean we query the host mode, then tell the guest driver inside VM, so that it could use the same mode as host right? That might be a little complicated, and the only benefit is to support legacy ring buffer mode ... And GVT-g host implementation was not well tested with ring buffer mode since it cannot start windows VM. Probably I should change the commit message to say explicitly that ring buffer mode is not supported with GVT-g? Thanks! Regards, -Zhiyuan > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote: > Hi Chris, > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote: > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: > > > Broadwell hardware supports both ring buffer mode and execlist mode. > > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode > > > only. The reason is that GVT-g does not support the dynamic mode > > > switch between ring buffer mode and execlist mode when running > > > multiple virtual machines. Consider that ring buffer mode is legacy > > > mode, it makes sense to drop it inside virtual machines. > > > > If that is the case, you should query the host as to what mode it is > > running. > > Thanks for the reply! You mean we query the host mode, then tell the > guest driver inside VM, so that it could use the same mode as host > right? That might be a little complicated, and the only benefit is to > support legacy ring buffer mode ... The only benefit being that the guest works no matter what the host does? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g
Hi Chris, On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote: > On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: > > Intel GVT-g will perform EXECLIST context shadowing and ring buffer > > shadowing. The shadow copy is created when guest creates a context. > > If a context changes its LRCA address, the hypervisor is hard to know > > whether it is a new context or not. We always pin context objects to > > global GTT to make life easier. > > Nak. Please explain why we need to workaround a bug in the host. We > cannot pin the context as that breaks userspace (e.g. synmark) who can > and will try to use more contexts than we have room. This change is only for i915 running inside a VM. Host i915 driver's behavior will not be impacted. The reason we want to pin context is that our hypervisor identifies guest contexts with their LRCA, and need LRCA unchanged during contexts' life cycle so that shadow copy of contexts can easily find their counterparts. If we cannot pin them, we have to consider more complicated shadow implementation ... BTW Chris, are there many apps like synmark that may used up GGTT with contexts if they are all pinned? Thanks! Regards, -Zhiyuan > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events
On Wed, 19 Aug 2015 10:48:58 +0200,
David Henningsson wrote:
>
> Whenever there is an event from the i915 driver, wake the codec
> and recheck plug/unplug + ELD status.
>
> This fixes the issue with lost unsol events in power save mode,
> the codec and controller can now sleep in D3 and still know when
> the HDMI monitor has been connected.
>
> Signed-off-by: David Henningsson
This addition looks fine, but then we'll get double notification for
the normal hotplug/unplug, one via component ops and another via unsol
event?
thanks,
Takashi
> ---
> sound/pci/hda/patch_hdmi.c | 22 +-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index a97db5f..932292c 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -37,6 +37,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include "hda_codec.h"
> #include "hda_local.h"
> #include "hda_jack.h"
> @@ -144,6 +146,9 @@ struct hdmi_spec {
>*/
> struct hda_multi_out multiout;
> struct hda_pcm_stream pcm_playback;
> +
> + /* i915/powerwell (Haswell+/Valleyview+) specific */
> + struct i915_audio_component_audio_ops i915_audio_ops;
> };
>
>
> @@ -2191,6 +2196,9 @@ static void generic_hdmi_free(struct hda_codec *codec)
> struct hdmi_spec *spec = codec->spec;
> int pin_idx;
>
> + if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> + snd_hdac_i915_register_notifier(NULL);
> +
> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
>
> @@ -2316,6 +2324,14 @@ static void haswell_set_power_state(struct hda_codec
> *codec, hda_nid_t fg,
> snd_hda_codec_set_power_to_all(codec, fg, power_state);
> }
>
> +static void intel_pin_eld_notify(void *audio_ptr, int port, int
> port_mst_index)
> +{
> + struct hda_codec *codec = audio_ptr;
> + int pin_nid = port + 0x04;
> +
> + check_presence_and_report(codec, pin_nid);
> +}
> +
> static int patch_generic_hdmi(struct hda_codec *codec)
> {
> struct hdmi_spec *spec;
> @@ -2342,8 +2358,12 @@ static int patch_generic_hdmi(struct hda_codec *codec)
> if (is_valleyview_plus(codec) || is_skylake(codec))
> codec->core.link_power_control = 1;
>
> - if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> + if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
> codec->depop_delay = 0;
> + spec->i915_audio_ops.audio_ptr = codec;
> + spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
> + snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
> + }
>
> if (hdmi_parse_codec(codec) < 0) {
> codec->spec = NULL;
> --
> 1.9.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events
On 2015-08-20 11:28, Takashi Iwai wrote:
On Wed, 19 Aug 2015 10:48:58 +0200,
David Henningsson wrote:
Whenever there is an event from the i915 driver, wake the codec
and recheck plug/unplug + ELD status.
This fixes the issue with lost unsol events in power save mode,
the codec and controller can now sleep in D3 and still know when
the HDMI monitor has been connected.
Signed-off-by: David Henningsson
This addition looks fine, but then we'll get double notification for
the normal hotplug/unplug, one via component ops and another via unsol
event?
Right, in case the unsol event actually works...
I would argue that the normal case would be that the controller and
codec is in D3 which means that the unsol event never gets through - due
to hw limitations - which is what triggered this patch set in the first
place.
But yes, in some case we might get double notification, but this should
not cause any trouble in practice. The unsol event could be turned off,
but would it be okay to save that for a later patch set (so I don't miss
the upcoming merge window)?
thanks,
Takashi
---
sound/pci/hda/patch_hdmi.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index a97db5f..932292c 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -37,6 +37,8 @@
#include
#include
#include
+#include
+#include
#include "hda_codec.h"
#include "hda_local.h"
#include "hda_jack.h"
@@ -144,6 +146,9 @@ struct hdmi_spec {
*/
struct hda_multi_out multiout;
struct hda_pcm_stream pcm_playback;
+
+ /* i915/powerwell (Haswell+/Valleyview+) specific */
+ struct i915_audio_component_audio_ops i915_audio_ops;
};
@@ -2191,6 +2196,9 @@ static void generic_hdmi_free(struct hda_codec *codec)
struct hdmi_spec *spec = codec->spec;
int pin_idx;
+ if (is_haswell_plus(codec) || is_valleyview_plus(codec))
+ snd_hdac_i915_register_notifier(NULL);
+
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
@@ -2316,6 +2324,14 @@ static void haswell_set_power_state(struct hda_codec
*codec, hda_nid_t fg,
snd_hda_codec_set_power_to_all(codec, fg, power_state);
}
+static void intel_pin_eld_notify(void *audio_ptr, int port, int port_mst_index)
+{
+ struct hda_codec *codec = audio_ptr;
+ int pin_nid = port + 0x04;
+
+ check_presence_and_report(codec, pin_nid);
+}
+
static int patch_generic_hdmi(struct hda_codec *codec)
{
struct hdmi_spec *spec;
@@ -2342,8 +2358,12 @@ static int patch_generic_hdmi(struct hda_codec *codec)
if (is_valleyview_plus(codec) || is_skylake(codec))
codec->core.link_power_control = 1;
- if (is_haswell_plus(codec) || is_valleyview_plus(codec))
+ if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
codec->depop_delay = 0;
+ spec->i915_audio_ops.audio_ptr = codec;
+ spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
+ snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
+ }
if (hdmi_parse_codec(codec) < 0) {
codec->spec = NULL;
--
1.9.1
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] kms_universal_plane: subtest for yuv pixel format in primary plane
This test commit YUV framebuffer in primay plane. I'm using empty FB,
because this fulfills the purpose of testing YUV.
V2: Revert chnages for MAX_PLANE count as per nabendu's comment. These
changes are already floating for review.
Signed-off-by: Kumar, Mahesh
---
lib/igt_fb.c| 4 +++
tests/kms_universal_plane.c | 82 +
2 files changed, 86 insertions(+)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 134dbd2..e570bfa 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -66,6 +66,10 @@ static struct format_desc_struct {
DF(XRGB,RGB24, 32, 24),
DF(XRGB2101010, RGB30, 32, 30),
DF(ARGB,ARGB32, 32, 32),
+ DF(YUYV,INVALID,16, 16),
+ DF(YVYU,INVALID,16, 16),
+ DF(UYVY,INVALID,16, 16),
+ DF(VYUY,INVALID,16, 16),
};
#undef DF
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index 635cc79..ed4b134 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -57,6 +57,12 @@ typedef struct {
struct igt_fb red_fb, blue_fb;
} pageflip_test_t;
+typedef struct {
+ data_t *data;
+ struct igt_fb fullsize_fb, undersize_fb;
+} primary_yuv_test_t;
+
+
static void
functional_test_init(functional_test_t *test, igt_output_t *output, enum pipe
pipe)
{
@@ -430,6 +436,77 @@ sanity_test_pipe(data_t *data, enum pipe pipe,
igt_output_t *output)
}
static void
+primary_yuv_test_init(primary_yuv_test_t *test, igt_output_t *output, enum
pipe pipe)
+{
+ data_t *data = test->data;
+ drmModeModeInfo *mode;
+
+ igt_output_set_pipe(output, pipe);
+
+ mode = igt_output_get_mode(output);
+ igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_YUYV,
+ LOCAL_DRM_FORMAT_MOD_NONE,
+ &test->fullsize_fb);
+ igt_create_fb(data->drm_fd, 300, 300,
+ DRM_FORMAT_YVYU,
+ LOCAL_DRM_FORMAT_MOD_NONE,
+ &test->undersize_fb);
+}
+
+static void
+primary_yuv_test_fini(primary_yuv_test_t *test, igt_output_t *output)
+{
+ igt_remove_fb(test->data->drm_fd, &test->fullsize_fb);
+ igt_remove_fb(test->data->drm_fd, &test->undersize_fb);
+
+ igt_output_set_pipe(output, PIPE_ANY);
+ igt_display_commit2(&test->data->display, COMMIT_LEGACY);
+}
+
+/*
+ * YUV pixel format test for primary plane
+ * Display Full frame in primay then 300x300 frame
+ */
+
+static void
+primary_yuv_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+ primary_yuv_test_t test = { .data = data };
+ igt_plane_t *primary;
+
+ igt_skip_on(pipe >= data->display.n_pipes);
+
+ igt_output_set_pipe(output, pipe);
+
+ primary_yuv_test_init(&test, output, pipe);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+
+ /* Use legacy API to set a mode with a Fullsize FB */
+ igt_plane_set_fb(primary, &test.fullsize_fb);
+ igt_display_commit2(&data->display, COMMIT_LEGACY);
+
+ /* Disable the primary plane */
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit2(&data->display, COMMIT_UNIVERSAL);
+
+ /* Use Universal API to set a mode with a Fullsize FB */
+ igt_plane_set_fb(primary, &test.fullsize_fb);
+ igt_display_commit2(&data->display, COMMIT_UNIVERSAL);
+
+ /* Use Universal API to set a mode with a Undersize FB */
+ igt_plane_set_fb(primary, &test.undersize_fb);
+ igt_display_commit2(&data->display, COMMIT_UNIVERSAL);
+
+ /* Disable the primary plane */
+ igt_plane_set_fb(primary, NULL);
+
+ primary_yuv_test_fini(&test, output);
+
+}
+
+static void
pageflip_test_init(pageflip_test_t *test, igt_output_t *output, enum pipe pipe)
{
data_t *data = test->data;
@@ -663,6 +740,11 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
kmstest_pipe_name(pipe))
for_each_connected_output(&data->display, output)
cursor_leak_test_pipe(data, pipe, output);
+
+ igt_subtest_f("primary-plane-pipe-%s-yuv",
+ kmstest_pipe_name(pipe))
+ for_each_connected_output(&data->display, output)
+ primary_yuv_test_pipe(data, pipe, output);
}
static data_t data;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events
On Thu, 20 Aug 2015 11:41:42 +0200,
David Henningsson wrote:
>
>
>
> On 2015-08-20 11:28, Takashi Iwai wrote:
> > On Wed, 19 Aug 2015 10:48:58 +0200,
> > David Henningsson wrote:
> >>
> >> Whenever there is an event from the i915 driver, wake the codec
> >> and recheck plug/unplug + ELD status.
> >>
> >> This fixes the issue with lost unsol events in power save mode,
> >> the codec and controller can now sleep in D3 and still know when
> >> the HDMI monitor has been connected.
> >>
> >> Signed-off-by: David Henningsson
> >
> > This addition looks fine, but then we'll get double notification for
> > the normal hotplug/unplug, one via component ops and another via unsol
> > event?
>
> Right, in case the unsol event actually works...
>
> I would argue that the normal case would be that the controller and
> codec is in D3 which means that the unsol event never gets through - due
> to hw limitations - which is what triggered this patch set in the first
> place.
>
> But yes, in some case we might get double notification, but this should
> not cause any trouble in practice. The unsol event could be turned off,
> but would it be okay to save that for a later patch set (so I don't miss
> the upcoming merge window)?
In that case, it should be mentioned in the changelog at least.
This series came a bit too late for the merge window, so I'm not sure
whether this can get in. I personally find it OK, so take my ack for
ALSA parts (patch 3/4), but the rest need review and ack from i915
guys. And we don't know who to merge this, if any. The changes are
almost even to i915 and hda. I don't mind either way, via drm or
sound tree.
In anyway,
Reviewed-by: Takashi Iwai
thanks,
Takashi
> >
> >
> > thanks,
> >
> > Takashi
> >
> >> ---
> >> sound/pci/hda/patch_hdmi.c | 22 +-
> >> 1 file changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> >> index a97db5f..932292c 100644
> >> --- a/sound/pci/hda/patch_hdmi.c
> >> +++ b/sound/pci/hda/patch_hdmi.c
> >> @@ -37,6 +37,8 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> +#include
> >> #include "hda_codec.h"
> >> #include "hda_local.h"
> >> #include "hda_jack.h"
> >> @@ -144,6 +146,9 @@ struct hdmi_spec {
> >> */
> >>struct hda_multi_out multiout;
> >>struct hda_pcm_stream pcm_playback;
> >> +
> >> + /* i915/powerwell (Haswell+/Valleyview+) specific */
> >> + struct i915_audio_component_audio_ops i915_audio_ops;
> >> };
> >>
> >>
> >> @@ -2191,6 +2196,9 @@ static void generic_hdmi_free(struct hda_codec
> >> *codec)
> >>struct hdmi_spec *spec = codec->spec;
> >>int pin_idx;
> >>
> >> + if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> >> + snd_hdac_i915_register_notifier(NULL);
> >> +
> >>for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> >>struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> >>
> >> @@ -2316,6 +2324,14 @@ static void haswell_set_power_state(struct
> >> hda_codec *codec, hda_nid_t fg,
> >>snd_hda_codec_set_power_to_all(codec, fg, power_state);
> >> }
> >>
> >> +static void intel_pin_eld_notify(void *audio_ptr, int port, int
> >> port_mst_index)
> >> +{
> >> + struct hda_codec *codec = audio_ptr;
> >> + int pin_nid = port + 0x04;
> >> +
> >> + check_presence_and_report(codec, pin_nid);
> >> +}
> >> +
> >> static int patch_generic_hdmi(struct hda_codec *codec)
> >> {
> >>struct hdmi_spec *spec;
> >> @@ -2342,8 +2358,12 @@ static int patch_generic_hdmi(struct hda_codec
> >> *codec)
> >>if (is_valleyview_plus(codec) || is_skylake(codec))
> >>codec->core.link_power_control = 1;
> >>
> >> - if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> >> + if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
> >>codec->depop_delay = 0;
> >> + spec->i915_audio_ops.audio_ptr = codec;
> >> + spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
> >> + snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
> >> + }
> >>
> >>if (hdmi_parse_codec(codec) < 0) {
> >>codec->spec = NULL;
> >> --
> >> 1.9.1
> >>
> >
>
> --
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote: > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote: > > Hi Chris, > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote: > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: > > > > Broadwell hardware supports both ring buffer mode and execlist mode. > > > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode > > > > only. The reason is that GVT-g does not support the dynamic mode > > > > switch between ring buffer mode and execlist mode when running > > > > multiple virtual machines. Consider that ring buffer mode is legacy > > > > mode, it makes sense to drop it inside virtual machines. > > > > > > If that is the case, you should query the host as to what mode it is > > > running. > > > > Thanks for the reply! You mean we query the host mode, then tell the > > guest driver inside VM, so that it could use the same mode as host > > right? That might be a little complicated, and the only benefit is to > > support legacy ring buffer mode ... > > The only benefit being that the guest works no matter what the host > does? Supporting ring buffer mode may need more work in GVT-g. When we started to enable BDW support, ring buffer mode used to work but was not well tested. And the inter-VM switch (all in ringbuffer mode) may be tricker comparing with driver context switch with "MI_SET_CONTEXT", because we need to switch ring buffer whereas driver does not. Regarding this, the EXECLIST mode looks cleaner. In order to support that, we may have to: 1, change more LRI commands to MMIO in current driver; 2, more testing/debugging of inter-VM context switch flow. Based on that, I think we should really make statement that "ring buffer mode" is not supported by GVT-g on BDW :-) > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: preallocate pdps for 32 bit vgpu
Hi,
Added Michel and Dave as CC too, to notice this, as they are specified
in the patch as CC.
On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> This is based on Mika Kuoppala's patch below:
> http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/61
> 104/match=workaround+hw+preload
>
> The patch will preallocate the page directories for 32-bit PPGTT when
> i915 runs inside a virtual machine with Intel GVT-g. With this
> change,
> the root pointers in EXECLIST context will always keep the same.
>
> The change is needed for vGPU because Intel GVT-g will do page table
> shadowing, and needs to track all the page table changes from guest
> i915 driver. However, if guest PPGTT is modified through GPU commands
> like LRI, it is not possible to trap the operations in the right
> time,
> so it will be hard to make shadow PPGTT to work correctly.
>
> Shadow PPGTT could be much simpler with this change. Meanwhile
> hypervisor could simply prohibit any attempt of PPGTT modification
> through GPU command for security.
>
> The function gen8_preallocate_top_level_pdps() in the patch is from
> Mika, with only one change to set "used_pdpes" to avoid duplicated
> allocation later.
>
> Cc: Mika Kuoppala
> Cc: Dave Gordon
> Cc: Michel Thierry
> Signed-off-by: Zhiyuan Lv
> Signed-off-by: Zhi Wang
>
Reviewed-by: Joonas Lahtinen
I'm just wondering if it's worth keeping the LRI method of updating the
PDPS at all, for the sake of a couple of KBs per PPGTT, now that we
have an occasional need for making them static. So this patch is R-b:d,
but I'd suggest discussion about removing the LRI update method, and
favoring static PDPS always for 32-bit.
Regards, Joonas
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 33
> +
> drivers/gpu/drm/i915/intel_lrc.c| 3 ++-
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4a76807..ed10e77 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1441,6 +1441,33 @@ static void gen8_dump_ppgtt(struct
> i915_hw_ppgtt *ppgtt, struct seq_file *m)
> }
> }
>
> +static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt
> *ppgtt)
> +{
> + unsigned long *new_page_dirs, **new_page_tables;
> + uint32_t pdpes = I915_PDPES_PER_PDP(dev);
> + int ret;
> +
> + /* We allocate temp bitmap for page tables for no gain
> + * but as this is for init only, lets keep the things simple
> + */
> + ret = alloc_gen8_temp_bitmaps(&new_page_dirs,
> &new_page_tables, pdpes);
> + if (ret)
> + return ret;
> +
> + /* Allocate for all pdps regardless of how the ppgtt
> + * was defined.
> + */
> + ret = gen8_ppgtt_alloc_page_directories(&ppgtt->base, &ppgtt
> ->pdp,
> + 0, 1ULL << 32,
> + new_page_dirs);
> + if (!ret)
> + *ppgtt->pdp.used_pdpes = *new_page_dirs;
> +
> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables,
> pdpes);
> +
> + return ret;
> +}
> +
> /*
> * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP
> registers
> * with a net effect resembling a 2-level page table in normal x86
> terms. Each
> @@ -1484,6 +1511,12 @@ static int gen8_ppgtt_init(struct
> i915_hw_ppgtt *ppgtt)
> trace_i915_page_directory_pointer_entry_alloc(&ppgtt
> ->base,
> 0, 0,
>
> GEN8_PML4E_SHIFT);
> +
> + if (intel_vgpu_active(ppgtt->base.dev)) {
> + ret =
> gen8_preallocate_top_level_pdps(ppgtt);
> + if (ret)
> + goto free_scratch;
> + }
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index e77b6b0..2dc8709 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1540,7 +1540,8 @@ static int gen8_emit_bb_start(struct
> drm_i915_gem_request *req,
>* not needed in 48-bit.*/
> if (req->ctx->ppgtt &&
> (intel_ring_flag(req->ring) & req->ctx->ppgtt
> ->pd_dirty_rings)) {
> - if (!USES_FULL_48BIT_PPGTT(req->i915)) {
> + if (!USES_FULL_48BIT_PPGTT(req->i915) &&
> + !intel_vgpu_active(req->i915->dev)) {
> ret = intel_logical_ring_emit_pdps(req);
> if (ret)
> return ret;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Enable full ppgtt for vgpu
On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote: > The full ppgtt is supported in Intel GVT-g device model. So the > restriction can be removed. > > Signed-off-by: Zhiyuan Lv > Signed-off-by: Zhi Wang Reviewed-by: Joonas Lahtinen > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index ed10e77..823005c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct > drm_device *dev, int enable_ppgtt) > has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6; > has_full_ppgtt = INTEL_INFO(dev)->gen >= 7; > > - if (intel_vgpu_active(dev)) > - has_full_ppgtt = false; /* emulation is too hard */ > - > /* >* We don't allow disabling PPGTT for gen9+ as it's a > requirement for >* execlists, the sole mechanism available to submit work. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
Hi, On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote: > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote: > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote: > > > Hi Chris, > > > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote: > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: > > > > > Broadwell hardware supports both ring buffer mode and > > > > > execlist mode. > > > > > When i915 runs inside a VM with Intel GVT-g, we allow > > > > > execlist mode > > > > > only. The reason is that GVT-g does not support the dynamic > > > > > mode > > > > > switch between ring buffer mode and execlist mode when > > > > > running > > > > > multiple virtual machines. Consider that ring buffer mode is > > > > > legacy > > > > > mode, it makes sense to drop it inside virtual machines. > > > > > > > > If that is the case, you should query the host as to what mode > > > > it is > > > > running. > > > > > > Thanks for the reply! You mean we query the host mode, then tell > > > the > > > guest driver inside VM, so that it could use the same mode as > > > host > > > right? That might be a little complicated, and the only benefit > > > is to > > > support legacy ring buffer mode ... > > > > The only benefit being that the guest works no matter what the host > > does? > > Supporting ring buffer mode may need more work in GVT-g. When we > started to > enable BDW support, ring buffer mode used to work but was not well > tested. And > the inter-VM switch (all in ringbuffer mode) may be tricker comparing > with > driver context switch with "MI_SET_CONTEXT", because we need to > switch ring > buffer whereas driver does not. Regarding this, the EXECLIST mode > looks > cleaner. In order to support that, we may have to: 1, change more LRI > commands > to MMIO in current driver; 2, more testing/debugging of inter-VM > context > switch flow. > > Based on that, I think we should really make statement that "ring > buffer mode" > is not supported by GVT-g on BDW :-) > I think just move the vpgu test even before test for GEN9 just making it return true on intel_vgpu_active(dev) and add a comment that currently vGPU only supports execlist command submission, and then add an error early in the init in some appropriate spot if intel_vgpu_active is true but logical ring context are not available (which would practically mean GEN < 8). Does that sound OK? Regards, Joonas > > -Chris > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Update PV INFO page definition for Intel GVT-g
On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> Some more definitions in the PV info page are added. They are mainly
> for the guest notification to Intel GVT-g device model. They are used
> for Broadwell enabling.
>
> Signed-off-by: Zhiyuan Lv
> Signed-off-by: Zhi Wang
>
Reviewed-by: Joonas Lahtinen
Is there any public document about the interface?
> ---
> drivers/gpu/drm/i915/i915_vgpu.h | 34
> --
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h
> b/drivers/gpu/drm/i915/i915_vgpu.h
> index 97a88b5..21c97f4 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -40,6 +40,19 @@
> #define INTEL_VGT_IF_VERSION \
> INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR,
> VGT_VERSION_MINOR)
>
> +/*
> + * notifications from guest to vgpu device model
> + */
> +enum vgt_g2v_type {
> + VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> + VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> + VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> + VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> + VGT_G2V_EXECLIST_CONTEXT_CREATE,
> + VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> + VGT_G2V_MAX,
> +};
> +
> struct vgt_if {
> uint64_t magic; /* VGT_MAGIC */
> uint16_t version_major;
> @@ -70,11 +83,28 @@ struct vgt_if {
> uint32_t rsv3[0x200 - 24]; /* pad to half page */
> /*
>* The bottom half page is for response from Gfx driver to
> hypervisor.
> - * Set to reserved fields temporarily by now.
>*/
> uint32_t rsv4;
> uint32_t display_ready; /* ready for display owner
> switch */
> - uint32_t rsv5[0x200 - 2]; /* pad to one page */
> +
> + uint32_t rsv5[4];
> +
> + uint32_t g2v_notify;
> + uint32_t rsv6[7];
> +
> + uint32_t pdp0_lo;
> + uint32_t pdp0_hi;
> + uint32_t pdp1_lo;
> + uint32_t pdp1_hi;
> + uint32_t pdp2_lo;
> + uint32_t pdp2_hi;
> + uint32_t pdp3_lo;
> + uint32_t pdp3_hi;
> +
> + uint32_t execlist_context_descriptor_lo;
> + uint32_t execlist_context_descriptor_hi;
> +
> + uint32_t rsv7[0x200 - 24];/* pad to one page */
> } __packed;
>
> #define vgtif_reg(x) \
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: guest i915 notification for Intel-GVTg
Hi,
Notes below.
On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> When i915 drivers run inside a VM with Intel-GVTg, some explicit
> notifications are needed from guest to host device model through PV
> INFO page write. The notifications include:
>
> PPGTT create/destroy
> EXECLIST create/destroy
>
> They are used for the shadow implementation of PPGTT and EXECLIST
> context. Intel GVT-g needs to write-protect the guest pages of PPGTT
> and contexts, and clear the write protection when they end their life
> cycle.
>
> Signed-off-by: Zhiyuan Lv
> Signed-off-by: Zhi Wang
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 41
> +
> drivers/gpu/drm/i915/intel_lrc.c| 25 ++
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 823005c..00dafb0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -896,6 +896,41 @@ static int gen8_init_scratch(struct
> i915_address_space *vm)
> return 0;
> }
>
> +static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool
> create)
> +{
> + enum vgt_g2v_type msg;
> + struct drm_device *dev = ppgtt->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + unsigned int offset = vgtif_reg(pdp0_lo);
> + int i;
> +
> + if (USES_FULL_48BIT_PPGTT(dev)) {
With regards the patch "preallocate pdps for 32 bit vgpu", is this code
path ever taken?
> + u64 daddr = px_dma(&ppgtt->pml4);
> +
> + I915_WRITE(offset, daddr & 0x);
> + I915_WRITE(offset + 4, daddr >> 32);
> +
> + msg = (create ? VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE :
> + VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY)
> ;
> + } else {
> + for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
> + u64 daddr = i915_page_dir_dma_addr(ppgtt,
> i);
> +
> + I915_WRITE(offset, daddr & 0x);
> + I915_WRITE(offset + 4, daddr >> 32);
> +
> + offset += 8;
> + }
> +
> + msg = (create ? VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE :
> + VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY)
> ;
> + }
> +
> + I915_WRITE(vgtif_reg(g2v_notify), msg);
> +
> + return 0;
> +}
> +
> static void gen8_free_scratch(struct i915_address_space *vm)
> {
> struct drm_device *dev = vm->dev;
> @@ -942,6 +977,9 @@ static void gen8_ppgtt_cleanup(struct
> i915_address_space *vm)
> struct i915_hw_ppgtt *ppgtt =
> container_of(vm, struct i915_hw_ppgtt, base);
>
> + if (intel_vgpu_active(vm->dev))
> + gen8_ppgtt_notify_vgt(ppgtt, false);
> +
> if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev))
> gen8_ppgtt_cleanup_3lvl(ppgtt->base.dev, &ppgtt
> ->pdp);
> else
> @@ -1516,6 +1554,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
> *ppgtt)
> }
> }
>
> + if (intel_vgpu_active(ppgtt->base.dev))
> + gen8_ppgtt_notify_vgt(ppgtt, true);
> +
> return 0;
>
> free_scratch:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 4b2ac37..80d424b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -136,6 +136,7 @@
> #include
> #include "i915_drv.h"
> #include "intel_mocs.h"
> +#include "i915_vgpu.h"
>
> #define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
> #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
> @@ -2122,6 +2123,22 @@ make_rpcs(struct drm_device *dev)
> return rpcs;
> }
>
> +static void intel_lr_context_notify_vgt(struct intel_context *ctx,
> + struct intel_engine_cs
> *ring,
> + int msg)
> +{
> + struct drm_device *dev = ring->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u64 tmp = intel_lr_context_descriptor(ctx, ring);
> +
> + I915_WRITE(vgtif_reg(execlist_context_descriptor_lo),
> + tmp & 0x);
> + I915_WRITE(vgtif_reg(execlist_context_descriptor_hi),
> + tmp >> 32);
> +
> + I915_WRITE(vgtif_reg(g2v_notify), msg);
> +}
> +
Why the other interface has bool for action and the other msg?
Regards, Joonas
> static int
> populate_lr_context(struct intel_context *ctx, struct
> drm_i915_gem_object *ctx_obj,
> struct intel_engine_cs *ring, struct
> intel_ringbuffer *ringbuf)
> @@ -2282,6 +2299,10 @@ void intel_lr_context_free(struct
> intel_context *ctx)
> ctx->engine[i].ringbuf;
> struct intel_engine_cs *ring = ringbuf
> ->ring;
>
> + if (intel_vgpu_active(ringbuf->ring->dev))
> + intel_lr_context_notif
Re: [Intel-gfx] [PATCH 7/7] drm/i915: Allow Broadwell guest with Intel GVT-g
On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote: > I915 Broadwell guest driver is now supported to run inside a VM with > Intel GVT-g > > Signed-off-by: Zhiyuan Lv > Signed-off-by: Zhi Wang After the other relevant patches are settled; Reviewed-by: Joonas Lahtinen > --- > drivers/gpu/drm/i915/i915_vgpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c > b/drivers/gpu/drm/i915/i915_vgpu.c > index 5eee75b..fdeb461 100644 > --- a/drivers/gpu/drm/i915/i915_vgpu.c > +++ b/drivers/gpu/drm/i915/i915_vgpu.c > @@ -66,7 +66,7 @@ void i915_check_vgpu(struct drm_device *dev) > > BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE); > > - if (!IS_HASWELL(dev)) > + if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) > return; > > magic = readq(dev_priv->regs + vgtif_reg(magic)); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/16] drm/i915: disable FBC on FIFO underruns
2015-08-19 9:06 GMT-03:00 Ville Syrjälä :
> On Fri, Aug 14, 2015 at 06:34:12PM -0300, Paulo Zanoni wrote:
>> If we want to try to enable FBC by default on any platform we need to
>> be on the safe side and disable it in case we get an underrun while
>> FBC is enabled on the corresponding pipe. We currently already have
>> other reasons for FIFO underruns on our driver, but the ones I saw
>> with FBC lead to black screens that only go away when you disable FBC.
>
> We don't try to deal with underruns on other platforms either, and yes
> on some, cough, chv, cough, they can definitely blank out the screen
> until the next modeset. On even older platforms it's even worse and an
> underrun can kill the display engine until display reset/reboot :(
> Especially annoying on gen2 where we have no reset support. So I'm not
> entirely convinced FBC deserves special treatment here.
I don't understand your logic. To me, it sounds like "you're proposing
adding airbags to new cars, but that is not going to protect against
every type of accident, and it's also not going to help the cars that
are already manufactured, so I don't think front-collisions of new
cars deserve special treatment".
>
>>
>> The current FIFO underrun I see happens when the CFB is using the top
>> 8mb of stolen memory. This is reproducible with a 2560x1440 DP Monitor
>> on a system with 32mb of stolen memory. On this case, all the IGT
>> tests fail while checking the screen CRC. A later patch on this series
>> will fix this problem for real.
>>
>> With this patch, the tests will start failing while checking if FBC is
>> enabled instead of failing while comparing the CRC of the black screen
>> against the correct CRC. So this patch is not hiding any IGT bugs: the
>> tests still fail, but now they fail with a different reason.
>>
>> Signed-off-by: Paulo Zanoni
>> ---
>> drivers/gpu/drm/i915/i915_drv.h| 5 +++
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_fbc.c | 61
>> ++
>> drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +
>> 4 files changed, 69 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 4fd7858..e74a844 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -926,6 +926,11 @@ struct i915_fbc {
>> struct drm_framebuffer *fb;
>> } *fbc_work;
>>
>> + struct intel_fbc_underrun_work {
>> + struct work_struct work;
>> + struct intel_crtc *crtc;
>> + } underrun_work;
>> +
>> enum no_fbc_reason {
>> FBC_OK, /* FBC is enabled */
>> FBC_UNSUPPORTED, /* FBC is not supported by this chipset */
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 81b7d77..246925d 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1247,6 +1247,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>>unsigned int frontbuffer_bits, enum fb_op_origin origin);
>> const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>> void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
>> +void intel_fbc_handle_fifo_underrun(struct intel_crtc *crtc);
>>
>> /* intel_hdmi.c */
>> void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
>> b/drivers/gpu/drm/i915/intel_fbc.c
>> index a63d10a..28e569c 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -955,6 +955,65 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>> mutex_unlock(&dev_priv->fbc.lock);
>> }
>>
>> +static void intel_fbc_underrun_work_fn(struct work_struct *__work)
>> +{
>> + struct intel_fbc_underrun_work *work =
>> + container_of(__work, struct intel_fbc_underrun_work, work);
>> + struct intel_crtc *crtc = work->crtc;
>> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>> +
>> + mutex_lock(&dev_priv->fbc.lock);
>> + if (!intel_fbc_enabled(dev_priv) || dev_priv->fbc.crtc != crtc)
>> + goto out;
>> +
>> + DRM_DEBUG_KMS("Disabling FBC due to FIFO underrun.\n");
>> + i915.enable_fbc = 0;
>> + __intel_fbc_disable(dev_priv);
>> +
>> +out:
>> + work->crtc = NULL;
>> + mutex_unlock(&dev_priv->fbc.lock);
>> +}
>> +
>> +/**
>> + * intel_fbc_handle_fifo_underrun - handle FIFO underruns if FBC is enabled
>> + * @crtc: the CRTC that caused the underrun
>> + *
>> + * Although we can't know for sure what caused an underrun, one of the
>> possible
>> + * reasons is FBC. And on the FBC case, the user may have a black screen
>> until
>> + * FBC is disabled. So whenever a FIFO underrun happens while FBC is
>> enabled,
>> + * disable FBC just because it may help.
>> + *
>> + * We disable FBC by changing the i915 param, so
Re: [Intel-gfx] [PATCH 07/16] drm/i915: disable FBC on FIFO underruns
On Thu, Aug 20, 2015 at 10:30:17AM -0300, Paulo Zanoni wrote:
> 2015-08-19 9:06 GMT-03:00 Ville Syrjälä :
> > On Fri, Aug 14, 2015 at 06:34:12PM -0300, Paulo Zanoni wrote:
> >> If we want to try to enable FBC by default on any platform we need to
> >> be on the safe side and disable it in case we get an underrun while
> >> FBC is enabled on the corresponding pipe. We currently already have
> >> other reasons for FIFO underruns on our driver, but the ones I saw
> >> with FBC lead to black screens that only go away when you disable FBC.
> >
> > We don't try to deal with underruns on other platforms either, and yes
> > on some, cough, chv, cough, they can definitely blank out the screen
> > until the next modeset. On even older platforms it's even worse and an
> > underrun can kill the display engine until display reset/reboot :(
> > Especially annoying on gen2 where we have no reset support. So I'm not
> > entirely convinced FBC deserves special treatment here.
>
> I don't understand your logic. To me, it sounds like "you're proposing
> adding airbags to new cars, but that is not going to protect against
> every type of accident, and it's also not going to help the cars that
> are already manufactured, so I don't think front-collisions of new
> cars deserve special treatment".
Currently FBC is more like like "driving backwards on one wheel". I'm
not sure there's much point in trying to make it fault tolerant while we
know the code is still broken. Once it's otherwise known to be solid,
then it might make sense, although a much cooler thing would be if we
could actually detect when the display has failed entirely and recover
it somehow.
Oh and to make the protection mechanism actually kick in reliably you
would somehow need to address the problems with the underrun interrupts.
At the moment, to continue your car analogy, it's like the airbags would
refuse to deploy in a real crash if you happened to ding the mailbox
while pulling out of your driveway.
>
> >
> >>
> >> The current FIFO underrun I see happens when the CFB is using the top
> >> 8mb of stolen memory. This is reproducible with a 2560x1440 DP Monitor
> >> on a system with 32mb of stolen memory. On this case, all the IGT
> >> tests fail while checking the screen CRC. A later patch on this series
> >> will fix this problem for real.
> >>
> >> With this patch, the tests will start failing while checking if FBC is
> >> enabled instead of failing while comparing the CRC of the black screen
> >> against the correct CRC. So this patch is not hiding any IGT bugs: the
> >> tests still fail, but now they fail with a different reason.
> >>
> >> Signed-off-by: Paulo Zanoni
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h| 5 +++
> >> drivers/gpu/drm/i915/intel_drv.h | 1 +
> >> drivers/gpu/drm/i915/intel_fbc.c | 61
> >> ++
> >> drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +
> >> 4 files changed, 69 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> b/drivers/gpu/drm/i915/i915_drv.h
> >> index 4fd7858..e74a844 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -926,6 +926,11 @@ struct i915_fbc {
> >> struct drm_framebuffer *fb;
> >> } *fbc_work;
> >>
> >> + struct intel_fbc_underrun_work {
> >> + struct work_struct work;
> >> + struct intel_crtc *crtc;
> >> + } underrun_work;
> >> +
> >> enum no_fbc_reason {
> >> FBC_OK, /* FBC is enabled */
> >> FBC_UNSUPPORTED, /* FBC is not supported by this chipset */
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >> b/drivers/gpu/drm/i915/intel_drv.h
> >> index 81b7d77..246925d 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -1247,6 +1247,7 @@ void intel_fbc_flush(struct drm_i915_private
> >> *dev_priv,
> >>unsigned int frontbuffer_bits, enum fb_op_origin
> >> origin);
> >> const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
> >> void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
> >> +void intel_fbc_handle_fifo_underrun(struct intel_crtc *crtc);
> >>
> >> /* intel_hdmi.c */
> >> void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port
> >> port);
> >> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> >> b/drivers/gpu/drm/i915/intel_fbc.c
> >> index a63d10a..28e569c 100644
> >> --- a/drivers/gpu/drm/i915/intel_fbc.c
> >> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> >> @@ -955,6 +955,65 @@ void intel_fbc_flush(struct drm_i915_private
> >> *dev_priv,
> >> mutex_unlock(&dev_priv->fbc.lock);
> >> }
> >>
> >> +static void intel_fbc_underrun_work_fn(struct work_struct *__work)
> >> +{
> >> + struct intel_fbc_underrun_work *work =
> >> + container_of(__work, struct intel_fbc_underrun_work, work);
> >> + struct intel_crtc *crtc = work->crtc;
> >> + struct
Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: add MISSING_CASE annotation to ibx_digital_port_connected
>-Original Message-
>From: Nikula, Jani
>Sent: Thursday, August 20, 2015 1:18 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: R, Durgadoss; Ville Syrjälä; Nikula, Jani
>Subject: [PATCH v2 3/7] drm/i915: add MISSING_CASE annotation to
>ibx_digital_port_connected
>
>With the case added for eDP on port A (always connected from this
>function's point of view), we should not be hitting any of the default
>cases in ibx_digital_port_connected, so add MISSING_CASE annotation.
>
For the entire v2 series,
Reviewed-by: Durgadoss R
Thanks,
Durga
>Signed-off-by: Jani Nikula
>---
> drivers/gpu/drm/i915/intel_dp.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index d35dc0711f9b..f08859471841 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -4487,6 +4487,8 @@ static bool ibx_digital_port_connected(struct
>drm_i915_private *dev_priv,
>
> if (HAS_PCH_IBX(dev_priv->dev)) {
> switch (port->port) {
>+ case PORT_A:
>+ return true;
> case PORT_B:
> bit = SDE_PORTB_HOTPLUG;
> break;
>@@ -4497,10 +4499,13 @@ static bool ibx_digital_port_connected(struct
>drm_i915_private *dev_priv,
> bit = SDE_PORTD_HOTPLUG;
> break;
> default:
>- return true;
>+ MISSING_CASE(port->port);
>+ return false;
> }
> } else {
> switch (port->port) {
>+ case PORT_A:
>+ return true;
> case PORT_B:
> bit = SDE_PORTB_HOTPLUG_CPT;
> break;
>@@ -4511,7 +4516,8 @@ static bool ibx_digital_port_connected(struct
>drm_i915_private *dev_priv,
> bit = SDE_PORTD_HOTPLUG_CPT;
> break;
> default:
>- return true;
>+ MISSING_CASE(port->port);
>+ return false;
> }
> }
>
>--
>2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/16] drm/i915: disable FBC on FIFO underruns
2015-08-20 10:58 GMT-03:00 Ville Syrjälä :
> On Thu, Aug 20, 2015 at 10:30:17AM -0300, Paulo Zanoni wrote:
>> 2015-08-19 9:06 GMT-03:00 Ville Syrjälä :
>> > On Fri, Aug 14, 2015 at 06:34:12PM -0300, Paulo Zanoni wrote:
>> >> If we want to try to enable FBC by default on any platform we need to
>> >> be on the safe side and disable it in case we get an underrun while
>> >> FBC is enabled on the corresponding pipe. We currently already have
>> >> other reasons for FIFO underruns on our driver, but the ones I saw
>> >> with FBC lead to black screens that only go away when you disable FBC.
>> >
>> > We don't try to deal with underruns on other platforms either, and yes
>> > on some, cough, chv, cough, they can definitely blank out the screen
>> > until the next modeset. On even older platforms it's even worse and an
>> > underrun can kill the display engine until display reset/reboot :(
>> > Especially annoying on gen2 where we have no reset support. So I'm not
>> > entirely convinced FBC deserves special treatment here.
>>
>> I don't understand your logic. To me, it sounds like "you're proposing
>> adding airbags to new cars, but that is not going to protect against
>> every type of accident, and it's also not going to help the cars that
>> are already manufactured, so I don't think front-collisions of new
>> cars deserve special treatment".
>
> Currently FBC is more like like "driving backwards on one wheel". I'm
> not sure there's much point in trying to make it fault tolerant while we
> know the code is still broken.
That's why this series contains other patches fixing the other
problems. That's also why I recently added a few thousands of lines of
code to IGT. Besides, no one is proposing to enable FBC by default on
every single platform. I don't think "feature X doesn't work at the
moment" is a reason to reject a patch that improves feature X.
> Once it's otherwise known to be solid,
> then it might make sense, although a much cooler thing would be if we
> could actually detect when the display has failed entirely and recover
> it somehow.
>
> Oh and to make the protection mechanism actually kick in reliably you
> would somehow need to address the problems with the underrun interrupts.
Which problems?
When I tested this patch, it was working very reliably: we detected
the underrun and disabled FBC 100% of the time.
It's also worth noticing that the cause of the underrun is actually
fixed by a later patch of this series, so we don't really need this
patch as part of the whole "fix FBC" task, but I really think it's
better to have it than not have it, just in case we regress somehow.
> At the moment, to continue your car analogy, it's like the airbags would
> refuse to deploy in a real crash if you happened to ding the mailbox
> while pulling out of your driveway.
>
>>
>> >
>> >>
>> >> The current FIFO underrun I see happens when the CFB is using the top
>> >> 8mb of stolen memory. This is reproducible with a 2560x1440 DP Monitor
>> >> on a system with 32mb of stolen memory. On this case, all the IGT
>> >> tests fail while checking the screen CRC. A later patch on this series
>> >> will fix this problem for real.
>> >>
>> >> With this patch, the tests will start failing while checking if FBC is
>> >> enabled instead of failing while comparing the CRC of the black screen
>> >> against the correct CRC. So this patch is not hiding any IGT bugs: the
>> >> tests still fail, but now they fail with a different reason.
>> >>
>> >> Signed-off-by: Paulo Zanoni
>> >> ---
>> >> drivers/gpu/drm/i915/i915_drv.h| 5 +++
>> >> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> >> drivers/gpu/drm/i915/intel_fbc.c | 61
>> >> ++
>> >> drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +
>> >> 4 files changed, 69 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> >> b/drivers/gpu/drm/i915/i915_drv.h
>> >> index 4fd7858..e74a844 100644
>> >> --- a/drivers/gpu/drm/i915/i915_drv.h
>> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> >> @@ -926,6 +926,11 @@ struct i915_fbc {
>> >> struct drm_framebuffer *fb;
>> >> } *fbc_work;
>> >>
>> >> + struct intel_fbc_underrun_work {
>> >> + struct work_struct work;
>> >> + struct intel_crtc *crtc;
>> >> + } underrun_work;
>> >> +
>> >> enum no_fbc_reason {
>> >> FBC_OK, /* FBC is enabled */
>> >> FBC_UNSUPPORTED, /* FBC is not supported by this chipset */
>> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> >> b/drivers/gpu/drm/i915/intel_drv.h
>> >> index 81b7d77..246925d 100644
>> >> --- a/drivers/gpu/drm/i915/intel_drv.h
>> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> >> @@ -1247,6 +1247,7 @@ void intel_fbc_flush(struct drm_i915_private
>> >> *dev_priv,
>> >>unsigned int frontbuffer_bits, enum fb_op_origin
>> >> origin);
>> >> const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>> >> v
[Intel-gfx] [PATCH] drm/i915: Update ring space correctly on lrc context reset
If we leave the last_retired_head to pre-reset value, we might end up in a situation where intel_ring_space() returns wrong value on next hardware init. The recent GuC changes made ringbuffer size much smaller. Thus the odds grew that we got pre-reset last_retired_head in a value so that intel_ring_space() returned too small value after reset for mocs values to fit, triggering the following trace: [ 337.622311] [ cut here ] [ 337.622362] WARNING: CPU: 0 PID: 1355 at drivers/gpu/drm/i915/intel_lrc.c:678 intel_logical_ring_begin+0x171/0x1dd [i915]() [ 337.622365] WARN_ON(&target->list == &ring->request_list) [ 337.622368] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy x86_pkg_temp_thermal coretemp kvm_intel snd_seq_oss kvm microcode snd_seq_midi asix joydev snd_rawmidi usbnet snd_seq_midi_event input_leds serio_raw snd_seq snd_seq_device snd_timer i915 snd soundcore drm_kms_helper shpchp syscopyarea sysfillrect sysimgblt fb_sys_fops drm wmi battery ipv6 bnep video bluetooth rfkill ac parport_pc button ppdev acpi_cpufreq lp parport sdhci_pci sdhci led_class mmc_core [ 337.622461] CPU: 0 PID: 1355 Comm: kworker/u16:2 Tainted: G U W 4.2.0-rc5-drm-intel-nightly-ww33+ #1 [ 337.622465] Hardware name: Intel Corporation Skylake Client platform/Skylake Y LPDDR3 RVP3, BIOS SKLSE2R1.R00.X093.B02.1507222151 07/22/2015 [ 337.622496] Workqueue: i915-hangcheck i915_hangcheck_elapsed [i915] [ 337.622500] 0009 8800370838e8 81897fb7 [ 337.622508] 880037083938 880037083928 810467c3 880037083908 [ 337.622515] a02a6aea 8800873400c0 88016d79e480 [ 337.622523] Call Trace: [ 337.622533] [] dump_stack+0x45/0x57 [ 337.622540] [] warn_slowpath_common+0xa1/0xbb [ 337.622584] [] ? intel_logical_ring_begin+0x171/0x1dd [i915] [ 337.622590] [] warn_slowpath_fmt+0x46/0x48 [ 337.622632] [] intel_logical_ring_begin+0x171/0x1dd [i915] [ 337.622674] [] intel_rcs_context_init_mocs+0x170/0x2a9 [i915] [ 337.622714] [] ? gen8_emit_flush_render+0x174/0x18f [i915] [ 337.622753] [] gen8_init_rcs_context+0x9d/0x1f9 [i915] [ 337.622792] [] ? intel_logical_ring_reserve_space+0x26/0x2a [i915] [ 337.622827] [] i915_gem_context_enable+0x26/0x4d [i915] [ 337.622866] [] i915_gem_init_hw+0x285/0x371 [i915] [ 337.622892] [] i915_reset+0xe2/0x132 [i915] [ 337.622920] [] i915_reset_and_wakeup+0xd3/0x133 [i915] [ 337.622948] [] i915_handle_error+0x5ab/0x5bd [i915] [ 337.622956] [] ? vprintk_default+0x1d/0x1f [ 337.622962] [] ? printk+0x46/0x48 [ 337.622990] [] i915_hangcheck_elapsed+0x387/0x3a7 [i915] [ 337.622996] [] process_one_work+0x225/0x409 [ 337.623001] [] ? process_one_work+0x1a6/0x409 [ 337.623007] [] worker_thread+0x275/0x369 [ 337.623013] [] ? cancel_delayed_work_sync+0x15/0x15 [ 337.623019] [] kthread+0xed/0xf5 [ 337.623027] [] ? kthread_create_on_node+0x1ac/0x1ac [ 337.623033] [] ret_from_fork+0x3f/0x70 [ 337.623039] [] ? kthread_create_on_node+0x1ac/0x1ac [ 337.623043] ---[ end trace bbf071dfdac9d9da ]--- [ 337.623081] [drm:gen8_init_rcs_context [i915]] *ERROR* MOCS failed to program: expect performance issues. Setup last_retired_head correctly on context reset and recalculate free space for ringbuf. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91634 Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_lrc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 60c683e..61c99e9 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2507,5 +2507,7 @@ void intel_lr_context_reset(struct drm_device *dev, ringbuf->head = 0; ringbuf->tail = 0; + ringbuf->last_retired_head = -1; + intel_ring_update_space(ringbuf); } } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] kms_addfb_basic: Require fb modifiers for unused field tests
The drm core doesn't check unused fields of ADDFB2 for pre-FB_MODIFIERS
userspace, so require that and use the local version of the defines.
Signed-off-by: Ander Conselvan de Oliveira
---
tests/kms_addfb_basic.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
index f10e12b..732d6dc 100644
--- a/tests/kms_addfb_basic.c
+++ b/tests/kms_addfb_basic.c
@@ -51,6 +51,7 @@ static void invalid_tests(int fd)
f.height = 512;
f.pixel_format = DRM_FORMAT_XRGB;
f.pitches[0] = 512*4;
+ f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
igt_fixture {
gem_bo = gem_create(fd, 1024*1024*4);
@@ -60,35 +61,43 @@ static void invalid_tests(int fd)
f.handles[0] = gem_bo;
- igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+ igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
f.fb_id = 0;
}
igt_subtest("unused-handle") {
+ igt_require_fb_modifiers(fd);
+
f.handles[1] = gem_bo_small;
- igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
+ igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == -1
&&
errno == EINVAL);
f.handles[1] = 0;
}
igt_subtest("unused-pitches") {
+ igt_require_fb_modifiers(fd);
+
f.pitches[1] = 512;
- igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
+ igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == -1
&&
errno == EINVAL);
f.pitches[1] = 0;
}
igt_subtest("unused-offsets") {
+ igt_require_fb_modifiers(fd);
+
f.offsets[1] = 512;
- igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
+ igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == -1
&&
errno == EINVAL);
f.offsets[1] = 0;
}
igt_subtest("unused-modifier") {
+ igt_require_fb_modifiers(fd);
+
f.modifier[1] = LOCAL_I915_FORMAT_MOD_X_TILED;
- igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
+ igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == -1
&&
errno == EINVAL);
f.modifier[1] = 0;
}
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/16] drm/i915: disable FBC on FIFO underruns
On Thu, Aug 20, 2015 at 11:29:29AM -0300, Paulo Zanoni wrote:
> 2015-08-20 10:58 GMT-03:00 Ville Syrjälä :
> > On Thu, Aug 20, 2015 at 10:30:17AM -0300, Paulo Zanoni wrote:
> >> 2015-08-19 9:06 GMT-03:00 Ville Syrjälä :
> >> > On Fri, Aug 14, 2015 at 06:34:12PM -0300, Paulo Zanoni wrote:
> >> >> If we want to try to enable FBC by default on any platform we need to
> >> >> be on the safe side and disable it in case we get an underrun while
> >> >> FBC is enabled on the corresponding pipe. We currently already have
> >> >> other reasons for FIFO underruns on our driver, but the ones I saw
> >> >> with FBC lead to black screens that only go away when you disable FBC.
> >> >
> >> > We don't try to deal with underruns on other platforms either, and yes
> >> > on some, cough, chv, cough, they can definitely blank out the screen
> >> > until the next modeset. On even older platforms it's even worse and an
> >> > underrun can kill the display engine until display reset/reboot :(
> >> > Especially annoying on gen2 where we have no reset support. So I'm not
> >> > entirely convinced FBC deserves special treatment here.
> >>
> >> I don't understand your logic. To me, it sounds like "you're proposing
> >> adding airbags to new cars, but that is not going to protect against
> >> every type of accident, and it's also not going to help the cars that
> >> are already manufactured, so I don't think front-collisions of new
> >> cars deserve special treatment".
> >
> > Currently FBC is more like like "driving backwards on one wheel". I'm
> > not sure there's much point in trying to make it fault tolerant while we
> > know the code is still broken.
>
> That's why this series contains other patches fixing the other
> problems.
Well the whole intel_update_fbc() thing is still very much a fail wrt.
locking. But maybe you're planning on resurrecting my fbc_score idea?
Also now I that I glanced at the code, intel_fbc_flush() looks busted.
To actually nuke the FBC state you need to wait at least until the next
vblank before re-enabling FBC. That was one reason I had the vblank
notify thingy in my "fix FBC" series.
> That's also why I recently added a few thousands of lines of
> code to IGT. Besides, no one is proposing to enable FBC by default on
> every single platform. I don't think "feature X doesn't work at the
> moment" is a reason to reject a patch that improves feature X.
>
> > Once it's otherwise known to be solid,
> > then it might make sense, although a much cooler thing would be if we
> > could actually detect when the display has failed entirely and recover
> > it somehow.
> >
> > Oh and to make the protection mechanism actually kick in reliably you
> > would somehow need to address the problems with the underrun interrupts.
>
> Which problems?
The fact that we disable them as soon as one occurs, and even worse the
error interrupt is a shared one on many platforms, so one underrun any
pipe or some other unrelated error kills underrun detection everywhere.
>
> When I tested this patch, it was working very reliably: we detected
> the underrun and disabled FBC 100% of the time.
>
> It's also worth noticing that the cause of the underrun is actually
> fixed by a later patch of this series, so we don't really need this
> patch as part of the whole "fix FBC" task, but I really think it's
> better to have it than not have it, just in case we regress somehow.
>
> > At the moment, to continue your car analogy, it's like the airbags would
> > refuse to deploy in a real crash if you happened to ding the mailbox
> > while pulling out of your driveway.
> >
> >>
> >> >
> >> >>
> >> >> The current FIFO underrun I see happens when the CFB is using the top
> >> >> 8mb of stolen memory. This is reproducible with a 2560x1440 DP Monitor
> >> >> on a system with 32mb of stolen memory. On this case, all the IGT
> >> >> tests fail while checking the screen CRC. A later patch on this series
> >> >> will fix this problem for real.
> >> >>
> >> >> With this patch, the tests will start failing while checking if FBC is
> >> >> enabled instead of failing while comparing the CRC of the black screen
> >> >> against the correct CRC. So this patch is not hiding any IGT bugs: the
> >> >> tests still fail, but now they fail with a different reason.
> >> >>
> >> >> Signed-off-by: Paulo Zanoni
> >> >> ---
> >> >> drivers/gpu/drm/i915/i915_drv.h| 5 +++
> >> >> drivers/gpu/drm/i915/intel_drv.h | 1 +
> >> >> drivers/gpu/drm/i915/intel_fbc.c | 61
> >> >> ++
> >> >> drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +
> >> >> 4 files changed, 69 insertions(+)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> >> b/drivers/gpu/drm/i915/i915_drv.h
> >> >> index 4fd7858..e74a844 100644
> >> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> >> @@ -926,6 +926,11 @@ struct i915_fbc {
> >> >> struct drm_framebuffer *fb;
>
Re: [Intel-gfx] [PATCH] drm/i915: Update ring space correctly on lrc context reset
On Thu, Aug 20, 2015 at 05:34:59PM +0300, Mika Kuoppala wrote: > If we leave the last_retired_head to pre-reset value, we might > end up in a situation where intel_ring_space() returns wrong > value on next hardware init. http://patchwork.freedesktop.org/patch/46612/ and earlier -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-firmware-i915 pull request
Hi,
Please consider pulling i915 updates to linux-firmware.git.
The following changes since commit
38358cfcf50436bf4462b3e45f5b30ab66bb63a6:
firmware: tegra: Update XHCI firmware to v50.10 for T210 (2015-08-12
14:39:31 -0400)
are available in the git repository at:
git://people.freedesktop.org/~vivijim/linux-firmware-i915
for you to fetch changes up to
d1fa230aff5d7bd198c9697b1eee396fd66d88a4:
linux-firmware: New major GuC release for Skylake. (2015-08-20
08:28:18 -0700)
Rodrigo Vivi (6):
linux-firmware: New minor DMC release for Skylake - ver1_19
linux-firmware: New minor DMC release for Broxton - ver1_05
linux-firmware: New minor DMC release for Skylake - ver1_20
linux-firmware: New minor DMC release for Skylake - ver1_21
linux-firmware: Clean up i915 by removing old skl dmc firmware.
linux-firmware: New major GuC release for Skylake.
WHENCE| 15 ++
-
i915/bxt_dmc_ver1.bin | 2 +-
i915/bxt_dmc_ver1_05.bin | Bin 0 -> 5872
bytes
i915/skl_dmc_ver1.bin | 2 +-
i915/skl_dmc_ver1_04.bin | Bin 8048 -> 0
bytes
i915/{skl_dmc_ver1_16.bin => skl_dmc_ver1_19.bin} | Bin 7892 -> 8380
bytes
i915/{skl_dmc_ver1_18.bin => skl_dmc_ver1_20.bin} | Bin 7892 -> 8380
bytes
i915/skl_dmc_ver1_21.bin | Bin 0 -> 8824
bytes
i915/skl_guc_ver4.bin | 1 +
i915/skl_guc_ver4_3.bin | Bin 0 -> 128320
bytes
10 files changed, 13 insertions(+), 7 deletions(-)
create mode 100644 i915/bxt_dmc_ver1_05.bin
delete mode 100644 i915/skl_dmc_ver1_04.bin
rename i915/{skl_dmc_ver1_16.bin => skl_dmc_ver1_19.bin} (90%)
rename i915/{skl_dmc_ver1_18.bin => skl_dmc_ver1_20.bin} (58%)
create mode 100644 i915/skl_dmc_ver1_21.bin
create mode 12 i915/skl_guc_ver4.bin
create mode 100644 i915/skl_guc_ver4_3.bin
Thanks,
Rodrigo.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
From: Ville Syrjälä
We are no longer checkling the DP link status on long hpd. We used to do
that from the .hot_plug() handler, but it was removed when MST got
introduced.
If there's no userspace we now fail to retrain the link if the sink
power is toggled (or cable yanked and replugged), meaning the user is
left staring at a blank screen. With the retraining put back that should
be fixed.
Also remove the leftover comment that referred to the old retraining
from .hot_plug().
Fixes a regression introduced in:
commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
Author: Dave Airlie
Date: Fri May 2 14:02:48 2014 +1000
drm/i915: add DP 1.2 MST support (v0.7)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
Cc: Dave Airlie
Cc: sta...@vger.kernel.org
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d32ce48..b014158 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
intel_dp_probe_oui(intel_dp);
- if (!intel_dp_probe_mst(intel_dp))
+ if (!intel_dp_probe_mst(intel_dp)) {
+ drm_modeset_lock(&dev->mode_config.connection_mutex,
NULL);
+ intel_dp_check_link_status(intel_dp);
+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
goto mst_fail;
-
+ }
} else {
if (intel_dp->is_mst) {
if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
@@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
}
if (!intel_dp->is_mst) {
- /*
-* we'll check the link status via the normal hot plug
path later -
-* but for short hpds we should check it now
-*/
drm_modeset_lock(&dev->mode_config.connection_mutex,
NULL);
intel_dp_check_link_status(intel_dp);
drm_modeset_unlock(&dev->mode_config.connection_mutex);
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Use dpcd read wake for sink crc calls.
From: Rodrigo Vivi
Let's use a native read with retry as suggested per spec to
fix Sink CRC on SKL when PSR is enabled.
With PSR enabled panel is probably taking more time to wake
and dpcd read is faling.
Cc: Sonika Jindal
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_dp.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d32ce48..34f5e33 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4037,7 +4037,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp
*intel_dp)
u8 buf;
int ret = 0;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK,
+ &buf, 1) < 0) {
DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
ret = -EIO;
goto out;
@@ -4069,7 +4070,8 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
return ret;
}
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK_MISC,
+ &buf, 1) < 0)
return -EIO;
if (!(buf & DP_TEST_CRC_SUPPORTED))
@@ -4077,7 +4079,7 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK, &buf, 1) < 0)
return -EIO;
hsw_disable_ips(intel_crtc);
@@ -4109,8 +4111,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
do {
intel_wait_for_vblank(dev, intel_crtc->pipe);
- if (drm_dp_dpcd_readb(&intel_dp->aux,
- DP_TEST_SINK_MISC, &buf) < 0) {
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux,
+ DP_TEST_SINK_MISC, &buf, 1) < 0) {
ret = -EIO;
goto stop;
}
@@ -4123,7 +4125,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
if (count == 0)
intel_dp->sink_crc.last_count = 0;
- if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6)
< 0) {
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_CRC_R_CR,
+ crc, 6) < 0) {
ret = -EIO;
goto stop;
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Use dpcd read wake for sink crc calls.
Let's use a native read with retry as suggested per spec to
fix Sink CRC on SKL when PSR is enabled.
With PSR enabled panel is probably taking more time to wake
and dpcd read is faling.
v2: Fix my email domain on commit message. Thanks Rafael.
Cc: Rafael Antognolli
Cc: Sonika Jindal
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_dp.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d32ce48..34f5e33 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4037,7 +4037,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp
*intel_dp)
u8 buf;
int ret = 0;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK,
+ &buf, 1) < 0) {
DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
ret = -EIO;
goto out;
@@ -4069,7 +4070,8 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
return ret;
}
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK_MISC,
+ &buf, 1) < 0)
return -EIO;
if (!(buf & DP_TEST_CRC_SUPPORTED))
@@ -4077,7 +4079,7 @@ static int intel_dp_sink_crc_start(struct intel_dp
*intel_dp)
intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK, &buf, 1) < 0)
return -EIO;
hsw_disable_ips(intel_crtc);
@@ -4109,8 +4111,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
do {
intel_wait_for_vblank(dev, intel_crtc->pipe);
- if (drm_dp_dpcd_readb(&intel_dp->aux,
- DP_TEST_SINK_MISC, &buf) < 0) {
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux,
+ DP_TEST_SINK_MISC, &buf, 1) < 0) {
ret = -EIO;
goto stop;
}
@@ -4123,7 +4125,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
if (count == 0)
intel_dp->sink_crc.last_count = 0;
- if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6)
< 0) {
+ if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_CRC_R_CR,
+ crc, 6) < 0) {
ret = -EIO;
goto stop;
}
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/7] drm/i915: PSR: Let's rely more on frontbuffer tracking.
Many reasons here:
- Hardware tracking also has hidden corner cases
- Frontbuffer tracking is mature and reliable now
- Our sw exit by unseting bit 31 is really fast and reliable.
Also frontbuffer tracking flush means invalidate and flush.
So, let's rely more and do the proper meaning of flush for
all cases without any workaround.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_psr.c | 22 +++---
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 92e2b467..63bbab2 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -719,25 +719,9 @@ void intel_psr_flush(struct drm_device *dev,
frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
- if (HAS_DDI(dev)) {
- /*
-* By definition every flush should mean invalidate + flush,
-* however on core platforms let's minimize the
-* disable/re-enable so we can avoid the invalidate when flip
-* originated the flush.
-*/
- if (frontbuffer_bits && origin != ORIGIN_FLIP)
- intel_psr_exit(dev);
- } else {
- /*
-* On Valleyview and Cherryview we don't use hardware tracking
-* so any plane updates or cursor moves don't result in a PSR
-* invalidating. Which means we need to manually fake this in
-* software for all flushes.
-*/
- if (frontbuffer_bits)
- intel_psr_exit(dev);
- }
+ /* By definition flush = invalidate + flush */
+ if (frontbuffer_bits)
+ intel_psr_exit(dev);
if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
schedule_delayed_work(&dev_priv->psr.work,
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915: Fix PSR disable sequence on core platforms.
According to spec the disable sequence is:
Driver will do the following on PSR Disable.
1. Disable PSR in PSR control register, SRD_CTL[bit 31].
2. Poll on PSR idle
3. Wait for VBlank
4. Disable VSC DIP.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_psr.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 51f0514..92e2b467 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -459,6 +459,10 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config->cpu_transcoder);
if (dev_priv->psr.active) {
I915_WRITE(EDP_PSR_CTL(dev),
@@ -469,6 +473,12 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
DRM_ERROR("Timed out waiting for PSR Idle State\n");
+ intel_wait_for_vblank(dev, pipe);
+
+ I915_WRITE(ctl_reg, I915_READ(ctl_reg)
+ & ~VIDEO_DIP_ENABLE_VSC_HSW);
+ POSTING_READ(ctl_reg);
+
dev_priv->psr.active = false;
} else {
WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/i915: Delay first PSR activation.
This affects PSR on VLV, CHV, HSW and BDW. When debuging the frozen screen caused by HW tracking with low power state I noticed that if we keep moving the mouse non stop you will miss the screen updates for a while. At least until we stop moving the mouse for a small time and move again. The actual enabling should happen immediately after Display Port enabling sequence finished with links trained and everything enabled. However we face many issues when enabling PSR right after a modeset. On VLV/CHV we face blank screens on this scenario and on HSW+ we face a recoverable frozen screen, at least until next exit-activate sequence. Another workaround for the same issue here would be to increase re-enable idle time from 100 to 500 as we did for VLV/CHV. However this patch workaround this issue in a better way since it doesn't reduce PSR residency and also allow us to reduce the delay time between re-enables at least on VLV/CHV. Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_psr.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index d02d4e2..2be4a62 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -427,6 +427,19 @@ void intel_psr_enable(struct intel_dp *intel_dp) vlv_psr_enable_source(intel_dp); } + /* +* FIXME: Activation should happen immediately since this function +* is just called after pipe is fully trained and enabled. +* However on previous platforms we face issues when first activation +* follows a modeset so quickly. +* - On VLV/CHV we get bank screen on first activation +* - On HSW/BDW we get a recoverable frozen screen until next +* exit-activate sequence. +*/ + if (INTEL_INFO(dev)->gen < 9) + schedule_delayed_work(&dev_priv->psr.work, + msecs_to_jiffies(500)); + dev_priv->psr.enabled = intel_dp; unlock: mutex_unlock(&dev_priv->psr.lock); @@ -729,8 +742,9 @@ void intel_psr_flush(struct drm_device *dev, intel_psr_exit(dev); if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) - schedule_delayed_work(&dev_priv->psr.work, - msecs_to_jiffies(delay_ms)); + if (!work_busy(&dev_priv->psr.work.work)) + schedule_delayed_work(&dev_priv->psr.work, + msecs_to_jiffies(delay_ms)); mutex_unlock(&dev_priv->psr.lock); } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/7] drm/i915: Reduce PSR re-activation time for VLV/CHV.
With commit 30886c5a ("drm/i915: VLV/CHV PSR: Increase wait delay
time before active PSR.") we fixed a blank screen when first
activation was happening immediatelly after PSR being enabled.
There we gave more time for idleness by increasing the delay
between re-activating sequences.
However, commit "drm/i915: Delay first PSR activation."
delay the first activation in a better way keeping a good psr
residency. So, we can now reduce the delay on re-enable.
Unfortunately we cannot reduce to 0 as on core platforms
because on SW mode the transiction to active state
happens immediatelly and panel needs some idle frames.
However instead to reduce it back to 100ms let's propperly
calculate the frame time and wait at least 2 frame time so
we assure 1 entire vblank period.
I also avoided wait_for_vblank to avoid long period block
on psr.lock. And also I calculated the time only once per
enable so we avoid re-calculating this time every exit.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_psr.c | 13 -
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e0f3f05..0e2cb35 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -984,6 +984,7 @@ struct i915_psr {
unsigned busy_frontbuffer_bits;
bool psr2_support;
bool aux_frame_sync;
+ int sw_idle_frame_delay;
};
enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index a850b7d..7517207 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -364,6 +364,8 @@ void intel_psr_enable(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
+ struct drm_display_mode *mode = &crtc->config->base.adjusted_mode;
+ int frame_time, idle_frames;
if (!HAS_PSR(dev)) {
DRM_DEBUG_KMS("PSR not supported on this platform\n");
@@ -425,6 +427,15 @@ void intel_psr_enable(struct intel_dp *intel_dp)
* to active transition, i.e. here.
*/
vlv_psr_enable_source(intel_dp);
+
+ /*
+* On VLV/CHV the transition to PSR active happens
+* immediatelly on SW mode. So we need to simulate
+* idle_frames to respect panel limitations.
+*/
+ frame_time = DIV_ROUND_UP(1000, drm_mode_vrefresh(mode));
+ idle_frames = dev_priv->vbt.psr.idle_frames + 2;
+ dev_priv->psr.sw_idle_frame_delay = idle_frames * frame_time;
}
/*
@@ -723,7 +734,7 @@ void intel_psr_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc;
enum pipe pipe;
- int delay_ms = HAS_DDI(dev) ? 0 : 500;
+ int delay_ms = dev_priv->psr.sw_idle_frame_delay;
mutex_lock(&dev_priv->psr.lock);
if (!dev_priv->psr.enabled) {
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/7] drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink.
This is wrong since my commit (89251b17). The intention of that commit was to remove this one here that is also wrong anyway, but it was forgotten. Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index a04b4dc..51f0514 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -170,9 +170,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp) aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0); - drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, - DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); - /* Enable AUX frame sync at sink */ if (dev_priv->psr.aux_frame_sync) drm_dp_dpcd_writeb(&intel_dp->aux, -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/7] drm/i915: PSR: Mask LPSP hw tracking back again.
At the beginning it was masked to allow PSR at all.
Than it got removed later by my
commit 09108b90f040 ("drm/i915: PSR: Remove Low Power HW tracking mask.")
in order to trying fixing one case reported at intel-gfx mailing list
where we were missing screen updates when runtime_pm was enabled.
However I verified that other patch that makes flush to force
invalidate also fixes this issue by itself.
commit 169de1316c1e ("drm/i915: PSR: Flush means invalidate + flush")
Mainly now that we are relying more on frontbuffer tracking it is a
good idea to mask this hw tracking again.
But besides all this above it is important to hightligh that with LPSP
unmasked we started seeing some screen freezings as reported at fd.o.
This patch fixes the unrecoverable frozen screens reported:
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=91436
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=91437
Cc: Ivan Mitev
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_psr.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 63bbab2..d02d4e2 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -398,9 +398,14 @@ void intel_psr_enable(struct intel_dp *intel_dp)
skl_psr_setup_su_vsc(intel_dp);
}
- /* Avoid continuous PSR exit by masking memup and hpd */
+ /*
+* Per Spec: Avoid continuous PSR exit by masking MEMUP and HPD.
+* Also mask LPSP to avoid dependency on other drivers that
+* might block runtime_pm besides preventing other hw tracking
+* issues now we can rely on frontbuffer tracking.
+*/
I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
- EDP_PSR_DEBUG_MASK_HPD);
+ EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
/* Enable PSR on the panel */
hsw_psr_enable_sink(intel_dp);
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/7] drm/i915: Remove psr re-activation delay on HSW+.
commit 97173eaf5f3 ("drm/i915: PSR: Increase idle_frames")
incresed the number of idle frames making PSR entry more
reliable on many panels. Also commit
"drm/i915: Delay first PSR activation." move add the
delay workaround to the right place since all errors that
made us to introduce this delay here was the first
activation after the modeset.
So, now we are able to remove the delay on re-activation
getting better PSR residency.
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_psr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2be4a62..a850b7d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -723,7 +723,7 @@ void intel_psr_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc;
enum pipe pipe;
- int delay_ms = HAS_DDI(dev) ? 100 : 500;
+ int delay_ms = HAS_DDI(dev) ? 0 : 500;
mutex_lock(&dev_priv->psr.lock);
if (!dev_priv->psr.enabled) {
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/7] More PSR patches
This patch series brings stability to PSR on VLV, CHV, HSW and BDW. It fixes issues Matthew Garrett without causing the blank and frozen screens Ivan Mitev was facing. It also move the VLV/CHV workaround of that big delay from re-enable to the first enable after modeset that was the real issue. With this besides the stability this series brings more PSR residency to these platforms. However the enable by default is still out of this series and will come only after an intensive QA. Thanks, Rodrigo. Rodrigo Vivi (7): drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink. drm/i915: Fix PSR disable sequence on core platforms. drm/i915: PSR: Let's rely more on frontbuffer tracking. drm/i915: PSR: Mask LPSP hw tracking back again. drm/i915: Delay first PSR activation. drm/i915: Remove psr re-activation delay on HSW+. drm/i915: Reduce PSR re-activation time for VLV/CHV. drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_psr.c | 75 +--- 2 files changed, 49 insertions(+), 27 deletions(-) -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/13] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code
Just pull the info out of the plane state structure rather than staging
it in an additional structure.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_pm.c | 133 +---
1 file changed, 70 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c22..c9cf7cf 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1791,9 +1791,6 @@ struct ilk_pipe_wm_parameters {
bool active;
uint32_t pipe_htotal;
uint32_t pixel_rate;
- struct intel_plane_wm_parameters pri;
- struct intel_plane_wm_parameters spr;
- struct intel_plane_wm_parameters cur;
};
struct ilk_wm_maximums {
@@ -1815,25 +1812,25 @@ struct intel_wm_config {
* mem_value must be in 0.1us units.
*/
static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t mem_value,
bool is_lp)
{
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !params->pri.enabled)
+ if (!params->active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate,
-params->pri.bytes_per_pixel,
-mem_value);
+ method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
if (!is_lp)
return method1;
method2 = ilk_wm_method2(params->pixel_rate,
params->pipe_htotal,
-params->pri.horiz_pixels,
-params->pri.bytes_per_pixel,
+drm_rect_width(&pstate->dst),
+bpp,
mem_value);
return min(method1, method2);
@@ -1844,20 +1841,20 @@ static uint32_t ilk_compute_pri_wm(const struct
ilk_pipe_wm_parameters *params,
* mem_value must be in 0.1us units.
*/
static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t mem_value)
{
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !params->spr.enabled)
+ if (!params->active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate,
-params->spr.bytes_per_pixel,
-mem_value);
+ method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
method2 = ilk_wm_method2(params->pixel_rate,
params->pipe_htotal,
-params->spr.horiz_pixels,
-params->spr.bytes_per_pixel,
+drm_rect_width(&pstate->dst),
+bpp,
mem_value);
return min(method1, method2);
}
@@ -1867,28 +1864,32 @@ static uint32_t ilk_compute_spr_wm(const struct
ilk_pipe_wm_parameters *params,
* mem_value must be in 0.1us units.
*/
static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t mem_value)
{
- if (!params->active || !params->cur.enabled)
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
+
+ if (!params->active || !pstate->visible)
return 0;
return ilk_wm_method2(params->pixel_rate,
params->pipe_htotal,
- params->cur.horiz_pixels,
- params->cur.bytes_per_pixel,
+ drm_rect_width(&pstate->dst),
+ bpp,
mem_value);
}
/* Only for WM_LP. */
static uint32_t ilk_compute_fbc_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t pri_val)
{
- if (!params->active || !params->pri.enabled)
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
+
+ if (!params->active || !pstate->visible)
return 0;
- return ilk_wm_fbc(pri_val,
- params->pri.horiz_pixels,
- params->pri.bytes_per_pixel);
+ return ilk_wm_fbc(pri_val, drm_rect_width(&pstate->dst), bpp);
}
static unsigned int ilk_display_fifo_size(const struct drm_device *dev)
@@ -2053,10 +2054,12 @@ static
[Intel-gfx] [PATCH 10/13] drm/i915: Calculate watermark configuration during atomic check
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/i915_drv.h | 10 ++
drivers/gpu/drm/i915/intel_display.c | 51 ++--
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 66 +++-
4 files changed, 71 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c91bab9..ac13cd7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1686,6 +1686,13 @@ struct i915_execbuffer_params {
struct drm_i915_gem_request *request;
};
+/* used in computing the new watermarks state */
+struct intel_wm_config {
+ unsigned int num_pipes_active;
+ bool sprites_enabled;
+ bool sprites_scaled;
+};
+
struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *objects;
@@ -1903,6 +1910,9 @@ struct drm_i915_private {
*/
uint16_t skl_latency[8];
+ /* Committed wm config */
+ struct intel_wm_config config;
+
/*
* The skl_wm_values structure is a bit too big for stack
* allocation, so we keep the staging struct where we store
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c40f025..8e9d87a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13005,6 +13005,44 @@ static int intel_modeset_checks(struct
drm_atomic_state *state)
return 0;
}
+/*
+ * Handle calculation of various watermark data at the end of the atomic check
+ * phase. The code here should be run after the per-crtc and per-plane 'check'
+ * handlers to ensure that all derived state has been updated.
+ */
+static void calc_watermark_data(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *cstate;
+ struct drm_plane *plane;
+ struct drm_plane_state *pstate;
+
+ /*
+* Calculate watermark configuration details now that derived
+* plane/crtc state is all properly updated.
+*/
+ drm_for_each_crtc(crtc, dev) {
+ cstate = drm_atomic_get_existing_crtc_state(state, crtc) ?:
+ crtc->state;
+
+ intel_state->wm_config.num_pipes_active++;
+ }
+ drm_for_each_legacy_plane(plane, dev) {
+ pstate = drm_atomic_get_existing_plane_state(state, plane) ?:
+ plane->state;
+
+ if (!to_intel_plane_state(pstate)->visible)
+ continue;
+
+ intel_state->wm_config.sprites_enabled = true;
+ if (pstate->crtc_w != pstate->src_w >> 16 ||
+ pstate->crtc_h != pstate->src_h >> 16)
+ intel_state->wm_config.sprites_scaled = true;
+ }
+}
+
/**
* intel_atomic_check - validate state object
* @dev: drm device
@@ -13013,6 +13051,7 @@ static int intel_modeset_checks(struct drm_atomic_state
*state)
static int intel_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int ret, i;
@@ -13076,10 +13115,15 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
} else
- to_intel_atomic_state(state)->cdclk =
- to_i915(state->dev)->cdclk_freq;
+ intel_state->cdclk = to_i915(state->dev)->cdclk_freq;
- return drm_atomic_helper_check_planes(state->dev, state);
+ ret = drm_atomic_helper_check_planes(state->dev, state);
+ if (ret)
+ return ret;
+
+ calc_watermark_data(state);
+
+ return 0;
}
/**
@@ -13119,6 +13163,7 @@ static int intel_atomic_commit(struct drm_device *dev,
return ret;
drm_atomic_helper_swap_state(dev, state);
+ dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f9cac4b..6ac1010c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -239,6 +239,7 @@ struct intel_atomic_state {
unsigned int cdclk;
bool dpll_set;
struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
+ struct intel_wm_config wm_config;
};
struct intel_plane_state {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bc29260..b32d6b0 10
[Intel-gfx] [PATCH 13/13] drm/i915/skl: Clarify pending vs hw watermark values
Although we calculate watermark values in the atomic state, we still
have both 'pending' and 'hardware' values that ultimately wind up in
dev_priv due to the somewhat complicated watermark flushing process.
Re-arranging how these are stored/named in the dev_priv substructure may
add a little bit of clarity to what they're for.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 29 +
drivers/gpu/drm/i915/intel_display.c | 4 ++--
drivers/gpu/drm/i915/intel_pm.c | 12 ++--
4 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 54508f1..2a24141 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3134,7 +3134,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
drm_modeset_lock_all(dev);
- ddb = &dev_priv->wm.skl_hw.ddb;
+ ddb = &dev_priv->wm.skl.hw.ddb;
seq_printf(m, "%-15s%8s%8s%8s\n", "", "Start", "End", "Size");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index be42dd8..c4ba9a2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1918,17 +1918,30 @@ struct drm_i915_private {
/* Committed wm config */
struct intel_wm_config config;
- /*
-* The skl_wm_values structure is a bit too big for stack
-* allocation, so we keep the staging struct where we store
-* intermediate results here instead.
-*/
- struct skl_wm_values skl_results;
-
/* current hardware state */
union {
struct ilk_wm_values hw;
- struct skl_wm_values skl_hw;
+ struct {
+ /*
+* The skl_wm_values structure is a bit too big
+* for stack allocation, so we keep the staging
+* struct where we store intermediate results
+* here instead.
+*
+* These watermark values correspond to the
+* state that has been swapped into the various
+* DRM objects, but may not have been
+* completely written to hardware yet due to
+* the multi-step flushing process.
+*/
+ struct skl_wm_values pending;
+
+ /*
+* Watermark values that have been fully
+* written and flushed to the hardware.
+*/
+ struct skl_wm_values hw;
+ } skl;
struct vlv_wm_values vlv;
};
} wm;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0a0e7f7..c58a83b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12581,7 +12581,7 @@ static void check_wm_state(struct drm_device *dev)
return;
skl_ddb_get_hw_state(dev_priv, &hw_ddb);
- sw_ddb = &dev_priv->wm.skl_hw.ddb;
+ sw_ddb = &dev_priv->wm.skl.hw.ddb;
for_each_intel_crtc(dev, intel_crtc) {
struct skl_ddb_entry *hw_entry, *sw_entry;
@@ -13192,7 +13192,7 @@ static int intel_atomic_commit(struct drm_device *dev,
drm_atomic_helper_swap_state(dev, state);
dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
- dev_priv->wm.skl_results = to_intel_atomic_state(state)->skl_wm;
+ dev_priv->wm.skl.pending = to_intel_atomic_state(state)->skl_wm;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a9e9e57..f1c989c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3183,7 +3183,7 @@ static bool skl_ddb_allocation_changed(const struct
skl_ddb_allocation *new_ddb,
{
struct drm_device *dev = intel_crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- const struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+ const struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl.hw.ddb;
enum pipe pipe = intel_crtc->pipe;
if (memcmp(new_ddb->plane[pipe], cur_ddb->plane[pipe],
@@ -3529,7 +3529,7 @@ static void skl_flush_wm_values(struct drm_i915_private
*dev_priv,
enum pipe pipe;
new_ddb = &new_value
[Intel-gfx] [PATCH 11/13] drm/i915: Add two-stage ILK-style watermark programming (v3)
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.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/i915_drv.h | 5 ++
drivers/gpu/drm/i915/intel_display.c | 77 ++-
drivers/gpu/drm/i915/intel_drv.h | 33 ++--
drivers/gpu/drm/i915/intel_pm.c | 144 ---
4 files changed, 208 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac13cd7..be42dd8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -622,6 +622,11 @@ struct drm_i915_display_funcs {
struct dpll *best_clock);
int (*compute_pipe_wm)(struct drm_crtc *crtc,
struct drm_atomic_state *state);
+ int (*compute_intermediate_wm)(struct drm_device *dev,
+ struct intel_crtc_state *newstate,
+ const struct intel_crtc_state *oldstate);
+ void (*program_watermarks)(struct drm_i915_private *dev_priv);
+ void (*optimize_watermarks)(struct intel_crtc_state *cstate);
void (*update_wm)(struct drm_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8e9d87a..f929676 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11634,6 +11634,11 @@ int intel_plane_atomic_calc_changes(struct
drm_crtc_state *crtc_state,
}
} else if (intel_wm_need_update(plane, plane_state)) {
intel_crtc->atomic.update_wm_pre = true;
+
+ /* Pre-gen9 platforms need two-step watermark updates */
+ if (INTEL_INFO(dev)->gen < 9 &&
+ dev_priv->display.program_watermarks)
+ cstate->wm.need_postvbl_update = true;
}
if (visible)
@@ -11769,6 +11774,8 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc_state);
+ struct intel_crtc_state *old_pipe_config =
+ to_intel_crtc_state(crtc->state);
struct drm_atomic_state *state = crtc_state->state;
int ret;
bool mode_changed = needs_modeset(crtc_state);
@@ -11793,8 +11800,28 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
ret = 0;
if (dev_priv->display.compute_pipe_wm) {
ret = dev_priv->display.compute_pipe_wm(crtc, state);
- if (ret)
+ if (ret) {
+ DRM_DEBUG_KMS("Target pipe watermarks are invalid\n");
+ return ret;
+ }
+ }
+
+ if (dev_priv->display.compute_intermediate_wm) {
+ if (WARN_ON(!dev_priv->display.compute_pipe_wm))
+ return 0;
+
+ /*
+* Calculate 'intermediate' watermarks that satisfy both the
+* old state and the new state. We can program these
+* immediately.
+*/
+ ret = dev_priv->display.compute_intermediate_wm(crtc->dev,
+ pipe_config,
+
old_pipe_config);
+ if (ret) {
+ DRM_DEBUG_KMS("No valid intermediate pipe watermarks
are possible\n");
return ret;
+ }
}
if (INTEL_INFO(dev)->gen >= 9) {
@@ -13149,6 +13176,7 @@ static int intel_atomic_commit(struct drm_device *dev,
struct drm_i915_private *dev_pri
[Intel-gfx] [PATCH 12/13] drm/i915/skl: Switch to atomic watermark programming
Atomic watermark programming involves pre-computing the watermark
results during the 'check' phase of an atomic transaction. Although
watermark updates are triggered by the change to a specific CRTC, the
results themselves are global, so they're stored in the
intel_atomic_state. If/when the state moves to the commit phase, the
results will be copied from the state object to dev_priv->skl_results at
the same time plane/crtc state is swapped into the various display
objects. dev_priv->wm.skl_results is a temporary staging area since actual
programming to the hardware can potentially take several vblanks
depending on the changes taking place (see skl_flush_wm_values). Once
the watermark programming has been fully flushed to the hardware, the
values will be copied to dev_priv->wm.skl_hw.
The goal here is to transition to an atomic design with minimal change
to the existing SKL WM code (which is pretty complicated). Going
forward, it probably doesn't make sense to keep triggering watermark
calculation on a CRTC-by-CRTC basis, since this can lead to us
recalculating the same results multiple times for a single transaction.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 ++
drivers/gpu/drm/i915/intel_pm.c | 56 +++-
3 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f929676..0a0e7f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13192,6 +13192,7 @@ static int intel_atomic_commit(struct drm_device *dev,
drm_atomic_helper_swap_state(dev, state);
dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
+ dev_priv->wm.skl_results = to_intel_atomic_state(state)->skl_wm;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6a73a53..d4b2e66 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -240,6 +240,9 @@ struct intel_atomic_state {
bool dpll_set;
struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
struct intel_wm_config wm_config;
+
+ /* gen9-only at the moment */
+ struct skl_wm_values skl_wm;
};
struct intel_plane_state {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e0a4c4d..a9e9e57 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3601,29 +3601,31 @@ static void skl_flush_wm_values(struct drm_i915_private
*dev_priv,
}
}
-static bool skl_update_pipe_wm(struct drm_crtc *crtc,
+static bool skl_update_pipe_wm(struct drm_crtc_state *cstate,
struct skl_ddb_allocation *ddb, /* out */
struct skl_pipe_wm *pipe_wm /* out */)
{
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_crtc_state *intel_cstate = to_intel_crtc_state(cstate);
+ struct drm_crtc *crtc = cstate->crtc;
- skl_allocate_pipe_ddb(crtc, cstate, ddb);
- skl_compute_pipe_wm(crtc, ddb, cstate, pipe_wm);
+ skl_allocate_pipe_ddb(crtc, intel_cstate, ddb);
+ skl_compute_pipe_wm(crtc, ddb, intel_cstate, pipe_wm);
- if (!memcmp(&cstate->wm.skl, pipe_wm, sizeof(*pipe_wm)))
+ if (!memcmp(&intel_cstate->wm.skl, pipe_wm, sizeof(*pipe_wm)))
return false;
- cstate->wm.skl = *pipe_wm;
+ intel_cstate->wm.skl = *pipe_wm;
return true;
}
-static void skl_update_other_pipe_wm(struct drm_device *dev,
+static void skl_update_other_pipe_wm(struct drm_atomic_state *state,
struct drm_crtc *crtc,
struct skl_wm_values *r)
{
struct intel_crtc *intel_crtc;
struct intel_crtc *this_crtc = to_intel_crtc(crtc);
+ struct drm_crtc_state *cstate;
/*
* If the WM update hasn't changed the allocation for this_crtc (the
@@ -3638,7 +3640,7 @@ static void skl_update_other_pipe_wm(struct drm_device
*dev,
* Otherwise, because of this_crtc being freshly enabled/disabled, the
* other active pipes need new DDB allocation and WM values.
*/
- list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+ list_for_each_entry(intel_crtc, &state->dev->mode_config.crtc_list,
base.head) {
struct skl_pipe_wm pipe_wm = {};
bool wm_changed;
@@ -3646,11 +3648,13 @@ static void skl_update_other_pipe_wm(struct drm_device
*dev,
if (this_crtc->pipe == intel_crtc->pipe)
continue;
- if (!intel_crtc->active)
+ cstate = drm_a
[Intel-gfx] [PATCH 09/13] drm/i915: Calculate ILK-style watermarks during atomic check (v2)
Calculate pipe watermarks during atomic calculation phase, based on the
contents of the atomic transaction's state structure. We still program
the watermarks at the same time we did before, but the computation now
happens much earlier.
While this patch isn't too exciting by itself, it paves the way for
future patches. The eventual goal (which will be realized in future
patches in this series) is to calculate multiple sets up watermark
values up front, and then program them at different times (pre- vs
post-vblank) on the platforms that need a two-step watermark update.
While we're at it, s/intel_compute_pipe_wm/ilk_compute_pipe_wm/ since
this function only applies to ILK-style watermarks and we have a
completely different function for SKL-style watermarks.
Note that the original code had a memcmp() in ilk_update_wm() to avoid
calling ilk_program_watermarks() if the watermarks hadn't changed. This
memcmp vanishes here, which means we may do some unnecessary result
generation and merging in cases where watermarks didn't change, but the
lower-level function ilk_write_wm_values already makes sure that we
don't actually try to program the watermark registers again.
v2: Squash a few commits from the original series together; no longer
leave pre-calculated wm's in a separate temporary structure since
it's easier to follow the logic if we just cut over to using the
pre-calculated values directly.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_display.c | 6 +++
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 87 ++--
4 files changed, 53 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c58f38..c91bab9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -620,6 +620,8 @@ struct drm_i915_display_funcs {
int target, int refclk,
struct dpll *match_clock,
struct dpll *best_clock);
+ int (*compute_pipe_wm)(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
void (*update_wm)(struct drm_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6bcc3da..c40f025 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11791,6 +11791,12 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
}
ret = 0;
+ if (dev_priv->display.compute_pipe_wm) {
+ ret = dev_priv->display.compute_pipe_wm(crtc, state);
+ if (ret)
+ return ret;
+ }
+
if (INTEL_INFO(dev)->gen >= 9) {
if (mode_changed)
ret = skl_update_scaler_crtc(pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dfbfba9..f9cac4b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1427,6 +1427,8 @@ intel_atomic_get_crtc_state(struct drm_atomic_state
*state,
int intel_atomic_setup_scalers(struct drm_device *dev,
struct intel_crtc *intel_crtc,
struct intel_crtc_state *crtc_state);
+int intel_check_crtc(struct drm_crtc *crtc,
+struct drm_crtc_state *state);
/* intel_atomic_plane.c */
struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 04fc092..bc29260 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2043,9 +2043,11 @@ static void ilk_compute_wm_level(const struct
drm_i915_private *dev_priv,
const struct intel_crtc *intel_crtc,
int level,
struct intel_crtc_state *cstate,
+struct intel_plane_state *pristate,
+struct intel_plane_state *sprstate,
+struct intel_plane_state *curstate,
struct intel_wm_level *result)
{
- struct intel_plane *intel_plane;
uint16_t pri_latency = dev_priv->wm.pri_latency[level];
uint16_t spr_latency = dev_priv->wm.spr_latency[level];
uint16_t cur_latency = dev_priv->wm.cur_latency[level];
@@ -2057,29 +2059,11 @@ static void ilk_compute_wm_level(const struct
drm_i915_private *dev_priv,
cur_latency *= 5;
}
- for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) {
- struct intel_plane_state *pstate =
- to_intel_plane_state(intel_plan
[Intel-gfx] [PATCH 07/13] drm/i915: Refactor ilk_update_wm (v3)
From: Ville Syrjälä
Split ilk_update_wm() into two parts; one doing the programming
and the other the calculations.
v2: Fix typo in commit message
v3 (by Matt): Heavily rebased for current codebase.
Reviewed-by(v2): Paulo Zanoni
Signed-off-by: Ville Syrjälä
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_pm.c | 60 ++---
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 399e3b5..5b79c2f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3652,37 +3652,14 @@ static void skl_update_wm(struct drm_crtc *crtc)
dev_priv->wm.skl_hw = *results;
}
-static void ilk_update_wm(struct drm_crtc *crtc)
+static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_device *dev = dev_priv->dev;
+ struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct ilk_wm_maximums max;
+ struct intel_wm_config config = {};
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
- struct intel_pipe_wm pipe_wm = {};
- struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
- struct intel_wm_config config = {};
-
- /*
-* IVB workaround: must disable low power watermarks for at least
-* one frame before enabling scaling. LP watermarks can be re-enabled
-* when scaling is disabled.
-*
-* WaCxSRDisabledForSpriteScaling:ivb
-*/
- if (cstate->disable_lp_wm) {
- ilk_disable_lp_wm(dev);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- }
-
- intel_compute_pipe_wm(cstate, &pipe_wm);
-
- if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
- return;
-
- intel_crtc->wm.active = pipe_wm;
ilk_compute_wm_config(dev, &config);
@@ -3708,6 +3685,35 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, &results);
}
+static void ilk_update_wm(struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_pipe_wm pipe_wm = {};
+
+ /*
+* IVB workaround: must disable low power watermarks for at least
+* one frame before enabling scaling. LP watermarks can be re-enabled
+* when scaling is disabled.
+*
+* WaCxSRDisabledForSpriteScaling:ivb
+*/
+ if (cstate->disable_lp_wm) {
+ ilk_disable_lp_wm(crtc->dev);
+ intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
+ }
+
+ intel_compute_pipe_wm(cstate, &pipe_wm);
+
+ if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
+ return;
+
+ intel_crtc->wm.active = pipe_wm;
+
+ ilk_program_watermarks(dev_priv);
+}
+
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/13] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM
Just pull the info out of the CRTC state structure rather than staging
it in an additional structure.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_pm.c | 84 ++---
1 file changed, 28 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c9cf7cf..d82ea82 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1787,12 +1787,6 @@ struct skl_pipe_wm_parameters {
struct intel_plane_wm_parameters cursor;
};
-struct ilk_pipe_wm_parameters {
- bool active;
- uint32_t pipe_htotal;
- uint32_t pixel_rate;
-};
-
struct ilk_wm_maximums {
uint16_t pri;
uint16_t spr;
@@ -1811,7 +1805,7 @@ struct intel_wm_config {
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
*/
-static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value,
bool is_lp)
@@ -1819,16 +1813,16 @@ static uint32_t ilk_compute_pri_wm(const struct
ilk_pipe_wm_parameters *params,
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
+ method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), bpp, mem_value);
if (!is_lp)
return method1;
- method2 = ilk_wm_method2(params->pixel_rate,
-params->pipe_htotal,
+ method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
+cstate->base.adjusted_mode.crtc_htotal,
drm_rect_width(&pstate->dst),
bpp,
mem_value);
@@ -1840,19 +1834,19 @@ static uint32_t ilk_compute_pri_wm(const struct
ilk_pipe_wm_parameters *params,
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
*/
-static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value)
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
- method2 = ilk_wm_method2(params->pixel_rate,
-params->pipe_htotal,
+ method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), bpp, mem_value);
+ method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
+cstate->base.adjusted_mode.crtc_htotal,
drm_rect_width(&pstate->dst),
bpp,
mem_value);
@@ -1863,30 +1857,30 @@ static uint32_t ilk_compute_spr_wm(const struct
ilk_pipe_wm_parameters *params,
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
*/
-static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value)
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
- return ilk_wm_method2(params->pixel_rate,
- params->pipe_htotal,
+ return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
+ cstate->base.adjusted_mode.crtc_htotal,
drm_rect_width(&pstate->dst),
bpp,
mem_value);
}
/* Only for WM_LP. */
-static uint32_t ilk_compute_fbc_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_fbc_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t pri_val)
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
return ilk_wm_fbc(pri
[Intel-gfx] [PATCH 04/13] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM
Just pull the info out of the state structures rather than staging
it in an additional set of structures.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_pm.c | 304 ++--
1 file changed, 135 insertions(+), 169 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b9bcb85..0cfba0a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1779,13 +1779,6 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t
horiz_pixels,
return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
}
-struct skl_pipe_wm_parameters {
- bool active;
- uint32_t pipe_htotal;
- uint32_t pixel_rate; /* in KHz */
- struct intel_plane_wm_parameters plane[I915_MAX_PLANES + 1];
-};
-
struct ilk_wm_maximums {
uint16_t pri;
uint16_t spr;
@@ -2826,18 +2819,40 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
#define SKL_DDB_SIZE 896 /* in blocks */
#define BXT_DDB_SIZE 512
+/*
+ * Return the index of a plane in the SKL DDB and wm result arrays. Primary
+ * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES, and
+ * other universal planes are in indices 1..n. Note that this may leave
+ * unused indices between the top "sprite" plane and the cursor.
+ */
+static int
+skl_wm_plane_id(const struct intel_plane *plane)
+{
+ switch (plane->base.type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ return 0;
+ case DRM_PLANE_TYPE_CURSOR:
+ return I915_MAX_PLANES;
+ case DRM_PLANE_TYPE_OVERLAY:
+ return plane->plane;
+ default:
+ MISSING_CASE(plane->base.type);
+ return plane->plane;
+ }
+}
+
static void
skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
struct drm_crtc *for_crtc,
const struct intel_wm_config *config,
- const struct skl_pipe_wm_parameters *params,
+ const struct intel_crtc_state *cstate,
struct skl_ddb_entry *alloc /* out */)
{
struct drm_crtc *crtc;
unsigned int pipe_size, ddb_size;
int nth_active_pipe;
- if (!params->active) {
+ if (!cstate->base.active) {
alloc->start = 0;
alloc->end = 0;
return;
@@ -2903,19 +2918,27 @@ void skl_ddb_get_hw_state(struct drm_i915_private
*dev_priv,
}
static unsigned int
-skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
+skl_plane_relative_data_rate(const struct drm_plane_state *state, int y)
{
+ struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
+ struct drm_framebuffer *fb = state->fb;
/* for planar format */
- if (p->y_bytes_per_pixel) {
+ if (fb->pixel_format == DRM_FORMAT_NV12) {
if (y) /* y-plane data rate */
- return p->horiz_pixels * p->vert_pixels *
p->y_bytes_per_pixel;
+ return intel_crtc->config->pipe_src_w *
+ intel_crtc->config->pipe_src_h *
+ drm_format_plane_cpp(fb->pixel_format, 0);
else/* uv-plane data rate */
- return (p->horiz_pixels/2) * (p->vert_pixels/2) *
p->bytes_per_pixel;
+ return (intel_crtc->config->pipe_src_w/2) *
+ (intel_crtc->config->pipe_src_h/2) *
+ drm_format_plane_cpp(fb->pixel_format, 1);
}
/* for packed formats */
- return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
+ return intel_crtc->config->pipe_src_w *
+ intel_crtc->config->pipe_src_h *
+ fb->bits_per_pixel / 8;
}
/*
@@ -2924,23 +2947,24 @@ skl_plane_relative_data_rate(const struct
intel_plane_wm_parameters *p, int y)
* 3 * 4096 * 8192 * 4 < 2^32
*/
static unsigned int
-skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
-const struct skl_pipe_wm_parameters *params)
+skl_get_total_relative_data_rate(const struct intel_crtc *intel_crtc)
{
+ struct drm_device *dev = intel_crtc->base.dev;
+ const struct intel_plane *intel_plane;
unsigned int total_data_rate = 0;
- int plane;
- for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
- const struct intel_plane_wm_parameters *p;
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ const struct drm_plane_state *state = intel_plane->base.state;
- p = ¶ms->plane[plane];
- if (!p->enabled)
+ if (WARN_ON(state->fb == NULL))
continue;
- total_data_rate += skl_p
[Intel-gfx] [PATCH 06/13] drm/i915: Drop intel_update_sprite_watermarks
The only platform that still has an update_sprite_wm entrypoint is SKL;
on SKL, intel_update_sprite_watermarks just updates intel_plane->wm and
then performs a regular watermark update. However intel_plane->wm is
only used to update a couple fields in intel_wm_config, and those fields
are never used by the SKL code, so on SKL an update_sprite_wm is
effectively identical to an update_wm call. Since we're already
ensuring that the regular intel_update_wm is called any time we'd try to
call intel_update_sprite_watermarks, the whole call is redundant and can
be dropped.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/i915_drv.h | 4 ---
drivers/gpu/drm/i915/intel_display.c | 5
drivers/gpu/drm/i915/intel_drv.h | 6
drivers/gpu/drm/i915/intel_pm.c | 58
drivers/gpu/drm/i915/intel_sprite.c | 15 --
5 files changed, 88 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f04b67e..7c58f38 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -621,10 +621,6 @@ struct drm_i915_display_funcs {
struct dpll *match_clock,
struct dpll *best_clock);
void (*update_wm)(struct drm_crtc *crtc);
- void (*update_sprite_wm)(struct drm_plane *plane,
-struct drm_crtc *crtc,
-uint32_t sprite_width, uint32_t sprite_height,
-int pixel_size, bool enable, bool scaled);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
/* Returns the active state of the crtc, and if the crtc is active,
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 16e5cb6..6bcc3da 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4752,7 +4752,6 @@ static void intel_post_plane_update(struct intel_crtc
*crtc)
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_plane *plane;
if (atomic->wait_vblank)
intel_wait_for_vblank(dev, crtc->pipe);
@@ -4771,10 +4770,6 @@ static void intel_post_plane_update(struct intel_crtc
*crtc)
if (atomic->post_enable_primary)
intel_post_enable_primary(&crtc->base);
- drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
- intel_update_sprite_watermarks(plane, &crtc->base,
- 0, 0, 0, false, false);
-
memset(atomic, 0, sizeof(*atomic));
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index da145b0..65f0171 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1352,12 +1352,6 @@ void intel_init_clock_gating(struct drm_device *dev);
void intel_suspend_hw(struct drm_device *dev);
int ilk_wm_max_level(const struct drm_device *dev);
void intel_update_watermarks(struct drm_crtc *crtc);
-void intel_update_sprite_watermarks(struct drm_plane *plane,
- struct drm_crtc *crtc,
- uint32_t sprite_width,
- uint32_t sprite_height,
- int pixel_size,
- bool enabled, bool scaled);
void intel_init_pm(struct drm_device *dev);
void intel_pm_setup(struct drm_device *dev);
void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d6792d8..399e3b5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3150,18 +3150,9 @@ static void skl_compute_wm_global_parameters(struct
drm_device *dev,
struct intel_wm_config *config)
{
struct drm_crtc *crtc;
- struct drm_plane *plane;
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
config->num_pipes_active += to_intel_crtc(crtc)->active;
-
- /* FIXME: I don't think we need those two global parameters on SKL */
- list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
- struct intel_plane *intel_plane = to_intel_plane(plane);
-
- config->sprites_enabled |= intel_plane->wm.enabled;
- config->sprites_scaled |= intel_plane->wm.scaled;
- }
}
static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
@@ -3661,39 +3652,6 @@ static void skl_update_wm(struct drm_crtc *crtc)
dev_priv->wm.skl_hw = *results;
}
-static void
-skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
-
[Intel-gfx] [PATCH 08/13] drm/i915: Move active watermarks into CRTC state (v2)
Since we allocate a few CRTC states on the stack, also switch the 'wm'
struct here to be a union so that we're not wasting stack space with
other platforms' watermark values.
v2: Don't move cxsr_allowed to state (Maarten)
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_drv.h | 45 +++-
drivers/gpu/drm/i915/intel_pm.c | 30 ---
2 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 65f0171..dfbfba9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -323,6 +323,21 @@ struct intel_crtc_scaler_state {
/* drm_mode->private_flags */
#define I915_MODE_FLAG_INHERITED 1
+struct intel_pipe_wm {
+ struct intel_wm_level wm[5];
+ uint32_t linetime;
+ bool fbc_wm_enabled;
+ bool pipe_enabled;
+ bool sprites_enabled;
+ bool sprites_scaled;
+};
+
+struct skl_pipe_wm {
+ struct skl_wm_level wm[8];
+ struct skl_wm_level trans_wm;
+ uint32_t linetime;
+};
+
struct intel_crtc_state {
struct drm_crtc_state base;
@@ -458,6 +473,17 @@ struct intel_crtc_state {
/* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
bool disable_lp_wm;
+
+ struct {
+ /*
+* final watermarks, programmed post-vblank when this state
+* is committed
+*/
+ union {
+ struct intel_pipe_wm ilk;
+ struct skl_pipe_wm skl;
+ } active;
+ } wm;
};
struct vlv_wm_state {
@@ -469,15 +495,6 @@ struct vlv_wm_state {
bool cxsr;
};
-struct intel_pipe_wm {
- struct intel_wm_level wm[5];
- uint32_t linetime;
- bool fbc_wm_enabled;
- bool pipe_enabled;
- bool sprites_enabled;
- bool sprites_scaled;
-};
-
struct intel_mmio_flip {
struct work_struct work;
struct drm_i915_private *i915;
@@ -485,12 +502,6 @@ struct intel_mmio_flip {
struct intel_crtc *crtc;
};
-struct skl_pipe_wm {
- struct skl_wm_level wm[8];
- struct skl_wm_level trans_wm;
- uint32_t linetime;
-};
-
/*
* Tracking of operations that need to be performed at the beginning/end of an
* atomic commit, outside the atomic section where interrupts are disabled.
@@ -555,10 +566,6 @@ struct intel_crtc {
/* per-pipe watermark state */
struct {
- /* watermarks currently being used */
- struct intel_pipe_wm active;
- /* SKL wm values currently in use */
- struct skl_pipe_wm skl_active;
/* allow CxSR on this pipe */
bool cxsr_allowed;
} wm;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5b79c2f..04fc092 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2345,7 +2345,9 @@ static void ilk_compute_wm_config(struct drm_device *dev,
/* Compute the currently _active_ config */
for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_pipe_wm *wm = &intel_crtc->wm.active;
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
+ const struct intel_pipe_wm *wm = &cstate->wm.active.ilk;
if (!wm->pipe_enabled)
continue;
@@ -2442,7 +2444,9 @@ static void ilk_merge_wm_level(struct drm_device *dev,
ret_wm->enable = true;
for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_pipe_wm *active = &intel_crtc->wm.active;
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
+ const struct intel_pipe_wm *active = &cstate->wm.active.ilk;
const struct intel_wm_level *wm = &active->wm[level];
if (!active->pipe_enabled)
@@ -2590,14 +2594,15 @@ static void ilk_compute_wm_results(struct drm_device
*dev,
/* LP0 register values */
for_each_intel_crtc(dev, intel_crtc) {
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
enum pipe pipe = intel_crtc->pipe;
- const struct intel_wm_level *r =
- &intel_crtc->wm.active.wm[0];
+ const struct intel_wm_level *r = &cstate->wm.active.ilk.wm[0];
if (WARN_ON(!r->enable))
continue;
- results->wm_linetime[pipe] = intel_crtc->wm.active.linetime;
+ results->wm_linetime[pipe] = cstate->wm.active.ilk.linetime;
results->wm_pipe[pipe] =
(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -3564,16 +3569,15 @@ static bool skl_update_pipe_
[Intel-gfx] [PATCH 05/13] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check
Determine whether we need to apply this workaround at atomic check time
and just set a flag that will be used by the main watermark update
routine.
Moving this workaround into the atomic framework reduces
ilk_update_sprite_wm() to just a standard watermark update, so drop it
completely and just ensure that ilk_update_wm() is called whenever a
sprite plane is updated in a way that would affect watermarks.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_atomic.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 40 +---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 35 +++
4 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 9336e80..244d6bb 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -100,6 +100,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
crtc_state->base.crtc = crtc;
+ crtc_state->disable_lp_wm = false;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 455b1bd..16e5cb6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11536,23 +11536,38 @@ retry:
static bool intel_wm_need_update(struct drm_plane *plane,
struct drm_plane_state *state)
{
- /* Update watermarks on tiling changes. */
+ struct intel_plane_state *new = to_intel_plane_state(state);
+ struct intel_plane_state *cur = to_intel_plane_state(plane->state);
+
+ /* Update watermarks on tiling or size changes. */
if (!plane->state->fb || !state->fb ||
plane->state->fb->modifier[0] != state->fb->modifier[0] ||
- plane->state->rotation != state->rotation)
- return true;
-
- if (plane->state->crtc_w != state->crtc_w)
+ plane->state->rotation != state->rotation ||
+ drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
+ drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
+ drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
+ drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
return true;
return false;
}
+static bool needs_scaling(struct intel_plane_state *state)
+{
+ int src_w = drm_rect_width(&state->src) >> 16;
+ int src_h = drm_rect_height(&state->src) >> 16;
+ int dst_w = drm_rect_width(&state->dst);
+ int dst_h = drm_rect_height(&state->dst);
+
+ return (src_w != dst_w || src_h != dst_h);
+}
+
int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
struct drm_plane_state *plane_state)
{
struct drm_crtc *crtc = crtc_state->crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc_state);
struct drm_plane *plane = plane_state->plane;
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -11563,7 +11578,6 @@ int intel_plane_atomic_calc_changes(struct
drm_crtc_state *crtc_state,
bool mode_changed = needs_modeset(crtc_state);
bool was_crtc_enabled = crtc->state->active;
bool is_crtc_enabled = crtc_state->active;
-
bool turn_off, turn_on, visible, was_visible;
struct drm_framebuffer *fb = plane_state->fb;
@@ -11681,11 +11695,23 @@ int intel_plane_atomic_calc_changes(struct
drm_crtc_state *crtc_state,
case DRM_PLANE_TYPE_CURSOR:
break;
case DRM_PLANE_TYPE_OVERLAY:
- if (turn_off && !mode_changed) {
+ /*
+* WaCxSRDisabledForSpriteScaling:ivb
+*
+* atomic.update_wm was already set above, so this flag will
+* take effect when we commit and program watermarks.
+*/
+ if (IS_IVYBRIDGE(dev) &&
+ needs_scaling(to_intel_plane_state(plane_state)) &&
+ !needs_scaling(old_plane_state)) {
+ cstate->disable_lp_wm = true;
+ } else if (turn_off && !mode_changed) {
intel_crtc->atomic.wait_vblank = true;
intel_crtc->atomic.update_sprite_watermarks |=
1 << i;
}
+
+ break;
}
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 81b7d77..da145b0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -455,6 +455,9 @@ struct intel_crtc_state {
/* w/a for waiting 2 vblanks during crtc enable */
[Intel-gfx] [PATCH 03/13] drm/i915/skl: Simplify wm structures slightly
A bunch of SKL watermark-related structures have the cursor plane as a
separate entry from the rest of the planes. If we just extend the
relevant plane arrays by one entry and use the top-most array element as
the cursor, it will simplify future patches.
There shouldn't be any functional change here; this is just shuffling
around how the data is stored in some of the data structures. The whole
patch is generated with Coccinelle via the following semantic patch:
@@ struct skl_pipe_wm_parameters WMP; @@
- WMP.cursor
+ WMP.plane[I915_MAX_PLANES]
@@ struct skl_pipe_wm_parameters *WMP; @@
- WMP->cursor
+ WMP->plane[I915_MAX_PLANES]
@@ @@
struct skl_pipe_wm_parameters {
...
- struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
+ struct intel_plane_wm_parameters plane[I915_MAX_PLANES+1];
- struct intel_plane_wm_parameters cursor;
...
};
@@
struct skl_ddb_allocation DDB;
expression E;
@@
- DDB.cursor[E]
+ DDB.plane[E][I915_MAX_PLANES]
@@
struct skl_ddb_allocation *DDB;
expression E;
@@
- DDB->cursor[E]
+ DDB->plane[E][I915_MAX_PLANES]
@@ @@
struct skl_ddb_allocation {
...
- struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES];
+ struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES+1];
...
- struct skl_ddb_entry cursor[I915_MAX_PIPES];
...
};
@@
struct skl_wm_values WMV;
expression E1, E2;
@@
(
- WMV.cursor[E1][E2]
+ WMV.plane[E1][I915_MAX_PLANES][E2]
|
- WMV.cursor_trans[E1]
+ WMV.plane_trans[E1][I915_MAX_PLANES]
)
@@
struct skl_wm_values *WMV;
expression E1, E2;
@@
(
- WMV->cursor[E1][E2]
+ WMV->plane[E1][I915_MAX_PLANES][E2]
|
- WMV->cursor_trans[E1]
+ WMV->plane_trans[E1][I915_MAX_PLANES]
)
@@ @@
struct skl_wm_values {
...
- uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
+ uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES+1][8];
...
- uint32_t cursor[I915_MAX_PIPES][8];
...
- uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
+ uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES+1];
...
- uint32_t cursor_trans[I915_MAX_PIPES];
...
};
@@ struct skl_wm_level WML; @@
(
- WML.cursor_en
+ WML.plane_en[I915_MAX_PLANES]
|
- WML.cursor_res_b
+ WML.plane_res_b[I915_MAX_PLANES]
|
- WML.cursor_res_l
+ WML.plane_res_l[I915_MAX_PLANES]
)
@@ struct skl_wm_level *WML; @@
(
- WML->cursor_en
+ WML->plane_en[I915_MAX_PLANES]
|
- WML->cursor_res_b
+ WML->plane_res_b[I915_MAX_PLANES]
|
- WML->cursor_res_l
+ WML->plane_res_l[I915_MAX_PLANES]
)
@@ @@
struct skl_wm_level {
...
- bool plane_en[I915_MAX_PLANES];
+ bool plane_en[I915_MAX_PLANES+1];
...
- bool cursor_en;
...
- uint16_t plane_res_b[I915_MAX_PLANES];
+ uint16_t plane_res_b[I915_MAX_PLANES+1];
...
- uint8_t plane_res_l[I915_MAX_PLANES];
+ uint8_t plane_res_l[I915_MAX_PLANES+1];
...
- uint16_t cursor_res_b;
- uint8_t cursor_res_l;
...
};
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 20 +++-
drivers/gpu/drm/i915/intel_display.c | 4 +-
drivers/gpu/drm/i915/intel_pm.c | 89 +++-
4 files changed, 56 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 7a28de5..54508f1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3148,7 +3148,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
skl_ddb_entry_size(entry));
}
- entry = &ddb->cursor[pipe];
+ entry = &ddb->plane[pipe][I915_MAX_PLANES];
seq_printf(m, " %-13s%8u%8u%8u\n", "Cursor", entry->start,
entry->end, skl_ddb_entry_size(entry));
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e0f3f05..f04b67e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1571,28 +1571,22 @@ static inline bool skl_ddb_entry_equal(const struct
skl_ddb_entry *e1,
struct skl_ddb_allocation {
struct skl_ddb_entry pipe[I915_MAX_PIPES];
- struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MA
[Intel-gfx] [PATCH 00/13] Atomic watermark updates (v3)
Previous atomic watermark patches were posted here: http://lists.freedesktop.org/archives/intel-gfx/2015-July/070465.html Key changes since the last series: * Quite a bit of rebasing; I got pulled away from working on this for a while, so parts of the upstream code evolved a bit before I was able to get back to working on this. * Completely drop the async aspect of writing the final watermarks on platforms that need two-step programming. Although this is the direction we want to go eventually, Maarten has indicated that what I was doing in previous patch sets was going to lead to conflicts with some of his in-progress work and asked that I just write the final watermarks at the very end of the atomic transaction, after waiting for vblanks. We can revisit the async final step again after Maarten's work lands and leverage the new interfaces he adds. In the meantime, this simplifies things a bit since we don't need to worry about async final watermark writing racing with our next transaction and clobbering the intermediate values for that next transaction that we've already written. * Early SKL-style atomic watermarks (completely untested at the moment!). Even though gen9 doesn't need the two-step programming process, we now pre-compute gen9 watermark values during the check phase and write+flush them during commit. However I think there's still more refactoring that should be eventually done here. We trigger watermark updates on a per-crtc basis, but the end results are global, not per-crtc. In an atomic transaction this could currently lead to us re-calculating the same values multiple times if multiple CRTC's all trigger a WM update. Note that I haven't had a chance to run any of this on real gen9 hardware yet, so all of the SKL patches here should be considered RFC at best; they may be completely broken. * Various review feedback from Daniel, Ville, and Maarten from previous iterations. Matt Roper (12): drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM drm/i915/skl: Simplify wm structures slightly drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check drm/i915: Drop intel_update_sprite_watermarks drm/i915: Move active watermarks into CRTC state (v2) drm/i915: Calculate ILK-style watermarks during atomic check (v2) drm/i915: Calculate watermark configuration during atomic check drm/i915: Add two-stage ILK-style watermark programming (v3) drm/i915/skl: Switch to atomic watermark programming drm/i915/skl: Clarify pending vs hw watermark values Ville Syrjälä (1): drm/i915: Refactor ilk_update_wm (v3) drivers/gpu/drm/i915/i915_debugfs.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 68 ++- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_display.c | 184 +++- drivers/gpu/drm/i915/intel_drv.h | 81 +++- drivers/gpu/drm/i915/intel_pm.c | 860 --- drivers/gpu/drm/i915/intel_sprite.c | 15 - 7 files changed, 659 insertions(+), 554 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
Hi Joonas,
Thanks for the review! And my reply inline.
Regards,
-Zhiyuan
On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> Hi,
>
> On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > Hi Chris,
> > > >
> > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > Broadwell hardware supports both ring buffer mode and
> > > > > > execlist mode.
> > > > > > When i915 runs inside a VM with Intel GVT-g, we allow
> > > > > > execlist mode
> > > > > > only. The reason is that GVT-g does not support the dynamic
> > > > > > mode
> > > > > > switch between ring buffer mode and execlist mode when
> > > > > > running
> > > > > > multiple virtual machines. Consider that ring buffer mode is
> > > > > > legacy
> > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > >
> > > > > If that is the case, you should query the host as to what mode
> > > > > it is
> > > > > running.
> > > >
> > > > Thanks for the reply! You mean we query the host mode, then tell
> > > > the
> > > > guest driver inside VM, so that it could use the same mode as
> > > > host
> > > > right? That might be a little complicated, and the only benefit
> > > > is to
> > > > support legacy ring buffer mode ...
> > >
> > > The only benefit being that the guest works no matter what the host
> > > does?
> >
> > Supporting ring buffer mode may need more work in GVT-g. When we
> > started to
> > enable BDW support, ring buffer mode used to work but was not well
> > tested. And
> > the inter-VM switch (all in ringbuffer mode) may be tricker comparing
> > with
> > driver context switch with "MI_SET_CONTEXT", because we need to
> > switch ring
> > buffer whereas driver does not. Regarding this, the EXECLIST mode
> > looks
> > cleaner. In order to support that, we may have to: 1, change more LRI
> > commands
> > to MMIO in current driver; 2, more testing/debugging of inter-VM
> > context
> > switch flow.
> >
> > Based on that, I think we should really make statement that "ring
> > buffer mode"
> > is not supported by GVT-g on BDW :-)
> >
>
> I think just move the vpgu test even before test for GEN9 just making
> it return true on intel_vgpu_active(dev) and add a comment that
> currently vGPU only supports execlist command submission, and then add
Will do. Thanks!
> an error early in the init in some appropriate spot if
> intel_vgpu_active is true but logical ring context are not available
> (which would practically mean GEN < 8).
>
> Does that sound OK?
Ring buffer mode for Haswell with vgpu is still supported. So probably I have
the check like below? Thanks!
@@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device *dev)
if (WARN_ON(dev_priv->ring[RCS].default_context))
return 0;
+ if (intel_vgpu_active(dev)) {
+ if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
+ !i915.enable_execlist))
+ return 0;
+ }
+
if (i915.enable_execlists) {
/* NB: intentionally left blank. We will allocate our own
* backing objects as we need them, thank you very much */
>
> Regards, Joonas
>
> > > -Chris
> > >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Update PV INFO page definition for Intel GVT-g
On Thu, Aug 20, 2015 at 03:58:43PM +0300, Joonas Lahtinen wrote:
> On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> > Some more definitions in the PV info page are added. They are mainly
> > for the guest notification to Intel GVT-g device model. They are used
> > for Broadwell enabling.
> >
> > Signed-off-by: Zhiyuan Lv
> > Signed-off-by: Zhi Wang
> >
>
> Reviewed-by: Joonas Lahtinen
>
> Is there any public document about the interface?
So far no ... The information we got is that the 4K MMIO range from 0x78000
is reserved for virtualization usage, but the detailed definition will not
appear in spec. Thanks!
>
> > ---
> > drivers/gpu/drm/i915/i915_vgpu.h | 34
> > --
> > 1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.h
> > b/drivers/gpu/drm/i915/i915_vgpu.h
> > index 97a88b5..21c97f4 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.h
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> > @@ -40,6 +40,19 @@
> > #define INTEL_VGT_IF_VERSION \
> > INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR,
> > VGT_VERSION_MINOR)
> >
> > +/*
> > + * notifications from guest to vgpu device model
> > + */
> > +enum vgt_g2v_type {
> > + VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> > + VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> > + VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> > + VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> > + VGT_G2V_EXECLIST_CONTEXT_CREATE,
> > + VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> > + VGT_G2V_MAX,
> > +};
> > +
> > struct vgt_if {
> > uint64_t magic; /* VGT_MAGIC */
> > uint16_t version_major;
> > @@ -70,11 +83,28 @@ struct vgt_if {
> > uint32_t rsv3[0x200 - 24]; /* pad to half page */
> > /*
> > * The bottom half page is for response from Gfx driver to
> > hypervisor.
> > -* Set to reserved fields temporarily by now.
> > */
> > uint32_t rsv4;
> > uint32_t display_ready; /* ready for display owner
> > switch */
> > - uint32_t rsv5[0x200 - 2]; /* pad to one page */
> > +
> > + uint32_t rsv5[4];
> > +
> > + uint32_t g2v_notify;
> > + uint32_t rsv6[7];
> > +
> > + uint32_t pdp0_lo;
> > + uint32_t pdp0_hi;
> > + uint32_t pdp1_lo;
> > + uint32_t pdp1_hi;
> > + uint32_t pdp2_lo;
> > + uint32_t pdp2_hi;
> > + uint32_t pdp3_lo;
> > + uint32_t pdp3_hi;
> > +
> > + uint32_t execlist_context_descriptor_lo;
> > + uint32_t execlist_context_descriptor_hi;
> > +
> > + uint32_t rsv7[0x200 - 24];/* pad to one page */
> > } __packed;
> >
> > #define vgtif_reg(x) \
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: guest i915 notification for Intel-GVTg
On Thu, Aug 20, 2015 at 04:11:57PM +0300, Joonas Lahtinen wrote:
> Hi,
>
> Notes below.
>
> On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> > When i915 drivers run inside a VM with Intel-GVTg, some explicit
> > notifications are needed from guest to host device model through PV
> > INFO page write. The notifications include:
> >
> > PPGTT create/destroy
> > EXECLIST create/destroy
> >
> > They are used for the shadow implementation of PPGTT and EXECLIST
> > context. Intel GVT-g needs to write-protect the guest pages of PPGTT
> > and contexts, and clear the write protection when they end their life
> > cycle.
> >
> > Signed-off-by: Zhiyuan Lv
> > Signed-off-by: Zhi Wang
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 41
> > +
> > drivers/gpu/drm/i915/intel_lrc.c| 25 ++
> > 2 files changed, 66 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 823005c..00dafb0 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -896,6 +896,41 @@ static int gen8_init_scratch(struct
> > i915_address_space *vm)
> > return 0;
> > }
> >
> > +static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool
> > create)
> > +{
> > + enum vgt_g2v_type msg;
> > + struct drm_device *dev = ppgtt->base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + unsigned int offset = vgtif_reg(pdp0_lo);
> > + int i;
> > +
> > + if (USES_FULL_48BIT_PPGTT(dev)) {
>
> With regards the patch "preallocate pdps for 32 bit vgpu", is this code
> path ever taken?
48-bit PPGTT will create root pml4 table in gen8_ppgtt_init(), and
after that, the PPGTT structure is complete, so we still need the
notification.
I did not tested the path though, because I found currently
i915.enable_ppgtt will not equal to 3. The whole 48-bit PPGTT support
in GVT-g is verified with windows VM. Thanks!
>
> > + u64 daddr = px_dma(&ppgtt->pml4);
> > +
> > + I915_WRITE(offset, daddr & 0x);
> > + I915_WRITE(offset + 4, daddr >> 32);
> > +
> > + msg = (create ? VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE :
> > + VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY)
> > ;
> > + } else {
> > + for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
> > + u64 daddr = i915_page_dir_dma_addr(ppgtt,
> > i);
> > +
> > + I915_WRITE(offset, daddr & 0x);
> > + I915_WRITE(offset + 4, daddr >> 32);
> > +
> > + offset += 8;
> > + }
> > +
> > + msg = (create ? VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE :
> > + VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY)
> > ;
> > + }
> > +
> > + I915_WRITE(vgtif_reg(g2v_notify), msg);
> > +
> > + return 0;
> > +}
> > +
> > static void gen8_free_scratch(struct i915_address_space *vm)
> > {
> > struct drm_device *dev = vm->dev;
> > @@ -942,6 +977,9 @@ static void gen8_ppgtt_cleanup(struct
> > i915_address_space *vm)
> > struct i915_hw_ppgtt *ppgtt =
> > container_of(vm, struct i915_hw_ppgtt, base);
> >
> > + if (intel_vgpu_active(vm->dev))
> > + gen8_ppgtt_notify_vgt(ppgtt, false);
> > +
> > if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev))
> > gen8_ppgtt_cleanup_3lvl(ppgtt->base.dev, &ppgtt
> > ->pdp);
> > else
> > @@ -1516,6 +1554,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
> > *ppgtt)
> > }
> > }
> >
> > + if (intel_vgpu_active(ppgtt->base.dev))
> > + gen8_ppgtt_notify_vgt(ppgtt, true);
> > +
> > return 0;
> >
> > free_scratch:
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index 4b2ac37..80d424b 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -136,6 +136,7 @@
> > #include
> > #include "i915_drv.h"
> > #include "intel_mocs.h"
> > +#include "i915_vgpu.h"
> >
> > #define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
> > #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
> > @@ -2122,6 +2123,22 @@ make_rpcs(struct drm_device *dev)
> > return rpcs;
> > }
> >
> > +static void intel_lr_context_notify_vgt(struct intel_context *ctx,
> > + struct intel_engine_cs
> > *ring,
> > + int msg)
> > +{
> > + struct drm_device *dev = ring->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + u64 tmp = intel_lr_context_descriptor(ctx, ring);
> > +
> > + I915_WRITE(vgtif_reg(execlist_context_descriptor_lo),
> > + tmp & 0x);
> > + I915_WRITE(vgtif_reg(execlist_context_descriptor_hi),
> > + tmp >> 32);
> > +
> > + I915_WRITE(vgtif_reg(g2v_notify), msg);
> > +}
> > +
>
> Why the other interface has bool for action a
Re: [Intel-gfx] [iGVT-g] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
> From: iGVT-g [mailto:igvt-g-boun...@lists.01.org] On Behalf Of Zhiyuan Lv > Sent: Thursday, August 20, 2015 5:40 PM > > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote: > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote: > > > Hi Chris, > > > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote: > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: > > > > > Broadwell hardware supports both ring buffer mode and execlist mode. > > > > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode > > > > > only. The reason is that GVT-g does not support the dynamic mode > > > > > switch between ring buffer mode and execlist mode when running > > > > > multiple virtual machines. Consider that ring buffer mode is legacy > > > > > mode, it makes sense to drop it inside virtual machines. > > > > > > > > If that is the case, you should query the host as to what mode it is > > > > running. > > > > > > Thanks for the reply! You mean we query the host mode, then tell the > > > guest driver inside VM, so that it could use the same mode as host > > > right? That might be a little complicated, and the only benefit is to > > > support legacy ring buffer mode ... > > > > The only benefit being that the guest works no matter what the host > > does? > > Supporting ring buffer mode may need more work in GVT-g. When we started to > enable BDW support, ring buffer mode used to work but was not well tested. And > the inter-VM switch (all in ringbuffer mode) may be tricker comparing with > driver context switch with "MI_SET_CONTEXT", because we need to switch ring > buffer whereas driver does not. Regarding this, the EXECLIST mode looks > cleaner. In order to support that, we may have to: 1, change more LRI commands > to MMIO in current driver; 2, more testing/debugging of inter-VM context > switch flow. > > Based on that, I think we should really make statement that "ring buffer mode" > is not supported by GVT-g on BDW :-) > I think the primary reason is that Windows gfx driver only uses execution list mode. They don't have plan to add back ring buffer mode. So we have to assume only execution list for all guests powered by GVT-g (being that dynamic mode switch is not feasible). Thanks Kevin ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g
Hi Chris, If we cannot always pin lr context into GGTT, the LRCA cannot be used as a context identifier for us. Then we have to consider a proper interface for i915 in VM to notify GVT-g device model. The context creation might be OK. We can pin context first, then notify the context creation, after that, unpin the context. In GVT-g, we can get the context's guest physical addresses from GTT table, and use that to identify a guest context. But the destroy can be a problem. It does not make sense to pin context and unpin that time right? Do you have any suggestions/comments? Thanks in advance! Regards, -Zhiyuan On Thu, Aug 20, 2015 at 05:16:54PM +0800, Zhiyuan Lv wrote: > Hi Chris, > > On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote: > > On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: > > > Intel GVT-g will perform EXECLIST context shadowing and ring buffer > > > shadowing. The shadow copy is created when guest creates a context. > > > If a context changes its LRCA address, the hypervisor is hard to know > > > whether it is a new context or not. We always pin context objects to > > > global GTT to make life easier. > > > > Nak. Please explain why we need to workaround a bug in the host. We > > cannot pin the context as that breaks userspace (e.g. synmark) who can > > and will try to use more contexts than we have room. > > This change is only for i915 running inside a VM. Host i915 driver's > behavior will not be impacted. > > The reason we want to pin context is that our hypervisor identifies > guest contexts with their LRCA, and need LRCA unchanged during > contexts' life cycle so that shadow copy of contexts can easily find > their counterparts. If we cannot pin them, we have to consider more > complicated shadow implementation ... > > BTW Chris, are there many apps like synmark that may used up GGTT with > contexts if they are all pinned? Thanks! > > Regards, > -Zhiyuan > > > -Chris > > > > -- > > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
On Thu, 20 Aug 2015, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We are no longer checkling the DP link status on long hpd. We used to do
> that from the .hot_plug() handler, but it was removed when MST got
> introduced.
>
> If there's no userspace we now fail to retrain the link if the sink
> power is toggled (or cable yanked and replugged), meaning the user is
> left staring at a blank screen. With the retraining put back that should
> be fixed.
>
> Also remove the leftover comment that referred to the old retraining
> from .hot_plug().
>
> Fixes a regression introduced in:
> commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> Author: Dave Airlie
> Date: Fri May 2 14:02:48 2014 +1000
>
> drm/i915: add DP 1.2 MST support (v0.7)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
Tested-by: Palmer Dabbelt
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> Cc: Dave Airlie
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d32ce48..b014158 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>
> intel_dp_probe_oui(intel_dp);
>
> - if (!intel_dp_probe_mst(intel_dp))
> + if (!intel_dp_probe_mst(intel_dp)) {
> + drm_modeset_lock(&dev->mode_config.connection_mutex,
> NULL);
> + intel_dp_check_link_status(intel_dp);
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> goto mst_fail;
> -
> + }
> } else {
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
> }
>
> if (!intel_dp->is_mst) {
> - /*
> - * we'll check the link status via the normal hot plug
> path later -
> - * but for short hpds we should check it now
> - */
> drm_modeset_lock(&dev->mode_config.connection_mutex,
> NULL);
> intel_dp_check_link_status(intel_dp);
> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> --
> 2.4.6
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
