[Intel-gfx] [PATCH 5/6] drm/i915: Squash gen lookup through multiple indirections inside GT access
The INTEL_INFO() macro extracts the dev_private pointer from the device,
so passing in the dev_private->dev is a long winded circumlocution.
v2: rebase onto uncore
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 40726ab..2c39467 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -349,7 +349,7 @@ u##x i915_read##x(struct drm_i915_private *dev_priv, u32
reg, bool trace) { \
unsigned long irqflags; \
u##x val = 0; \
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
- if (IS_GEN5(dev_priv->dev)) \
+ if (dev_priv->info->gen == 5) \
ilk_dummy_write(dev_priv); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
if (dev_priv->uncore.forcewake_count == 0) \
@@ -380,7 +380,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32
reg, u##x val, bool tr
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
- if (IS_GEN5(dev_priv->dev)) \
+ if (dev_priv->info->gen == 5) \
ilk_dummy_write(dev_priv); \
hsw_unclaimed_reg_clear(dev_priv, reg); \
__raw_i915_write##x(dev_priv, reg, val); \
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: extend lpt_enable_clkout_dp
From: Paulo Zanoni
Now it implements 3 different sequences from BSpec and also has
support for ULT.
v2: - Change IS_ULT checks for LPT-LP checks
- Add check for LPT-LP + with_fdi (Ben)
- Merge DBUFF0/GEN0 bit definitions since they're the same
register (Ben)
- DBUFF0 (1<<0) is Disable, not Enable
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_reg.h | 3 ++-
drivers/gpu/drm/i915/intel_display.c | 43 +---
2 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e20f093..0dfcbad 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4942,7 +4942,8 @@
#define SBI_SSCAUXDIV60x0610
#define SBI_SSCAUXDIV_FINALDIV2SEL(x)((x)<<4)
#define SBI_DBUFF00x2a00
-#define SBI_DBUFF0_ENABLE(1<<0)
+#define SBI_GEN0 0x1f00
+#define SBI_GEN0_CFG_BUFFENABLE_DISABLE (1<<0)
/* LPT PIXCLK_GATE */
#define PIXCLK_GATE0xC6020
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c9e9ad0..3f31ca0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5260,11 +5260,23 @@ static void lpt_program_fdi_mphy(struct
drm_i915_private *dev_priv)
intel_sbi_write(dev_priv, 0x21EC, tmp, SBI_MPHY);
}
-/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
-static void lpt_enable_clkout_dp(struct drm_device *dev)
+/* Implements 3 different sequences from BSpec chapter "Display iCLK
+ * Programming" based on the parameters passed:
+ * - Sequence to enable CLKOUT_DP
+ * - Sequence to enable CLKOUT_DP without spread
+ * - Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O
+ */
+static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
+bool with_fdi)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- uint32_t tmp;
+ uint32_t reg, tmp;
+
+ if (WARN(with_fdi && !with_spread, "FDI requires downspread\n"))
+ with_spread = true;
+ if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE &&
+with_fdi, "LP PCH doesn't have FDI\n"))
+ with_fdi = false;
mutex_lock(&dev_priv->dpio_lock);
@@ -5275,17 +5287,22 @@ static void lpt_enable_clkout_dp(struct drm_device *dev)
udelay(24);
- tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
- tmp &= ~SBI_SSCCTL_PATHALT;
- intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
+ if (with_spread) {
+ tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
+ tmp &= ~SBI_SSCCTL_PATHALT;
+ intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
- lpt_reset_fdi_mphy(dev_priv);
- lpt_program_fdi_mphy(dev_priv);
+ if (with_fdi) {
+ lpt_reset_fdi_mphy(dev_priv);
+ lpt_program_fdi_mphy(dev_priv);
+ }
+ }
- /* ULT uses SBI_GEN0, but ULT doesn't have VGA, so we don't care. */
- tmp = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
- tmp |= SBI_DBUFF0_ENABLE;
- intel_sbi_write(dev_priv, SBI_DBUFF0, tmp, SBI_ICLK);
+ reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
+ SBI_GEN0 : SBI_DBUFF0;
+ tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
+ tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE;
+ intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
mutex_unlock(&dev_priv->dpio_lock);
}
@@ -5307,7 +5324,7 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
if (!has_vga)
return;
- lpt_enable_clkout_dp(dev);
+ lpt_enable_clkout_dp(dev, true, true);
}
/*
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] drm/i915: add some assertions to hsw_disable_lcpll
2013/7/18 Ben Widawsky :
> On Fri, Jul 12, 2013 at 02:19:42PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni
>>
>> Most of the hardware needs to be disabled before LCPLL is disabled, so
>> let's add a function to assert some of items listed in the "Display
>> Sequences for LCPLL disabling" documentation.
>>
>> The idea is that hsw_disable_lcpll should not disable the hardware,
>> the callers need to take care of calling hsw_disable_lcpll only once
>> everything is already disabled.
>>
>> Signed-off-by: Paulo Zanoni
>
> I don't see a reason to separate this patch from the previous one. It
> makes review easier, but it's weird bisect wise. The correct order, if
> you wanted to do them as separate patches would be to introduce the
> assertions, and then add the functionality (I think).
Originally this code was part of other function. I agree that the way
things look now, it should be on the same patch.
>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 8
>> drivers/gpu/drm/i915/intel_display.c | 28
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 8e5a5ec..9556dff 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2195,6 +2195,8 @@
>> #define BLC_PWM_CPU_CTL2 0x48250
>> #define BLC_PWM_CPU_CTL 0x48254
>>
>> +#define HSW_BLC_PWM2_CTL 0x48350
>> +
>> /* PCH CTL1 is totally different, all but the below bits are reserved. CTL2
>> is
>> * like the normal CTL from gen4 and earlier. Hooray for confusing naming.
>> */
>> #define BLC_PWM_PCH_CTL1 0xc8250
>> @@ -2203,6 +2205,12 @@
>> #define BLM_PCH_POLARITY (1 << 29)
>> #define BLC_PWM_PCH_CTL2 0xc8254
>>
>> +#define UTIL_PIN_CTL 0x48400
>> +#define UTIL_PIN_ENABLE(1 << 31)
>> +
>> +#define PCH_GTC_CTL 0xe7000
>> +#define PCH_GTC_ENABLE (1 << 31)
>> +
>> /* TV port control */
>> #define TV_CTL 0x68000
>> /** Enables the TV encoder */
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index ffb08bf..9055f60 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5922,6 +5922,32 @@ static bool ironlake_get_pipe_config(struct
>> intel_crtc *crtc,
>> return true;
>> }
>>
>> +static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
>> + struct intel_crtc *crtc;
>> +
>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
>> + WARN(crtc->base.enabled, "CRTC for pipe %c enabled\n",
>> + pipe_name(crtc->pipe));
>> +
>> + WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
>> + WARN(plls->spll_refcount, "SPLL enabled\n");
>> + WARN(plls->wrpll1_refcount, "WRPLL1 enabled\n");
>> + WARN(plls->wrpll2_refcount, "WRPLL2 enabled\n");
>> + WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
>> + WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
>> + "CPU PWM1 enabled\n");
>> + WARN(I915_READ(HSW_BLC_PWM2_CTL) & BLM_PWM_ENABLE,
>> + "CPU PWM2 enabled\n");
>> + WARN(I915_READ(BLC_PWM_PCH_CTL1) & BLM_PCH_PWM_ENABLE,
>> + "PCH PWM1 enabled\n");
>> + WARN(I915_READ(UTIL_PIN_CTL) & UTIL_PIN_ENABLE,
>> + "Utility pin enabled\n");
>> + WARN(I915_READ(PCH_GTC_CTL) & PCH_GTC_ENABLE, "PCH GTC enabled\n");
>
> Looking at probably the same list you've looked at, I don't see:
> Audio controller
The audio registers go away when we disable the power well, which is
part of the assertion.
> eDP HPD
> Other CPU/PCH interrupts
I guess these ones deserve to be part of the assertion, but checking
interrupts is dangerous and racy...I'll try to find a good solution.
>
> You've worked with this code longer than I have, so maybe you can
> explain why you've skipped them.
>
>> +}
>> +
>> /*
>> * This function implements pieces of two sequences from BSpec:
>> * - Sequence for display software to disable LCPLL
>> @@ -5935,6 +5961,8 @@ void hsw_disable_lcpll(struct drm_i915_private
>> *dev_priv,
>> {
>> uint32_t val;
>>
>> + assert_can_disable_lcpll(dev_priv);
>> +
>
> Do we want to proceed if the above fails? I was sort of under the
> impression that hard hangs can occur as a result of some functions still
> being enabled when we change clocks. I'm fine with continuing on since
> we have the WARNS, just wondering if you've thought about it.
This function should should be the last thing on a big sequence that
involves disabling all the outputs, rearranging the interrupts, etc.
If we detect a problem here, we can't just return, the caller would
have to unwind everything it did. Anyway, this is really something we
don't expect and shoul
[Intel-gfx] [PATCH] drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941
BugLink: https://bugs.launchpad.net/bugs/1163720
BugLink: https://bugs.launchpad.net/bugs/1162026
Some machines suffer from non-functional backlight controls if
BLM_PCH_PWM_ENABLE is set, so provide a quirk to avoid doing so.
Apply this quirk to Dell XPS 13 models.
Tested-by: Eric Griffith
Tested-by: Kent Baxley
Cc: # v3.8+
Signed-off-by: Kamal Mostafa
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 16
drivers/gpu/drm/i915/intel_panel.c | 3 ++-
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a416645..204c3ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -555,6 +555,7 @@ enum intel_sbi_destination {
#define QUIRK_PIPEA_FORCE (1<<0)
#define QUIRK_LVDS_SSC_DISABLE (1<<1)
#define QUIRK_INVERT_BRIGHTNESS (1<<2)
+#define QUIRK_NO_PCH_PWM_ENABLE (1<<3)
struct intel_fbdev;
struct intel_fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 85f3eb7..42e207e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9398,6 +9398,17 @@ static void quirk_invert_brightness(struct drm_device
*dev)
DRM_INFO("applying inverted panel brightness quirk\n");
}
+/*
+ * Some machines (Dell XPS13) suffer broken backlight controls if
+ * BLM_PCH_PWM_ENABLE is set.
+ */
+static void quirk_no_pcm_pwm_enable(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE;
+ DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n");
+}
+
struct intel_quirk {
int device;
int subsystem_vendor;
@@ -9467,6 +9478,11 @@ static struct intel_quirk intel_quirks[] = {
/* Acer Aspire 4736Z */
{ 0x2a42, 0x1025, 0x0260, quirk_invert_brightness },
+
+ /* Dell XPS13 HD Sandy Bridge */
+ { 0x0116, 0x1028, 0x052e, quirk_no_pcm_pwm_enable },
+ /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */
+ { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable },
};
static void intel_init_quirks(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 80bea1d..01b5a51 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -580,7 +580,8 @@ void intel_panel_enable_backlight(struct drm_device *dev,
POSTING_READ(reg);
I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
- if (HAS_PCH_SPLIT(dev)) {
+ if (HAS_PCH_SPLIT(dev) &&
+ !(dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)) {
tmp = I915_READ(BLC_PWM_PCH_CTL1);
tmp |= BLM_PCH_PWM_ENABLE;
tmp &= ~BLM_PCH_OVERRIDE_ENABLE;
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: add HAS_LP_PCH check
From: Paulo Zanoni
We have 2 possible LPT PCHs: the normal version, which contains the
pixel path (FDI, transcoders, VGA, etc), and the LP version, which
comes with ULT machines and doesn't contain the pixel path. Both
models return true for HAS_PCH_LPT.
We already have a few places where we explicitly check for LPT-LP, so
add a HAS_LP_PCH check to simplify the code.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_display.c | 9 +++--
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2885265..262c3d4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1578,6 +1578,8 @@ struct drm_i915_file_private {
#define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
#define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
#define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE)
+#define HAS_LP_PCH(dev_priv) ((dev_priv)->pch_id == \
+ INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
#define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c35a613..1d3c805 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5274,8 +5274,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev,
bool with_spread,
if (WARN(with_fdi && !with_spread, "FDI requires downspread\n"))
with_spread = true;
- if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE &&
-with_fdi, "LP PCH doesn't have FDI\n"))
+ if (WARN(HAS_LP_PCH(dev_priv) && with_fdi, "LP PCH doesn't have FDI\n"))
with_fdi = false;
mutex_lock(&dev_priv->dpio_lock);
@@ -5298,8 +5297,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev,
bool with_spread,
}
}
- reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
- SBI_GEN0 : SBI_DBUFF0;
+ reg = HAS_LP_PCH(dev_priv) ? SBI_GEN0 : SBI_DBUFF0;
tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE;
intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
@@ -5315,8 +5313,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
mutex_lock(&dev_priv->dpio_lock);
- reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
- SBI_GEN0 : SBI_DBUFF0;
+ reg = HAS_LP_PCH(dev_priv) ? SBI_GEN0 : SBI_DBUFF0;
tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE;
intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 43031ec..7643b16 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4650,7 +4650,7 @@ static void lpt_init_clock_gating(struct drm_device *dev)
* TODO: this bit should only be enabled when really needed, then
* disabled when not needed anymore in order to save power.
*/
- if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
+ if (HAS_LP_PCH(dev_priv))
I915_WRITE(SOUTH_DSPCLK_GATE_D,
I915_READ(SOUTH_DSPCLK_GATE_D) |
PCH_LP_PARTITION_LEVEL_DISABLE);
@@ -4665,7 +4665,7 @@ static void lpt_suspend_hw(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
+ if (HAS_LP_PCH(dev_priv)) {
uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D);
val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [v3] drm/i915: Make i915 events part of uapi
On 07/19/2013 09:16 AM, Ben Widawsky wrote: Make the uevent strings part of the user API for people who wish to write their own listeners. v2: Make a space in the string concatenation. (Chad) Use the "UEVENT" suffix intead of "EVENT" (Chad) Make kernel-doc parseable Docbook comments (Daniel) v3: Undid reset change introduced in last submission (Daniel) Fixed up comments to address removal changes. Thanks to Daniel Vetter for a majority of the parity error comments. CC: Chad Versace Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_irq.c | 8 include/uapi/drm/i915_drm.h | 24 2 files changed, 28 insertions(+), 4 deletions(-) For what it's worth, Reviewed-by: Chad Versace ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: add functions to disable and restore LCPLL
From: Paulo Zanoni
For now there are no callers, but these functions are going to be
needed for the code that allows Package C8+. Other future features may
also require this code.
Also merge the commit which introduced assert_can_disable_lcpll and
had the following commit message:
Most of the hardware needs to be disabled before LCPLL is disabled, so
let's add a function to assert some of items listed in the "Display
Sequences for LCPLL disabling" documentation.
The idea is that hsw_disable_lcpll should not disable the hardware,
the callers need to take care of calling hsw_disable_lcpll only once
everything is already disabled.
v2: - Rebase.
- Fix D_COMP wait timeout.
v3: - Use wait_for_atomic_use (Ben)
- Remove/add a useless/needed POSTING_READ (Ben)
- Early return in case LCPLL is already restored (Ben)
- Add ndelay(100) (Ben)
v4: - Merge the commit that added assert_can_disable_lcpll (Ben)
- Add interrupt assertions (Ben)
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_reg.h | 15
drivers/gpu/drm/i915/intel_display.c | 136 +++
drivers/gpu/drm/i915/intel_drv.h | 3 +
3 files changed, 154 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0dfcbad..6caa748 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2261,6 +2261,8 @@
#define BLC_PWM_CPU_CTL2 0x48250
#define BLC_PWM_CPU_CTL0x48254
+#define HSW_BLC_PWM2_CTL 0x48350
+
/* PCH CTL1 is totally different, all but the below bits are reserved. CTL2 is
* like the normal CTL from gen4 and earlier. Hooray for confusing naming. */
#define BLC_PWM_PCH_CTL1 0xc8250
@@ -2269,6 +2271,12 @@
#define BLM_PCH_POLARITY (1 << 29)
#define BLC_PWM_PCH_CTL2 0xc8254
+#define UTIL_PIN_CTL 0x48400
+#define UTIL_PIN_ENABLE (1 << 31)
+
+#define PCH_GTC_CTL0xe7000
+#define PCH_GTC_ENABLE (1 << 31)
+
/* TV port control */
#define TV_CTL 0x68000
/** Enables the TV encoder */
@@ -5009,7 +5017,14 @@
#define LCPLL_CLK_FREQ_450(0<<26)
#define LCPLL_CD_CLOCK_DISABLE(1<<25)
#define LCPLL_CD2X_CLOCK_DISABLE (1<<23)
+#define LCPLL_POWER_DOWN_ALLOW(1<<22)
#define LCPLL_CD_SOURCE_FCLK (1<<21)
+#define LCPLL_CD_SOURCE_FCLK_DONE (1<<19)
+
+#define D_COMP (MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
+#define D_COMP_RCOMP_IN_PROGRESS (1<<9)
+#define D_COMP_COMP_FORCE (1<<8)
+#define D_COMP_COMP_DISABLE (1<<0)
/* Pipe WM_LINETIME - watermark line time */
#define PIPE_WM_LINETIME_A 0x45270
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1b41178..c35a613 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5922,6 +5922,142 @@ static bool ironlake_get_pipe_config(struct intel_crtc
*crtc,
return true;
}
+static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
+ struct intel_crtc *crtc;
+ unsigned long irqflags;
+ uint32_t val, pch_hpd_mask;
+
+ pch_hpd_mask = SDE_PORTB_HOTPLUG_CPT | SDE_PORTC_HOTPLUG_CPT;
+ if (!HAS_LP_PCH(dev_priv))
+ pch_hpd_mask |= SDE_PORTD_HOTPLUG_CPT | SDE_CRT_HOTPLUG_CPT;
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
+ WARN(crtc->base.enabled, "CRTC for pipe %c enabled\n",
+pipe_name(crtc->pipe));
+
+ WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
+ WARN(plls->spll_refcount, "SPLL enabled\n");
+ WARN(plls->wrpll1_refcount, "WRPLL1 enabled\n");
+ WARN(plls->wrpll2_refcount, "WRPLL2 enabled\n");
+ WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
+ WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
+"CPU PWM1 enabled\n");
+ WARN(I915_READ(HSW_BLC_PWM2_CTL) & BLM_PWM_ENABLE,
+"CPU PWM2 enabled\n");
+ WARN(I915_READ(BLC_PWM_PCH_CTL1) & BLM_PCH_PWM_ENABLE,
+"PCH PWM1 enabled\n");
+ WARN(I915_READ(UTIL_PIN_CTL) & UTIL_PIN_ENABLE,
+"Utility pin enabled\n");
+ WARN(I915_READ(PCH_GTC_CTL) & PCH_GTC_ENABLE, "PCH GTC enabled\n");
+
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ val = I915_READ(DEIMR);
+ WARN((val & ~DE_PCH_EVENT_IVB) != val,
+"Unexpected DEIMR bits enabled: 0x%x\n", val);
+ val = I915_READ(SDEIMR);
+ WARN((val & ~pch_hpd_mask) != val,
+"Unexpected SDEIMR bits enabled: 0x%x\n", val);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
+/*
+ * This function implements pieces of two sequences from BSpec:
+ * - Seque
[Intel-gfx] [PATCH 4/5] drm/i915: invert {ilk, snb}_gt_irq_handler check
From: Paulo Zanoni
Requested by Chris Wilson on IRC.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 58ee826..f708e4e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1340,10 +1340,10 @@ static irqreturn_t ironlake_irq_handler(int irq, void
*arg)
gt_iir = I915_READ(GTIIR);
if (gt_iir) {
- if (IS_GEN5(dev))
- ilk_gt_irq_handler(dev, dev_priv, gt_iir);
- else
+ if (INTEL_INFO(dev)->gen >= 6)
snb_gt_irq_handler(dev, dev_priv, gt_iir);
+ else
+ ilk_gt_irq_handler(dev, dev_priv, gt_iir);
I915_WRITE(GTIIR, gt_iir);
ret = IRQ_HANDLED;
}
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: disable CLKOUT_DP when it's not needed
From: Paulo Zanoni
We currently don't support HDMI clock bending nor use SSC for DP or
HDMI on Haswell, so the only case where we need CLKOUT_DP is for VGA.
v2: - Replace the IS_ULT check for LPT-LP
- Simplify GEN0/DBUFF0 check due to change on the previous patch
- Also check for SBI_SSCCTL_DISABLE (Ben).
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_display.c | 36
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3f31ca0..1b41178 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5307,6 +5307,34 @@ static void lpt_enable_clkout_dp(struct drm_device *dev,
bool with_spread,
mutex_unlock(&dev_priv->dpio_lock);
}
+/* Sequence to disable CLKOUT_DP */
+static void lpt_disable_clkout_dp(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ uint32_t reg, tmp;
+
+ mutex_lock(&dev_priv->dpio_lock);
+
+ reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
+ SBI_GEN0 : SBI_DBUFF0;
+ tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
+ tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE;
+ intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
+
+ tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
+ if (!(tmp & SBI_SSCCTL_DISABLE)) {
+ if (!(tmp & SBI_SSCCTL_PATHALT)) {
+ tmp |= SBI_SSCCTL_PATHALT;
+ intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
+ udelay(32);
+ }
+ tmp |= SBI_SSCCTL_DISABLE;
+ intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
+ }
+
+ mutex_unlock(&dev_priv->dpio_lock);
+}
+
static void lpt_init_pch_refclk(struct drm_device *dev)
{
struct drm_mode_config *mode_config = &dev->mode_config;
@@ -5321,10 +5349,10 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
}
}
- if (!has_vga)
- return;
-
- lpt_enable_clkout_dp(dev, true, true);
+ if (has_vga)
+ lpt_enable_clkout_dp(dev, true, true);
+ else
+ lpt_disable_clkout_dp(dev);
}
/*
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] build: Provide an --enable-simulation configure option
On Fri, Jul 19, 2013 at 07:05:29PM +0200, Daniel Vetter wrote: > On Fri, Jul 19, 2013 at 05:49:48PM +0100, Damien Lespiau wrote: > > Remembering to set an environment variable is too much trouble, we can > > now specify --enable-simulation at configure time and always run the > > test suite tuned for simulation. > > > > Signed-off-by: Damien Lespiau > > tbh I'd vote for runtime detection, we could simply add a debugfs file on > the internal tree which exposes dev_priv->is_simulator. Or heck merge that > part upstream even. I like this better as well. I pushed up to patch 7/8. I'll come back for more. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: Convert the register access tracepoint to be conditional
The TRACE_EVENT_CONDITION is supposed to generate more efficient code
than if (cond) trace(), which is what we are currently using inside the
register access functions.
v2: Rebase onto uncore
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_trace.h | 8 +---
drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0513743..cc3e74a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1004,7 +1004,7 @@ static int gen6_drpc_info(struct seq_file *m)
}
gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS);
- trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4);
+ trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true);
rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
rcctl1 = I915_READ(GEN6_RC_CONTROL);
diff --git a/drivers/gpu/drm/i915/i915_trace.h
b/drivers/gpu/drm/i915/i915_trace.h
index 7d283b5..2933e2f 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -406,10 +406,12 @@ TRACE_EVENT(i915_flip_complete,
TP_printk("plane=%d, obj=%p", __entry->plane, __entry->obj)
);
-TRACE_EVENT(i915_reg_rw,
- TP_PROTO(bool write, u32 reg, u64 val, int len),
+TRACE_EVENT_CONDITION(i915_reg_rw,
+ TP_PROTO(bool write, u32 reg, u64 val, int len, bool trace),
- TP_ARGS(write, reg, val, len),
+ TP_ARGS(write, reg, val, len, trace),
+
+ TP_CONDITION(trace),
TP_STRUCT__entry(
__field(u64, val)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 2c39467..b2703db 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -361,7 +361,7 @@ u##x i915_read##x(struct drm_i915_private *dev_priv, u32
reg, bool trace) { \
val = __raw_i915_read##x(dev_priv, reg); \
} \
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
- if (trace) trace_i915_reg_rw(false, reg, val, sizeof(val)); \
+ trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
return val; \
}
@@ -375,7 +375,7 @@ __i915_read(64)
void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool
trace) { \
unsigned long irqflags; \
u32 __fifo_ret = 0; \
- if (trace) trace_i915_reg_rw(true, reg, val, sizeof(val)); \
+ trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: Use a private interface for register access within GT
The GT functions for enabling register access also need to occasionally
write to and read from registers. To avoid the potential recursion as we
modify the public interface to be stricter, introduce a private register
access API for the GT functions.
v2: Rebase
v3: Rebase onto uncore
v4: Use raw interfaces consistently so that we only use the low-level
readN functions from a single location.
Signed-off-by: Chris Wilson
Reviewed-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 22 +++---
drivers/gpu/drm/i915/intel_uncore.c | 136 +---
2 files changed, 90 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 73c84fc..3071de1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2130,22 +2130,20 @@ void intel_sbi_write(struct drm_i915_private *dev_priv,
u16 reg, u32 value,
int vlv_gpu_freq(int ddr_freq, int val);
int vlv_freq_opcode(int ddr_freq, int val);
-#define __i915_read(x, y) \
+#define __i915_read(x) \
u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg);
-
-__i915_read(8, b)
-__i915_read(16, w)
-__i915_read(32, l)
-__i915_read(64, q)
+__i915_read(8)
+__i915_read(16)
+__i915_read(32)
+__i915_read(64)
#undef __i915_read
-#define __i915_write(x, y) \
+#define __i915_write(x) \
void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x
val);
-
-__i915_write(8, b)
-__i915_write(16, w)
-__i915_write(32, l)
-__i915_write(64, q)
+__i915_write(8)
+__i915_write(16)
+__i915_write(32)
+__i915_write(64)
#undef __i915_write
#define I915_READ8(reg)i915_read8(dev_priv, (reg))
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index ba04282..93fcfa0 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -26,6 +26,21 @@
#define FORCEWAKE_ACK_TIMEOUT_MS 2
+#define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
+#define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__,
(dev_priv__)->regs + (reg__))
+
+#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs +
(reg__))
+#define __raw_i915_write16(dev_priv__, reg__, val__) writew(val__,
(dev_priv__)->regs + (reg__))
+
+#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs +
(reg__))
+#define __raw_i915_write32(dev_priv__, reg__, val__) writel(val__,
(dev_priv__)->regs + (reg__))
+
+#define __raw_i915_read64(dev_priv__, reg__) readq((dev_priv__)->regs +
(reg__))
+#define __raw_i915_write64(dev_priv__, reg__, val__) writeq(val__,
(dev_priv__)->regs + (reg__))
+
+#define __raw_posting_read(dev_priv__, reg__)
(void)__raw_i915_read32(dev_priv__, reg__)
+
+
static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
{
u32 gt_thread_status_mask;
@@ -38,26 +53,28 @@ static void __gen6_gt_wait_for_thread_c0(struct
drm_i915_private *dev_priv)
/* w/a for a sporadic read returning 0 by waiting for the GT
* thread to wake up.
*/
- if (wait_for_atomic_us((I915_READ_NOTRACE(GEN6_GT_THREAD_STATUS_REG) &
gt_thread_status_mask) == 0, 500))
+ if (wait_for_atomic_us((__raw_i915_read32(dev_priv,
GEN6_GT_THREAD_STATUS_REG) & gt_thread_status_mask) == 0, 500))
DRM_ERROR("GT thread status wait timed out\n");
}
static void __gen6_gt_force_wake_reset(struct drm_i915_private *dev_priv)
{
- I915_WRITE_NOTRACE(FORCEWAKE, 0);
- POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE
*/
+ __raw_i915_write32(dev_priv, FORCEWAKE, 0);
+ /* something from same cacheline, but !FORCEWAKE */
+ __raw_posting_read(dev_priv, ECOBUS);
}
static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
{
- if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1) == 0,
+ if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK) & 1) ==
0,
FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Timed out waiting for forcewake old ack to
clear.\n");
- I915_WRITE_NOTRACE(FORCEWAKE, 1);
- POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE
*/
+ __raw_i915_write32(dev_priv, FORCEWAKE, 1);
+ /* something from same cacheline, but !FORCEWAKE */
+ __raw_posting_read(dev_priv, ECOBUS);
- if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK) & 1),
+ if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK) & 1),
FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Timed out waiting for forcewake to ack request.\n");
@@ -67,9 +84,9 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private
*dev_priv)
static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
{
- I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0x));
+ __raw_i915_write
[Intel-gfx] [PATCH 4/6] drm/i915: Use the common register access functions for NOTRACE variants
Detangle the confusion that NOTRACE variants of the register read/write
routines were directly using the raw register access. We need for those
routines to reuse the common code for serializing register access and
ensuring the correct register power states. This is only possible now
that the only routines that required raw access use their own API.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 28 ++--
drivers/gpu/drm/i915/intel_uncore.c | 8
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3071de1..775a28d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2131,7 +2131,7 @@ int vlv_gpu_freq(int ddr_freq, int val);
int vlv_freq_opcode(int ddr_freq, int val);
#define __i915_read(x) \
- u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg);
+ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool
trace);
__i915_read(8)
__i915_read(16)
__i915_read(32)
@@ -2139,28 +2139,28 @@ __i915_read(64)
#undef __i915_read
#define __i915_write(x) \
- void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x
val);
+ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x
val, bool trace);
__i915_write(8)
__i915_write(16)
__i915_write(32)
__i915_write(64)
#undef __i915_write
-#define I915_READ8(reg)i915_read8(dev_priv, (reg))
-#define I915_WRITE8(reg, val) i915_write8(dev_priv, (reg), (val))
+#define I915_READ8(reg)i915_read8(dev_priv, (reg), true)
+#define I915_WRITE8(reg, val) i915_write8(dev_priv, (reg), (val), true)
-#define I915_READ16(reg) i915_read16(dev_priv, (reg))
-#define I915_WRITE16(reg, val) i915_write16(dev_priv, (reg), (val))
-#define I915_READ16_NOTRACE(reg) readw(dev_priv->regs + (reg))
-#define I915_WRITE16_NOTRACE(reg, val) writew(val, dev_priv->regs + (reg))
+#define I915_READ16(reg) i915_read16(dev_priv, (reg), true)
+#define I915_WRITE16(reg, val) i915_write16(dev_priv, (reg), (val), true)
+#define I915_READ16_NOTRACE(reg) i915_read16(dev_priv, (reg), false)
+#define I915_WRITE16_NOTRACE(reg, val) i915_write16(dev_priv, (reg), (val),
false)
-#define I915_READ(reg) i915_read32(dev_priv, (reg))
-#define I915_WRITE(reg, val) i915_write32(dev_priv, (reg), (val))
-#define I915_READ_NOTRACE(reg) readl(dev_priv->regs + (reg))
-#define I915_WRITE_NOTRACE(reg, val) writel(val, dev_priv->regs + (reg))
+#define I915_READ(reg) i915_read32(dev_priv, (reg), true)
+#define I915_WRITE(reg, val) i915_write32(dev_priv, (reg), (val), true)
+#define I915_READ_NOTRACE(reg) i915_read32(dev_priv, (reg), false)
+#define I915_WRITE_NOTRACE(reg, val) i915_write32(dev_priv, (reg), (val),
false)
-#define I915_WRITE64(reg, val) i915_write64(dev_priv, (reg), (val))
-#define I915_READ64(reg) i915_read64(dev_priv, (reg))
+#define I915_WRITE64(reg, val) i915_write64(dev_priv, (reg), (val), true)
+#define I915_READ64(reg) i915_read64(dev_priv, (reg), true)
#define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg)
#define POSTING_READ16(reg)(void)I915_READ16_NOTRACE(reg)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 93fcfa0..40726ab 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -345,7 +345,7 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv,
u32 reg)
}
#define __i915_read(x) \
-u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
+u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool trace) { \
unsigned long irqflags; \
u##x val = 0; \
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
@@ -361,7 +361,7 @@ u##x i915_read##x(struct drm_i915_private *dev_priv, u32
reg) { \
val = __raw_i915_read##x(dev_priv, reg); \
} \
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
- trace_i915_reg_rw(false, reg, val, sizeof(val)); \
+ if (trace) trace_i915_reg_rw(false, reg, val, sizeof(val)); \
return val; \
}
@@ -372,10 +372,10 @@ __i915_read(64)
#undef __i915_read
#define __i915_write(x) \
-void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
+void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool
trace) { \
unsigned long irqflags; \
u32 __fifo_ret = 0; \
- trace_i915_reg_rw(true, reg, val, sizeof(val)); \
+ if (trace) trace_i915_reg_rw(true, reg, val, sizeof(val)); \
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freede
[Intel-gfx] [PATCH 1/6] drm/i915: Serialize almost all register access
In theory, the different register blocks were meant to be only ever
touched when holding either the struct_mutex, mode_config.lock or even a
specific localised lock. This does not seem to be the case, and the
hardware reacts extremely badly if we attempt to concurrently access two
registers within the same cacheline.
The HSD suggests that we only need to do this workaround for display
range registers. However, upon review we need to serialize the multiple
stages in our register write functions - if only for preemption
protection.
Irrespective of the hardware requirements, the current io functions are
a little too loose with respect to the combination of pre- and
post-condition testing that we do in conjunction with the actual io. As
a result, we may be pre-empted and generate both false-postive and
false-negative errors.
Note well that this is a "90%" solution, there remains a few direct
users of ioread/iowrite which will be fixed up in the next few patches.
Since they are more invasive and that this simple change will prevent
almost all lockups on Haswell, we kept this patch simple to facilitate
backporting to stable.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63914
Signed-off-by: Chris Wilson
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/i915_drv.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 613e786..401502d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1271,21 +1271,21 @@ hsw_unclaimed_reg_check(struct drm_i915_private
*dev_priv, u32 reg)
#define __i915_read(x, y) \
u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
+ unsigned long irqflags; \
u##x val = 0; \
+ spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
if (IS_GEN5(dev_priv->dev)) \
ilk_dummy_write(dev_priv); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
- unsigned long irqflags; \
- spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
if (dev_priv->forcewake_count == 0) \
dev_priv->gt.force_wake_get(dev_priv); \
val = read##y(dev_priv->regs + reg); \
if (dev_priv->forcewake_count == 0) \
dev_priv->gt.force_wake_put(dev_priv); \
- spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
} else { \
val = read##y(dev_priv->regs + reg); \
} \
+ spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
trace_i915_reg_rw(false, reg, val, sizeof(val)); \
return val; \
}
@@ -1298,8 +1298,10 @@ __i915_read(64, q)
#define __i915_write(x, y) \
void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
+ unsigned long irqflags; \
u32 __fifo_ret = 0; \
trace_i915_reg_rw(true, reg, val, sizeof(val)); \
+ spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
@@ -1311,6 +1313,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32
reg, u##x val) { \
gen6_gt_check_fifodbg(dev_priv); \
} \
hsw_unclaimed_reg_check(dev_priv, reg); \
+ spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
}
__i915_write(8, b)
__i915_write(16, w)
--
1.8.3.2
___
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: add functions to disable and restore LCPLL
2013/7/18 Ben Widawsky :
> On Fri, Jul 12, 2013 at 02:19:41PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni
>>
>> For now there are no callers, but these functions are going to be
>> needed for the code that allows Package C8+. Other future features may
>> also require this code.
>>
>
> The thing that's missing from the patches is any sort of assertions
> about things being on before the disable sequence. Is this something we
> don't need to address?
I'll merge this patch with the next one when sending, so we'll have
our assertions.
>
>> Signed-off-by: Paulo Zanoni
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 7 +++
>> drivers/gpu/drm/i915/intel_display.c | 95
>>
>> drivers/gpu/drm/i915/intel_drv.h | 3 ++
>> 3 files changed, 105 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index be6164f..8e5a5ec 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4930,7 +4930,14 @@
>> #define LCPLL_CLK_FREQ_450 (0<<26)
>> #define LCPLL_CD_CLOCK_DISABLE (1<<25)
>> #define LCPLL_CD2X_CLOCK_DISABLE(1<<23)
>> +#define LCPLL_POWER_DOWN_ALLOW (1<<22)
>> #define LCPLL_CD_SOURCE_FCLK(1<<21)
>> +#define LCPLL_CD_SOURCE_FCLK_DONE (1<<19)
>
> Hmm... the doc I am looking at says
>
>> +
>> +#define D_COMP (MCHBAR_MIRROR_BASE_SNB +
>> 0x5F0C)
>> +#define D_COMP_RCOMP_IN_PROGRESS(1<<9)
>> +#define D_COMP_COMP_FORCE (1<<8)
>> +#define D_COMP_COMP_DISABLE (1<<0)
>>
>> /* Pipe WM_LINETIME - watermark line time */
>> #define PIPE_WM_LINETIME_A 0x45270
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 059c9a8..ffb08bf 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5922,6 +5922,101 @@ static bool ironlake_get_pipe_config(struct
>> intel_crtc *crtc,
>> return true;
>> }
>>
>> +/*
>> + * This function implements pieces of two sequences from BSpec:
>> + * - Sequence for display software to disable LCPLL
>> + * - Sequence for display software to allow package C8+
>> + * The steps implemented here are just the steps that actually touch the
>> LCPLL
>> + * register. Callers should take care of disabling all the display engine
>> + * functions, doing the mode unset, fixing interrupts, etc.
>> + */
>> +void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
>> +bool switch_to_fclk, bool allow_power_down)
>> +{
>> + uint32_t val;
>> +
>> + val = I915_READ(LCPLL_CTL);
>> +
>> + if (switch_to_fclk) {
>> + val |= LCPLL_CD_SOURCE_FCLK;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +
>> + udelay(1);
>> +
>> + val = I915_READ(LCPLL_CTL);
>> + if (!(val & LCPLL_CD_SOURCE_FCLK_DONE))
>> + DRM_ERROR("Switching to FCLK failed\n");
>
> wait_for_us(..., 1)?
Done.
>
>> + }
>> +
>> + val |= LCPLL_PLL_DISABLE;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +
>> + if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1))
>> + DRM_ERROR("LCPLL still locked\n");
>> +
>> + val = I915_READ(D_COMP);
>> + val |= D_COMP_COMP_DISABLE;
>> + I915_WRITE(D_COMP, val);
>> + POSTING_READ(D_COMP);
>> +
>> + udelay(2);
>
> ndelay(100)?
>
>> +
>> + val = I915_READ(D_COMP);
>> + if (val & D_COMP_RCOMP_IN_PROGRESS)
>> + DRM_ERROR("D_COMP RCOMP still in progress\n");
>
> wait_for(..., 1)?
You reviewed v1. In v2 this wait_for was already there. But I re-added
the ndelay(100).
>
>> +
>> + if (allow_power_down) {
>> + val = I915_READ(LCPLL_CTL);
>> + val |= LCPLL_POWER_DOWN_ALLOW;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> + }
>> +}
>> +
>> +/*
>> + * Fully restores LCPLL, disallowing power down and switching back to LCPLL
>> + * source.
>> + */
>> +void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + val = I915_READ(LCPLL_CTL);
>> +
>
> I think we could potentially exit early here if the PLL is already
> locked, and we're on CDclk. And indeed, I've already seen this case
> occur, but I'm not sure I will ever see that case again.
Makes sense. Done.
>
>> + if (val & LCPLL_POWER_DOWN_ALLOW) {
>> + val &= ~LCPLL_POWER_DOWN_ALLOW;
>> + I915_WRITE(LCPLL_CTL, val);
>> + }
>> +
>> + val = I915_READ(D_COMP);
>> + val |= D_COMP_COMP_FORCE;
>> + val &= ~D_COMP_COMP_DISABLE;
>> + I915_WRITE(D_COMP, val);
>> +
>
> I think you need a posting read here. I am not sure we're allowed to
> read LCPLL_CTL until we know the write has landed.
Done.
>
>
>> + val = I915_READ(LCP
Re: [Intel-gfx] [PATCH 6/7] drm/i915: add functions to disable and restore LCPLL
2013/7/18 Ben Widawsky :
> On Thu, Jul 18, 2013 at 04:26:42PM -0700, Ben Widawsky wrote:
>> On Fri, Jul 12, 2013 at 02:19:41PM -0300, Paulo Zanoni wrote:
>> > From: Paulo Zanoni
>> >
>> > For now there are no callers, but these functions are going to be
>> > needed for the code that allows Package C8+. Other future features may
>> > also require this code.
>> >
>>
>> The thing that's missing from the patches is any sort of assertions
>> about things being on before the disable sequence. Is this something we
>> don't need to address?
>>
>> > Signed-off-by: Paulo Zanoni
>> > ---
>> > drivers/gpu/drm/i915/i915_reg.h | 7 +++
>> > drivers/gpu/drm/i915/intel_display.c | 95
>> >
>> > drivers/gpu/drm/i915/intel_drv.h | 3 ++
>> > 3 files changed, 105 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h
>> > index be6164f..8e5a5ec 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -4930,7 +4930,14 @@
>> > #define LCPLL_CLK_FREQ_450(0<<26)
>> > #define LCPLL_CD_CLOCK_DISABLE(1<<25)
>> > #define LCPLL_CD2X_CLOCK_DISABLE (1<<23)
>> > +#define LCPLL_POWER_DOWN_ALLOW(1<<22)
>> > #define LCPLL_CD_SOURCE_FCLK (1<<21)
>> > +#define LCPLL_CD_SOURCE_FCLK_DONE (1<<19)
>>
>> Hmm... the doc I am looking at says
>
> Oops. The doc I was looking at had some different names for things, was
> what I wanted to say.
I looked at the LCPLL_CTL register definition. Bit 22 is called
"Display Power Down Allow" and value 1 means "Allow". Bit 19 is called
"CD Source Fclk" and value 1 means "Done".
>
>>
>> > +
>> > +#define D_COMP (MCHBAR_MIRROR_BASE_SNB +
>> > 0x5F0C)
>> > +#define D_COMP_RCOMP_IN_PROGRESS (1<<9)
>> > +#define D_COMP_COMP_FORCE (1<<8)
>> > +#define D_COMP_COMP_DISABLE (1<<0)
>> >
>> > /* Pipe WM_LINETIME - watermark line time */
>> > #define PIPE_WM_LINETIME_A 0x45270
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 059c9a8..ffb08bf 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -5922,6 +5922,101 @@ static bool ironlake_get_pipe_config(struct
>> > intel_crtc *crtc,
>> > return true;
>> > }
>> >
>> > +/*
>> > + * This function implements pieces of two sequences from BSpec:
>> > + * - Sequence for display software to disable LCPLL
>> > + * - Sequence for display software to allow package C8+
>> > + * The steps implemented here are just the steps that actually touch the
>> > LCPLL
>> > + * register. Callers should take care of disabling all the display engine
>> > + * functions, doing the mode unset, fixing interrupts, etc.
>> > + */
>> > +void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
>> > + bool switch_to_fclk, bool allow_power_down)
>> > +{
>> > + uint32_t val;
>> > +
>> > + val = I915_READ(LCPLL_CTL);
>> > +
>> > + if (switch_to_fclk) {
>> > + val |= LCPLL_CD_SOURCE_FCLK;
>> > + I915_WRITE(LCPLL_CTL, val);
>> > + POSTING_READ(LCPLL_CTL);
>> > +
>> > + udelay(1);
>> > +
>> > + val = I915_READ(LCPLL_CTL);
>> > + if (!(val & LCPLL_CD_SOURCE_FCLK_DONE))
>> > + DRM_ERROR("Switching to FCLK failed\n");
>>
>> wait_for_us(..., 1)?
>>
>> > + }
>> > +
>> > + val |= LCPLL_PLL_DISABLE;
>> > + I915_WRITE(LCPLL_CTL, val);
>> > + POSTING_READ(LCPLL_CTL);
>> > +
>> > + if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1))
>> > + DRM_ERROR("LCPLL still locked\n");
>> > +
>> > + val = I915_READ(D_COMP);
>> > + val |= D_COMP_COMP_DISABLE;
>> > + I915_WRITE(D_COMP, val);
>> > + POSTING_READ(D_COMP);
>> > +
>> > + udelay(2);
>>
>> ndelay(100)?
>>
>> > +
>> > + val = I915_READ(D_COMP);
>> > + if (val & D_COMP_RCOMP_IN_PROGRESS)
>> > + DRM_ERROR("D_COMP RCOMP still in progress\n");
>>
>> wait_for(..., 1)?
>>
>> > +
>> > + if (allow_power_down) {
>> > + val = I915_READ(LCPLL_CTL);
>> > + val |= LCPLL_POWER_DOWN_ALLOW;
>> > + I915_WRITE(LCPLL_CTL, val);
>> > + POSTING_READ(LCPLL_CTL);
>> > + }
>> > +}
>> > +
>> > +/*
>> > + * Fully restores LCPLL, disallowing power down and switching back to
>> > LCPLL
>> > + * source.
>> > + */
>> > +void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
>> > +{
>> > + uint32_t val;
>> > +
>> > + val = I915_READ(LCPLL_CTL);
>> > +
>>
>> I think we could potentially exit early here if the PLL is already
>> locked, and we're on CDclk. And indeed, I've already seen this case
>> occur, but I'm not sure I will ever see that case again.
>>
>> > + if (val & LCPLL_POWER_DOWN_ALLOW) {
>> > + val &= ~LCPLL_POWER_DOWN_ALLOW;
>> > + I915_WRITE(LCPLL_CTL, val);
>> > + }
>> >
[Intel-gfx] [PATCH 1/8] build: Fix a small typo in configure.ac
Signed-off-by: Damien Lespiau --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2eba12a..59d01cf 100644 --- a/configure.ac +++ b/configure.ac @@ -61,7 +61,7 @@ XORG_MACROS_VERSION(1.16) XORG_DEFAULT_OPTIONS # warning flags for the assembler. We can't quite use CWARNFLAGS for it yet as -# it generates wy to many warnings. +# it generates wy too many warnings. ASSEMBLER_WARN_CFLAGS="" if test "x$GCC" = "xyes"; then ASSEMBLER_WARN_CFLAGS="-Wall -Wstrict-prototypes \ -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/8] tests: Allow a shell test to declare it doesn't need to be DRM master
Signed-off-by: Damien Lespiau --- tests/drm_lib.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/drm_lib.sh b/tests/drm_lib.sh index 2532352..25197d4 100755 --- a/tests/drm_lib.sh +++ b/tests/drm_lib.sh @@ -26,7 +26,8 @@ fi # read everything we can if [ `cat $i915_dfs_path/clients | wc -l` -gt "2" ] ; then - die "ERROR: other drm clients running" + [ -n "$DRM_LIB_ALLOW_NO_MASTER" ] || \ + die "ERROR: other drm clients running" fi i915_sfs_path= -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/8] build: Provide an --enable-simulation configure option
Remembering to set an environment variable is too much trouble, we can
now specify --enable-simulation at configure time and always run the
test suite tuned for simulation.
Signed-off-by: Damien Lespiau
---
configure.ac| 11 +++
lib/drmtest.c | 14 --
tests/.gitignore| 1 +
tests/{drm_lib.sh => drm_lib.sh.in} | 3 ++-
4 files changed, 26 insertions(+), 3 deletions(-)
rename tests/{drm_lib.sh => drm_lib.sh.in} (88%)
diff --git a/configure.ac b/configure.ac
index 1626ce8..0bf4764 100644
--- a/configure.ac
+++ b/configure.ac
@@ -168,6 +168,15 @@ if test "x$BUILD_TESTS" = xyes; then
fi
AM_CONDITIONAL(BUILD_TESTS, [test "x$BUILD_TESTS" = xyes])
+AC_ARG_ENABLE([simulation],
+ [AS_HELP_STRING([--enable-simulation],
+ [Tune i-g-t for simulation environments])],
+ [], [enable_simulation=no])
+AS_IF([test x"$enable_simulation" = "xyes"],
+ [AC_DEFINE([ENABLE_SIMULATION], [1],
+[i-g-t compiled with simulation environments support])])
+AC_SUBST([ENABLE_SIMULATION], [$enable_simulation])
+
AC_CONFIG_FILES([
Makefile
benchmarks/Makefile
@@ -178,6 +187,7 @@ AC_CONFIG_FILES([
tests/Makefile
tools/Makefile
tools/quick_dump/Makefile
+tests/drm_lib.sh
debugger/Makefile
debugger/system_routine/Makefile
assembler/Makefile
@@ -195,6 +205,7 @@ echo ""
echo " ??? Tests:"
echo " Build tests: ${BUILD_TESTS}"
echo " Compile prime tests: ${NOUVEAU}"
+echo " Simulation support : ${enable_simulation}"
echo ""
echo " ??? Tools:"
echo " Assembler : ${enable_assembler}"
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 26748b3..17e48b0 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -26,7 +26,10 @@
*
*/
-#define _GNU_SOURCE
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
#include
#include
#include
@@ -700,12 +703,19 @@ static bool env_set(const char *env_var, bool
default_value)
return atoi(val) != 0;
}
+#ifdef ENABLE_SIMULATION
+#define INTEL_SIMULATION_DEFAULT true
+#else
+#define INTEL_SIMULATION_DEFAULT false
+#endif
+
bool drmtest_run_in_simulation(void)
{
static int simulation = -1;
if (simulation == -1)
- simulation = env_set("INTEL_SIMULATION", false);
+ simulation = env_set("INTEL_SIMULATION",
+INTEL_SIMULATION_DEFAULT);
return simulation;
}
diff --git a/tests/.gitignore b/tests/.gitignore
index d256695..6131c78 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,4 +1,5 @@
ddi_compute_wrpll
+drm_lib.sh
drm_vma_limiter
drm_vma_limiter_cached
drm_vma_limiter_cpu
diff --git a/tests/drm_lib.sh b/tests/drm_lib.sh.in
similarity index 88%
rename from tests/drm_lib.sh
rename to tests/drm_lib.sh.in
index 25197d4..9e482ea 100755
--- a/tests/drm_lib.sh
+++ b/tests/drm_lib.sh.in
@@ -39,9 +39,10 @@ if [ -d /sys/class/drm ] ; then
fi
# sysfs may not exist as the 'error' is a new interface in 3.11
+enable_simulation=@ENABLE_SIMULATION@
function drmtest_skip_on_simulation()
{
- [ -n "$INTEL_SIMULATION" ] && exit 77
+ [ x"$enable_simulation" = "xyes" -o -n "$INTEL_SIMULATION" ] && exit 77
}
drmtest_skip_on_simulation
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/8] build: Pimp up the configure summary
This helps people compiling i-g-t figuring out what can be optional.
Signed-off-by: Damien Lespiau
---
configure.ac | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index caf929c..1626ce8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -192,9 +192,14 @@ echo ""
echo "Intel GPU tools"
echo ""
+echo " ??? Tests:"
+echo " Build tests: ${BUILD_TESTS}"
+echo " Compile prime tests: ${NOUVEAU}"
+echo ""
echo " ??? Tools:"
-echo " Assembler: ${enable_assembler}"
-echo " Debugger: ${enable_debugger}"
+echo " Assembler : ${enable_assembler}"
+echo " Debugger : ${enable_debugger}"
+echo " Python dumper : ${DUMPER}"
echo ""
# vim: set ft=config ts=8 sw=8 tw=0 noet :
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/8] tests: Source drm_lib.sh instead of having its own INTEL_SIMULATION test
One code path to maintain is always better.
Signed-off-by: Damien Lespiau
---
tests/sysfs_edid_timing | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/sysfs_edid_timing b/tests/sysfs_edid_timing
index 2a43cca..e90e374 100755
--- a/tests/sysfs_edid_timing
+++ b/tests/sysfs_edid_timing
@@ -5,7 +5,10 @@
# we sometimes take a *really* long time. So let's just check for some
reasonable timing here
#
-[ -n "$INTEL_SIMULATION" ] && exit 77
+DRM_LIB_ALLOW_NO_MASTER=1
+
+SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
+. $SOURCE_DIR/drm_lib.sh
TIME1=$(date +%s%N)
cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/8] lib: Allow users of env_set() to specify a default value
So when the environment value isn't set, one can specify the default
value to return.
Signed-off-by: Damien Lespiau
---
lib/drmtest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 016619c..26748b3 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -689,13 +689,13 @@ bool drmtest_only_list_subtests(void)
return list_subtests;
}
-static bool env_set(const char *env_var)
+static bool env_set(const char *env_var, bool default_value)
{
char *val;
val = getenv(env_var);
if (!val)
- return false;
+ return default_value;
return atoi(val) != 0;
}
@@ -705,7 +705,7 @@ bool drmtest_run_in_simulation(void)
static int simulation = -1;
if (simulation == -1)
- simulation = env_set("INTEL_SIMULATION");
+ simulation = env_set("INTEL_SIMULATION", false);
return simulation;
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/8] build: Fix automake 1.13 warnings
warning: deprecated feature: target 'sr' overrides 'sr$(EXEEXT)' Signed-off-by: Damien Lespiau --- debugger/system_routine/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debugger/system_routine/Makefile.am b/debugger/system_routine/Makefile.am index 2576e2a..03f0f0f 100644 --- a/debugger/system_routine/Makefile.am +++ b/debugger/system_routine/Makefile.am @@ -21,7 +21,7 @@ sr.c: sr.asm $(GEN4ASM) $(GEN4ASM_FLAGS) sr.asm -o $@ sr.o : sr.c $(CC) -c -o $@ sr.c -sr : sr.o +sr$(EXEEXT) : sr.o $(OBJCOPY) -O binary -K gen_eu_bytes sr.o $@ # Test.g4a is the simplest possible system routine we can run on the GPU @@ -35,7 +35,7 @@ tiny.c: tiny.asm $(GEN4ASM) $(GEN4ASM_FLAGS) tiny.asm -o $@ tiny.o : tiny.c $(CC) -c -o $@ tiny.c -tiny : tiny.o +tiny$(EXEEXT) : tiny.o $(OBJCOPY) -O binary -K gen_eu_bytes tiny.o $@ CLEANFILES = evict.h sr.cpp sr.asm sr.c tiny.cpp tiny.asm tiny.c -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/8] build: Fix unbalanced quoting in the python dumper AC_ARG_ENABLE()
Signed-off-by: Damien Lespiau --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 59d01cf..caf929c 100644 --- a/configure.ac +++ b/configure.ac @@ -104,7 +104,7 @@ AM_CONDITIONAL(HAVE_NOUVEAU, [test "x$NOUVEAU" = xyes]) AC_ARG_ENABLE(dumper, AS_HELP_STRING([--disable-dumper], [Disable the python based register dumper (default: enabled)]), -DUMPER=$enableval], [DUMPER=yes]) +[DUMPER=$enableval], [DUMPER=yes]) if test "x$DUMPER" == xyes; then AC_DEFINE(HAVE_DUMPER, 1, [Have dumper support]) # SWIG configuration -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] i-g-t --enable-simulation and various build fixes
Jesse suggested an --enable-simulation configure option because having to remember setting the env variable is too hard. After having experienced that myself and gone through several painful stop/restart cycles of the simulation, I decided it was worth making it happen. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] build: Provide an --enable-simulation configure option
On Fri, Jul 19, 2013 at 05:49:48PM +0100, Damien Lespiau wrote:
> Remembering to set an environment variable is too much trouble, we can
> now specify --enable-simulation at configure time and always run the
> test suite tuned for simulation.
>
> Signed-off-by: Damien Lespiau
tbh I'd vote for runtime detection, we could simply add a debugfs file on
the internal tree which exposes dev_priv->is_simulator. Or heck merge that
part upstream even.
Imo that's better since iirc QA is using a central build server and
distributes binaries to test machines. Ofc they need to build different
sources already, but different options is a bit more messy than just
changing the git remote ...
-Daniel
> ---
> configure.ac| 11 +++
> lib/drmtest.c | 14 --
> tests/.gitignore| 1 +
> tests/{drm_lib.sh => drm_lib.sh.in} | 3 ++-
> 4 files changed, 26 insertions(+), 3 deletions(-)
> rename tests/{drm_lib.sh => drm_lib.sh.in} (88%)
>
> diff --git a/configure.ac b/configure.ac
> index 1626ce8..0bf4764 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -168,6 +168,15 @@ if test "x$BUILD_TESTS" = xyes; then
> fi
> AM_CONDITIONAL(BUILD_TESTS, [test "x$BUILD_TESTS" = xyes])
>
> +AC_ARG_ENABLE([simulation],
> + [AS_HELP_STRING([--enable-simulation],
> + [Tune i-g-t for simulation environments])],
> + [], [enable_simulation=no])
> +AS_IF([test x"$enable_simulation" = "xyes"],
> + [AC_DEFINE([ENABLE_SIMULATION], [1],
> + [i-g-t compiled with simulation environments support])])
> +AC_SUBST([ENABLE_SIMULATION], [$enable_simulation])
> +
> AC_CONFIG_FILES([
>Makefile
>benchmarks/Makefile
> @@ -178,6 +187,7 @@ AC_CONFIG_FILES([
>tests/Makefile
>tools/Makefile
>tools/quick_dump/Makefile
> + tests/drm_lib.sh
>debugger/Makefile
>debugger/system_routine/Makefile
>assembler/Makefile
> @@ -195,6 +205,7 @@ echo ""
> echo " • Tests:"
> echo " Build tests: ${BUILD_TESTS}"
> echo " Compile prime tests: ${NOUVEAU}"
> +echo " Simulation support : ${enable_simulation}"
> echo ""
> echo " • Tools:"
> echo " Assembler : ${enable_assembler}"
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 26748b3..17e48b0 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -26,7 +26,10 @@
> *
> */
>
> -#define _GNU_SOURCE
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> #include
> #include
> #include
> @@ -700,12 +703,19 @@ static bool env_set(const char *env_var, bool
> default_value)
> return atoi(val) != 0;
> }
>
> +#ifdef ENABLE_SIMULATION
> +#define INTEL_SIMULATION_DEFAULT true
> +#else
> +#define INTEL_SIMULATION_DEFAULT false
> +#endif
> +
> bool drmtest_run_in_simulation(void)
> {
> static int simulation = -1;
>
> if (simulation == -1)
> - simulation = env_set("INTEL_SIMULATION", false);
> + simulation = env_set("INTEL_SIMULATION",
> + INTEL_SIMULATION_DEFAULT);
>
> return simulation;
> }
> diff --git a/tests/.gitignore b/tests/.gitignore
> index d256695..6131c78 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,4 +1,5 @@
> ddi_compute_wrpll
> +drm_lib.sh
> drm_vma_limiter
> drm_vma_limiter_cached
> drm_vma_limiter_cpu
> diff --git a/tests/drm_lib.sh b/tests/drm_lib.sh.in
> similarity index 88%
> rename from tests/drm_lib.sh
> rename to tests/drm_lib.sh.in
> index 25197d4..9e482ea 100755
> --- a/tests/drm_lib.sh
> +++ b/tests/drm_lib.sh.in
> @@ -39,9 +39,10 @@ if [ -d /sys/class/drm ] ; then
> fi
> # sysfs may not exist as the 'error' is a new interface in 3.11
>
> +enable_simulation=@ENABLE_SIMULATION@
> function drmtest_skip_on_simulation()
> {
> - [ -n "$INTEL_SIMULATION" ] && exit 77
> + [ x"$enable_simulation" = "xyes" -o -n "$INTEL_SIMULATION" ] && exit 77
> }
>
> drmtest_skip_on_simulation
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 9/9] drm/i915: kill ivybridge_irq_postinstall
On Fri, Jul 19, 2013 at 04:44:18PM +0300, Mika Kuoppala wrote:
> Paulo Zanoni writes:
>
> > From: Paulo Zanoni
> >
> > It was very similar to ironlake_irq_postinstall, so IMHO merging both
> > functions results in a code that is easier to maintain.
> >
> > With this change, all the irq handler vfuncs between ironlake and
> > ivybridge are now unified.
> >
> > v2: Add "(" and ")" to make at least one vim user much happier (Chris)
> >
> > Signed-off-by: Paulo Zanoni
>
> Reviewed-by: Mika Kuoppala
Slurped in the entire series, thanks for patches&review.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [v3] drm/i915: Make i915 events part of uapi
On Fri, Jul 19, 2013 at 09:16:42AM -0700, Ben Widawsky wrote: > Make the uevent strings part of the user API for people who wish to > write their own listeners. > > v2: Make a space in the string concatenation. (Chad) > Use the "UEVENT" suffix intead of "EVENT" (Chad) > Make kernel-doc parseable Docbook comments (Daniel) > > v3: Undid reset change introduced in last submission (Daniel) > Fixed up comments to address removal changes. > > Thanks to Daniel Vetter for a majority of the parity error comments. > > CC: Chad Versace > Signed-off-by: Ben Widawsky Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] lib/drmtest: add drmtest_disable/enable_prefault() function
On Fri, Jul 19, 2013 at 06:14:27PM +0200, Daniel Vetter wrote: > On Fri, Jul 19, 2013 at 06:42:51PM +0800, Xiong Zhang wrote: > > V2: add exit handler to enable prefault (Daniel) > > > > Signed-off-by: Xiong Zhang > > Thanks a lot for doing these patches, I've merged them both. Just to > check: Are the subtests added now instead of your gem_prefault testcase > good enough to still hit all slowpaths? I have to admit that I didn't even > really compile-test my little idea that I've pasted as a diff into my > reply ;-) gcc found a const mismatch and warned about it, I've fixed it up. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: extract ilk_display_irq_handler
On Fri, Jul 19, 2013 at 11:24:21AM -0300, Paulo Zanoni wrote: > 2013/7/19 Mika Kuoppala : > > Paulo Zanoni writes: > > > >> From: Paulo Zanoni > >> > >> It's the code that deals with de_iir. > >> > >> Signed-off-by: Paulo Zanoni > > > > Minor observation on the whole irq stuff: ilk, ironlake and ivb, > > ivybridge are both used and I couldn't figure out the pattern. > > You mean the function names, right? This is something that confuses me > too, all our code is inconsistent and I'd like to know what's the > preferred way. We seem to use both ways: vlv_find_best_dpll vs > valleyview_crtc_enable, ironlake_crtc_mode_set vs ilk_update_plane, > haswell_modeset_global_resources vs ivb_modeset_global_resources. I > tend to prefer the shorter versions because I believe these acronyms > should be obvious (or easy to discover) for people reading our code, > but having some guidance on which one to use on each case would be > good. Opinions? I prefer the shorter ones, too. But iirc Jesse is liking the longer ones more, at least he tends to use them in his patches. I guess we'll just have to flag this in patches. New fodder for a good bikeshd ;-) Cheeers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] [v3] drm/i915: Make i915 events part of uapi
Make the uevent strings part of the user API for people who wish to
write their own listeners.
v2: Make a space in the string concatenation. (Chad)
Use the "UEVENT" suffix intead of "EVENT" (Chad)
Make kernel-doc parseable Docbook comments (Daniel)
v3: Undid reset change introduced in last submission (Daniel)
Fixed up comments to address removal changes.
Thanks to Daniel Vetter for a majority of the parity error comments.
CC: Chad Versace
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_irq.c | 8
include/uapi/drm/i915_drm.h | 24
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9910699..4e4fa5f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -812,7 +812,7 @@ static void ivybridge_parity_work(struct work_struct *work)
mutex_unlock(&dev_priv->dev->struct_mutex);
- parity_event[0] = "L3_PARITY_ERROR=1";
+ parity_event[0] = I915_L3_PARITY_UEVENT "=1";
parity_event[1] = kasprintf(GFP_KERNEL, "ROW=%d", row);
parity_event[2] = kasprintf(GFP_KERNEL, "BANK=%d", bank);
parity_event[3] = kasprintf(GFP_KERNEL, "SUBBANK=%d", subbank);
@@ -1435,9 +1435,9 @@ static void i915_error_work_func(struct work_struct *work)
gpu_error);
struct drm_device *dev = dev_priv->dev;
struct intel_ring_buffer *ring;
- char *error_event[] = { "ERROR=1", NULL };
- char *reset_event[] = { "RESET=1", NULL };
- char *reset_done_event[] = { "ERROR=0", NULL };
+ char *error_event[] = { I915_ERROR_UEVENT "=1", NULL };
+ char *reset_event[] = { I915_RESET_UEVENT "=1", NULL };
+ char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL };
int i, ret;
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 923ed7f..a1a7b6b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -33,6 +33,30 @@
* subject to backwards-compatibility constraints.
*/
+/**
+ * DOC: uevents generated by i915 on it's device node
+ *
+ * I915_L3_PARITY_UEVENT - Generated when the driver receives a parity mismatch
+ * event from the gpu l3 cache. Additional information supplied is ROW,
+ * BANK, SUBBANK of the affected cacheline. Userspace should keep track of
+ * these events and if a specific cache-line seems to have a persistent
+ * error remap it with the l3 remapping tool supplied in intel-gpu-tools.
+ * The value supplied with the event is always 1.
+ *
+ * I915_ERROR_UEVENT - Generated upon error detection, currently only via
+ * hangcheck. The error detection event is a good indicator of when things
+ * began to go badly. The value supplied with the event is a 1 upon error
+ * detection, and a 0 upon reset completion, signifying no more error
+ * exists. NOTE: Disabling hangcheck or reset via module parameter will
+ * cause the related events to not be seen.
+ *
+ * I915_RESET_UEVENT - Event is generated just before an attempt to reset the
+ * the GPU. The value supplied with the event is always 1. NOTE: Disable
+ * reset via module parameter will cause this event to not be seen.
+ */
+#define I915_L3_PARITY_UEVENT "L3_PARITY_ERROR"
+#define I915_ERROR_UEVENT "ERROR"
+#define I915_RESET_UEVENT "RESET"
/* Each region is a minimum of 16k, and there are at most 255 of them.
*/
--
1.8.3.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] lib/drmtest: add drmtest_disable/enable_prefault() function
On Fri, Jul 19, 2013 at 06:42:51PM +0800, Xiong Zhang wrote:
> V2: add exit handler to enable prefault (Daniel)
>
> Signed-off-by: Xiong Zhang
Thanks a lot for doing these patches, I've merged them both. Just to
check: Are the subtests added now instead of your gem_prefault testcase
good enough to still hit all slowpaths? I have to admit that I didn't even
really compile-test my little idea that I've pasted as a diff into my
reply ;-)
Cheers, Daniel
> ---
> lib/drmtest.c | 50 ++
> lib/drmtest.h | 3 +++
> 2 files changed, 53 insertions(+)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 011d8c1..980fa49 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -1593,3 +1593,53 @@ void kmstest_free_connector_config(struct
> kmstest_connector_config *config)
> drmModeFreeEncoder(config->encoder);
> drmModeFreeConnector(config->connector);
> }
> +
> +#define PREFAULT_DEBUGFS "/sys/module/i915/parameters/prefault_disable"
> +static int drmtest_prefault_control(bool enable)
> +{
> + char *name = PREFAULT_DEBUGFS;
> + int fd;
> + char buf[2] = {'Y', 'N'};
> + int index;
> + int result = 0;
> +
> + fd = open(name, O_RDWR);
> + if (fd == -1) {
> + fprintf(stderr, "Couldn't open prefault_debugfs.%s\n",
> + strerror(errno));
> + return -1;
> + }
> +
> + if (enable)
> + index = 1;
> + else
> + index = 0;
> +
> + if (write(fd, &buf[index], 1) != 1) {
> + fprintf(stderr, "write prefault_debugfs error.%s\n",
> + strerror(errno));
> + result = -1;
> + }
> +
> + close(fd);
> +
> + return result;
> +}
> +
> +static void enable_prefault_at_exit(int sig)
> +{
> + drmtest_enable_prefault();
> +}
> +
> +int drmtest_disable_prefault(void)
> +{
> + drmtest_install_exit_handler(enable_prefault_at_exit);
> +
> + return drmtest_prefault_control(false);
> +}
> +
> +int drmtest_enable_prefault(void)
> +{
> + return drmtest_prefault_control(true);
> +}
> +
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index e3a9275..80b344c 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -179,3 +179,6 @@ void drmtest_enable_exit_handler(void);
> void drmtest_disable_exit_handler(void);
>
> int drmtest_set_vt_graphics_mode(void);
> +
> +int drmtest_disable_prefault(void);
> +int drmtest_enable_prefault(void);
> --
> 1.8.3.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: extend lpt_enable_clkout_dp
2013/7/18 Ben Widawsky :
> On Fri, Jul 12, 2013 at 02:19:39PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni
>>
>> Now it implements 3 different sequences from BSpec and also has
>> support for ULT.
>>
>> Signed-off-by: Paulo Zanoni
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 2 ++
>> drivers/gpu/drm/i915/intel_display.c | 41
>> +---
>> 2 files changed, 31 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index dc3d6a7..be6164f 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4863,6 +4863,8 @@
>> #define SBI_SSCAUXDIV_FINALDIV2SEL(x) ((x)<<4)
>> #define SBI_DBUFF0 0x2a00
>> #define SBI_DBUFF0_ENABLE (1<<0)
>> +#define SBI_GEN00x1f00
>> +#define SBI_GEN0_ENABLE(1<<0)
>
> bikeshed: I wouldn't haved bothered defining SBI_GEN0_ENABLE
Our docs have changed considerably since I originally wrote this
patch. They have joined both registers and a single definition makes
sense now. I also just noticed the docs say 0 is Enable and 1 is
Disable, even though the documentation for the Sequences suggests that
1 enables stuff and 0 disables stuff :(
>
>>
>> /* LPT PIXCLK_GATE */
>> #define PIXCLK_GATE 0xC6020
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index f4c5263..5f3b636 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5258,12 +5258,20 @@ static void lpt_program_fdi_mphy(struct
>> drm_i915_private *dev_priv)
>> intel_sbi_write(dev_priv, 0x21EC, tmp, SBI_MPHY);
>> }
>>
>> -/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
>> -static void lpt_enable_clkout_dp(struct drm_device *dev)
>> +/* Implements 3 different sequences from BSpec chapter "Display iCLK
>> + * Programming" based on the parameters passed:
>> + * - Sequence to enable CLKOUT_DP
>> + * - Sequence to enable CLKOUT_DP without spread
>> + * - Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O
>> + */
>> +static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
>> + bool with_fdi)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> uint32_t tmp;
>>
>> + WARN(with_fdi && !with_spread, "FDI requires downspread\n");
>> +
>
> WARN_ON(with_fdi && IS_ULT(dev))?
Yeah, but I noticed I'm using IS_ULT checks where I should be checking
for LPT-LP.
> Should we proceed after the WARN, or break out early?
Either way we'll end up with possibly broken PCH clocks I'll try
to adjust the arguments to something sane and print the warn.
>
>> mutex_lock(&dev_priv->dpio_lock);
>>
>> tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
>> @@ -5273,17 +5281,26 @@ static void lpt_enable_clkout_dp(struct drm_device
>> *dev)
>>
>> udelay(24);
>>
>> - tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
>> - tmp &= ~SBI_SSCCTL_PATHALT;
>> - intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
>> + if (with_spread) {
>> + tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
>> + tmp &= ~SBI_SSCCTL_PATHALT;
>> + intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
>>
>> - lpt_reset_fdi_mphy(dev_priv);
>> - lpt_program_fdi_mphy(dev_priv);
>> + if (with_fdi) {
>> + lpt_reset_fdi_mphy(dev_priv);
>> + lpt_program_fdi_mphy(dev_priv);
>> + }
>> + }
>>
>> - /* ULT uses SBI_GEN0, but ULT doesn't have VGA, so we don't care. */
>> - tmp = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
>> - tmp |= SBI_DBUFF0_ENABLE;
>> - intel_sbi_write(dev_priv, SBI_DBUFF0, tmp, SBI_ICLK);
>> + if (IS_ULT(dev)) {
>> + tmp = intel_sbi_read(dev_priv, SBI_GEN0, SBI_ICLK);
>> + tmp |= SBI_GEN0_ENABLE;
>> + intel_sbi_write(dev_priv, SBI_GEN0, tmp, SBI_ICLK);
>> + } else {
>> + tmp = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
>> + tmp |= SBI_DBUFF0_ENABLE;
>> + intel_sbi_write(dev_priv, SBI_DBUFF0, tmp, SBI_ICLK);
>> + }
>>
>> mutex_unlock(&dev_priv->dpio_lock);
>> }
>> @@ -5305,7 +5322,7 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>> if (!has_vga)
>> return;
>>
>> - lpt_enable_clkout_dp(dev);
>> + lpt_enable_clkout_dp(dev, true, true);
>> }
>>
>> /*
>> --
>> 1.8.1.2
> --
> Ben Widawsky, Intel Open Source Technology Center
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: extract ilk_display_irq_handler
2013/7/19 Mika Kuoppala :
> Paulo Zanoni writes:
>
>> From: Paulo Zanoni
>>
>> It's the code that deals with de_iir.
>>
>> Signed-off-by: Paulo Zanoni
>
> Minor observation on the whole irq stuff: ilk, ironlake and ivb,
> ivybridge are both used and I couldn't figure out the pattern.
You mean the function names, right? This is something that confuses me
too, all our code is inconsistent and I'd like to know what's the
preferred way. We seem to use both ways: vlv_find_best_dpll vs
valleyview_crtc_enable, ironlake_crtc_mode_set vs ilk_update_plane,
haswell_modeset_global_resources vs ivb_modeset_global_resources. I
tend to prefer the shorter versions because I believe these acronyms
should be obvious (or easy to discover) for people reading our code,
but having some guidance on which one to use on each case would be
good. Opinions?
>
> Reviewed-by: Mika Kuoppala
Thanks for all the reviews!
>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 104
>> +---
>> 1 file changed, 56 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index f397f9a..39160a2 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1202,6 +1202,60 @@ static void cpt_irq_handler(struct drm_device *dev,
>> u32 pch_iir)
>> cpt_serr_int_handler(dev);
>> }
>>
>> +static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
>> +{
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + if (de_iir & DE_AUX_CHANNEL_A)
>> + dp_aux_irq_handler(dev);
>> +
>> + if (de_iir & DE_GSE)
>> + intel_opregion_asle_intr(dev);
>> +
>> + if (de_iir & DE_PIPEA_VBLANK)
>> + drm_handle_vblank(dev, 0);
>> +
>> + if (de_iir & DE_PIPEB_VBLANK)
>> + drm_handle_vblank(dev, 1);
>> +
>> + if (de_iir & DE_POISON)
>> + DRM_ERROR("Poison interrupt\n");
>> +
>> + if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
>> + if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
>> + DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
>> +
>> + if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
>> + if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
>> + DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
>> +
>> + if (de_iir & DE_PLANEA_FLIP_DONE) {
>> + intel_prepare_page_flip(dev, 0);
>> + intel_finish_page_flip_plane(dev, 0);
>> + }
>> +
>> + if (de_iir & DE_PLANEB_FLIP_DONE) {
>> + intel_prepare_page_flip(dev, 1);
>> + intel_finish_page_flip_plane(dev, 1);
>> + }
>> +
>> + /* check event from PCH */
>> + if (de_iir & DE_PCH_EVENT) {
>> + u32 pch_iir = I915_READ(SDEIIR);
>> +
>> + if (HAS_PCH_CPT(dev))
>> + cpt_irq_handler(dev, pch_iir);
>> + else
>> + ibx_irq_handler(dev, pch_iir);
>> +
>> + /* should clear PCH hotplug event before clear CPU irq */
>> + I915_WRITE(SDEIIR, pch_iir);
>> + }
>> +
>> + if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
>> + ironlake_rps_change_irq_handler(dev);
>> +}
>> +
>> static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
>> {
>> struct drm_device *dev = (struct drm_device *) arg;
>> @@ -1360,54 +1414,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void
>> *arg)
>> else
>> snb_gt_irq_handler(dev, dev_priv, gt_iir);
>>
>> - if (de_iir & DE_AUX_CHANNEL_A)
>> - dp_aux_irq_handler(dev);
>> -
>> - if (de_iir & DE_GSE)
>> - intel_opregion_asle_intr(dev);
>> -
>> - if (de_iir & DE_PIPEA_VBLANK)
>> - drm_handle_vblank(dev, 0);
>> -
>> - if (de_iir & DE_PIPEB_VBLANK)
>> - drm_handle_vblank(dev, 1);
>> -
>> - if (de_iir & DE_POISON)
>> - DRM_ERROR("Poison interrupt\n");
>> -
>> - if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
>> - if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
>> - DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
>> -
>> - if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
>> - if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
>> - DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
>> -
>> - if (de_iir & DE_PLANEA_FLIP_DONE) {
>> - intel_prepare_page_flip(dev, 0);
>> - intel_finish_page_flip_plane(dev, 0);
>> - }
>> -
>> - if (de_iir & DE_PLANEB_FLIP_DONE) {
>> - intel_prepare_page_flip(dev, 1);
>> - intel_finish_page_flip_plane(dev, 1);
>> - }
>> -
>> - /* check event from PCH */
>> - if (de_iir & DE_PCH_EVENT) {
>> - u32 pch_iir = I915_READ(SDEIIR);
>> -
>> - if (HAS_PCH_CPT(dev))
>> - cpt_irq_handler(dev, pch_iir);
>> - else
Re: [Intel-gfx] [PATCH 9/9] drm/i915: kill ivybridge_irq_postinstall
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> It was very similar to ironlake_irq_postinstall, so IMHO merging both
> functions results in a code that is easier to maintain.
>
> With this change, all the irq handler vfuncs between ironlake and
> ivybridge are now unified.
>
> v2: Add "(" and ")" to make at least one vim user much happier (Chris)
>
> Signed-off-by: Paulo Zanoni
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c | 69
> -
> 1 file changed, 20 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d084057..8b48a32 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2161,21 +2161,33 @@ static void gen5_gt_irq_postinstall(struct drm_device
> *dev)
> static int ironlake_irq_postinstall(struct drm_device *dev)
> {
> unsigned long irqflags;
> -
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - /* enable kind of interrupts always enabled */
> - u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
> -DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
> -DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
> -DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
> + u32 display_mask, extra_mask;
> +
> + if (INTEL_INFO(dev)->gen >= 7) {
> + display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE_IVB |
> + DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
> + DE_PLANEB_FLIP_DONE_IVB |
> + DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB |
> + DE_ERR_INT_IVB);
> + extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
> + DE_PIPEA_VBLANK_IVB);
> +
> + I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
> + } else {
> + display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
> + DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
> + DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
> + DE_PIPEA_FIFO_UNDERRUN | DE_POISON);
> + extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT;
> + }
>
> dev_priv->irq_mask = ~display_mask;
>
> /* should always can generate irq */
> I915_WRITE(DEIIR, I915_READ(DEIIR));
> I915_WRITE(DEIMR, dev_priv->irq_mask);
> - I915_WRITE(DEIER, display_mask |
> - DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT);
> + I915_WRITE(DEIER, display_mask | extra_mask);
> POSTING_READ(DEIER);
>
> gen5_gt_irq_postinstall(dev);
> @@ -2196,38 +2208,6 @@ static int ironlake_irq_postinstall(struct drm_device
> *dev)
> return 0;
> }
>
> -static int ivybridge_irq_postinstall(struct drm_device *dev)
> -{
> - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - /* enable kind of interrupts always enabled */
> - u32 display_mask =
> - DE_MASTER_IRQ_CONTROL | DE_GSE_IVB | DE_PCH_EVENT_IVB |
> - DE_PLANEC_FLIP_DONE_IVB |
> - DE_PLANEB_FLIP_DONE_IVB |
> - DE_PLANEA_FLIP_DONE_IVB |
> - DE_AUX_CHANNEL_A_IVB |
> - DE_ERR_INT_IVB;
> -
> - dev_priv->irq_mask = ~display_mask;
> -
> - /* should always can generate irq */
> - I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
> - I915_WRITE(DEIIR, I915_READ(DEIIR));
> - I915_WRITE(DEIMR, dev_priv->irq_mask);
> - I915_WRITE(DEIER,
> -display_mask |
> -DE_PIPEC_VBLANK_IVB |
> -DE_PIPEB_VBLANK_IVB |
> -DE_PIPEA_VBLANK_IVB);
> - POSTING_READ(DEIER);
> -
> - gen5_gt_irq_postinstall(dev);
> -
> - ibx_irq_postinstall(dev);
> -
> - return 0;
> -}
> -
> static int valleyview_irq_postinstall(struct drm_device *dev)
> {
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> @@ -3036,15 +3016,6 @@ void intel_irq_init(struct drm_device *dev)
> dev->driver->enable_vblank = valleyview_enable_vblank;
> dev->driver->disable_vblank = valleyview_disable_vblank;
> dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> - } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> - /* Share uninstall handlers with ILK/SNB */
> - dev->driver->irq_handler = ironlake_irq_handler;
> - dev->driver->irq_preinstall = ironlake_irq_preinstall;
> - dev->driver->irq_postinstall = ivybridge_irq_postinstall;
> - dev->driver->irq_uninstall = ironlake_irq_uninstall;
> - dev->driver->enable_vblank = ironlake_enable_vblank;
> - dev->driver->disable_vblank = ironlake_disable_vblank;
> - dev_priv->di
Re: [Intel-gfx] [PATCH 8/9] drm/i915: kill Ivybridge vblank irq vfuncs
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> The IVB funtions are exactly the same as the ILK ones, with the
> exception of the bit register. So add IVB/HSW support to
> ironlake_enable_vblank and ironlake_disable_vblank, then kill the
> ivybridge functions.
>
> Signed-off-by: Paulo Zanoni
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c | 41
> -
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> 2 files changed, 11 insertions(+), 33 deletions(-)
>
> This replaces patches 8 and 9 from the series. I'm not really sure what Chris
> had in mind when he mentioned I could be a little more creative, so this is my
> attempt.
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f54a02b..d084057 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1663,29 +1663,14 @@ static int ironlake_enable_vblank(struct drm_device
> *dev, int pipe)
> {
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> unsigned long irqflags;
> + uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> + DE_PIPE_VBLANK_ILK(pipe);
>
> if (!i915_pipe_enabled(dev, pipe))
> return -EINVAL;
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - ironlake_enable_display_irq(dev_priv, (pipe == 0) ?
> - DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
> - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> -
> - return 0;
> -}
> -
> -static int ivybridge_enable_vblank(struct drm_device *dev, int pipe)
> -{
> - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - unsigned long irqflags;
> -
> - if (!i915_pipe_enabled(dev, pipe))
> - return -EINVAL;
> -
> - spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - ironlake_enable_display_irq(dev_priv,
> - DE_PIPEA_VBLANK_IVB << (5 * pipe));
> + ironlake_enable_display_irq(dev_priv, bit);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> return 0;
> @@ -1736,21 +1721,11 @@ static void ironlake_disable_vblank(struct drm_device
> *dev, int pipe)
> {
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> unsigned long irqflags;
> + uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> + DE_PIPE_VBLANK_ILK(pipe);
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - ironlake_disable_display_irq(dev_priv, (pipe == 0) ?
> - DE_PIPEA_VBLANK : DE_PIPEB_VBLANK);
> - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> -}
> -
> -static void ivybridge_disable_vblank(struct drm_device *dev, int pipe)
> -{
> - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - unsigned long irqflags;
> -
> - spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - ironlake_disable_display_irq(dev_priv,
> - DE_PIPEA_VBLANK_IVB << (pipe * 5));
> + ironlake_disable_display_irq(dev_priv, bit);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
>
> @@ -3067,8 +3042,8 @@ void intel_irq_init(struct drm_device *dev)
> dev->driver->irq_preinstall = ironlake_irq_preinstall;
> dev->driver->irq_postinstall = ivybridge_irq_postinstall;
> dev->driver->irq_uninstall = ironlake_irq_uninstall;
> - dev->driver->enable_vblank = ivybridge_enable_vblank;
> - dev->driver->disable_vblank = ivybridge_disable_vblank;
> + dev->driver->enable_vblank = ironlake_enable_vblank;
> + dev->driver->disable_vblank = ironlake_disable_vblank;
> dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> } else if (HAS_PCH_SPLIT(dev)) {
> dev->driver->irq_handler = ironlake_irq_handler;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9556dff..dd2306e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3729,6 +3729,9 @@
> #define DE_PLANEA_FLIP_DONE_IVB (1<<3)
> #define DE_PIPEA_VBLANK_IVB (1<<0)
>
> +#define DE_PIPE_VBLANK_ILK(pipe) (1 << ((pipe * 8) + 7))
> +#define DE_PIPE_VBLANK_IVB(pipe) (1 << (pipe * 5))
> +
> #define VLV_MASTER_IER 0x4400c /* Gunit master IER */
> #define MASTER_INTERRUPT_ENABLE(1<<31)
>
> --
> 1.8.1.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listi
Re: [Intel-gfx] [PATCH 7/9] drm/i915: add ILK/SNB support to ivybridge_irq_handler
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> And then rename it to ironlake_irq_handler. Also move
> ilk_gt_irq_handler up to avoid forward declarations.
>
> In the previous patches I did small modifications to both
> ironlake_irq_handler an ivybridge_irq_handler so they became very
> similar functions. Now it should be very easy to verify that all we
> need to add ILK/SNB support is to call ilk_gt_irq_handler, call
> ilk_display_irq_handler and avoid reading pm_iir on gen 5.
>
> v2: - Rebase due to changes on the previous patches
> - Move pm_iir to a tighter scope (Chris)
> - Change some Gen checks for readability
>
> Signed-off-by: Paulo Zanoni
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c | 115
> +++-
> 1 file changed, 32 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ba38aa8..f54a02b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -849,6 +849,17 @@ static void ivybridge_parity_error_irq_handler(struct
> drm_device *dev)
> queue_work(dev_priv->wq, &dev_priv->l3_parity.error_work);
> }
>
> +static void ilk_gt_irq_handler(struct drm_device *dev,
> +struct drm_i915_private *dev_priv,
> +u32 gt_iir)
> +{
> + if (gt_iir &
> + (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
> + notify_ring(dev, &dev_priv->ring[RCS]);
> + if (gt_iir & ILK_BSD_USER_INTERRUPT)
> + notify_ring(dev, &dev_priv->ring[VCS]);
> +}
> +
> static void snb_gt_irq_handler(struct drm_device *dev,
> struct drm_i915_private *dev_priv,
> u32 gt_iir)
> @@ -1290,11 +1301,11 @@ static void ivb_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
> }
> }
>
> -static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
> +static irqreturn_t ironlake_irq_handler(int irq, void *arg)
> {
> struct drm_device *dev = (struct drm_device *) arg;
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier = 0;
> + u32 de_iir, gt_iir, de_ier, sde_ier = 0;
> irqreturn_t ret = IRQ_NONE;
>
> atomic_inc(&dev_priv->irq_received);
> @@ -1334,27 +1345,34 @@ static irqreturn_t ivybridge_irq_handler(int irq,
> void *arg)
>
> gt_iir = I915_READ(GTIIR);
> if (gt_iir) {
> - snb_gt_irq_handler(dev, dev_priv, gt_iir);
> + if (IS_GEN5(dev))
> + ilk_gt_irq_handler(dev, dev_priv, gt_iir);
> + else
> + snb_gt_irq_handler(dev, dev_priv, gt_iir);
> I915_WRITE(GTIIR, gt_iir);
> ret = IRQ_HANDLED;
> }
>
> de_iir = I915_READ(DEIIR);
> if (de_iir) {
> - ivb_display_irq_handler(dev, de_iir);
> -
> + if (INTEL_INFO(dev)->gen >= 7)
> + ivb_display_irq_handler(dev, de_iir);
> + else
> + ilk_display_irq_handler(dev, de_iir);
> I915_WRITE(DEIIR, de_iir);
> ret = IRQ_HANDLED;
> }
>
> - pm_iir = I915_READ(GEN6_PMIIR);
> - if (pm_iir) {
> - if (IS_HASWELL(dev))
> - hsw_pm_irq_handler(dev_priv, pm_iir);
> - else if (pm_iir & GEN6_PM_RPS_EVENTS)
> - gen6_rps_irq_handler(dev_priv, pm_iir);
> - I915_WRITE(GEN6_PMIIR, pm_iir);
> - ret = IRQ_HANDLED;
> + if (INTEL_INFO(dev)->gen >= 6) {
> + u32 pm_iir = I915_READ(GEN6_PMIIR);
> + if (pm_iir) {
> + if (IS_HASWELL(dev))
> + hsw_pm_irq_handler(dev_priv, pm_iir);
> + else if (pm_iir & GEN6_PM_RPS_EVENTS)
> + gen6_rps_irq_handler(dev_priv, pm_iir);
> + I915_WRITE(GEN6_PMIIR, pm_iir);
> + ret = IRQ_HANDLED;
> + }
> }
>
> if (IS_HASWELL(dev)) {
> @@ -1374,75 +1392,6 @@ static irqreturn_t ivybridge_irq_handler(int irq, void
> *arg)
> return ret;
> }
>
> -static void ilk_gt_irq_handler(struct drm_device *dev,
> -struct drm_i915_private *dev_priv,
> -u32 gt_iir)
> -{
> - if (gt_iir &
> - (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
> - notify_ring(dev, &dev_priv->ring[RCS]);
> - if (gt_iir & ILK_BSD_USER_INTERRUPT)
> - notify_ring(dev, &dev_priv->ring[VCS]);
> -}
> -
> -static irqreturn_t ironlake_irq_handler(int irq, void *arg)
> -{
> - struct drm_device *dev = (struct drm_device *) arg;
> - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - int ret = IRQ_NONE;
> - u32 de_iir, gt_iir,
Re: [Intel-gfx] [PATCH 06/10] drm/i915: POSTING_READ(DEIER) on ivybridge_irq_handler
Paulo Zanoni writes: > From: Paulo Zanoni > > We have this POSTING_READ inside ironlake_irq_handler. I suppose we > also want it on IVB because we want to stop the IRQ handler as soon as > possible at this point. > > Signed-off-by: Paulo Zanoni Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_irq.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 7bc36ae..bdaab93 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1310,6 +1310,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void > *arg) > /* disable master interrupt before clearing iir */ > de_ier = I915_READ(DEIER); > I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL); > + POSTING_READ(DEIER); > > /* Disable south interrupts. We'll only write to SDEIIR once, so further >* interrupts will will be stored on its back queue, and then we'll be > -- > 1.8.1.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: reorganize ironlake_irq_handler
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> The ironlake_irq_handler and ivybridge_irq_handler functions do
> basically the same thing, but they have different implementation
> styles. With this patch we reorganize ironlake_irq_handler in a way
> that makes it look very similar to ivybridge_irq_handler.
>
> One of the advantages of this new function style is that we don't
> write 0 to the IIR registers anymore.
>
> v2: - Rebase due to changes on previous patches
> - Move pm_iir to a tighter scope (Chris)
>
> Signed-off-by: Paulo Zanoni
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c | 46
> -
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index c674dc3..88eb380 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void
> *arg)
> struct drm_device *dev = (struct drm_device *) arg;
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> int ret = IRQ_NONE;
> - u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier;
> + u32 de_iir, gt_iir, de_ier, sde_ier;
>
> atomic_inc(&dev_priv->irq_received);
>
> @@ -1407,33 +1407,33 @@ static irqreturn_t ironlake_irq_handler(int irq, void
> *arg)
> I915_WRITE(SDEIER, 0);
> POSTING_READ(SDEIER);
>
> - de_iir = I915_READ(DEIIR);
> gt_iir = I915_READ(GTIIR);
> - if (IS_GEN6(dev))
> - pm_iir = I915_READ(GEN6_PMIIR);
> -
> - if (de_iir == 0 && gt_iir == 0 && pm_iir == 0)
> - goto done;
> -
> - ret = IRQ_HANDLED;
> -
> - if (IS_GEN5(dev))
> - ilk_gt_irq_handler(dev, dev_priv, gt_iir);
> - else
> - snb_gt_irq_handler(dev, dev_priv, gt_iir);
> + if (gt_iir) {
> + if (IS_GEN5(dev))
> + ilk_gt_irq_handler(dev, dev_priv, gt_iir);
> + else
> + snb_gt_irq_handler(dev, dev_priv, gt_iir);
> + I915_WRITE(GTIIR, gt_iir);
> + ret = IRQ_HANDLED;
> + }
>
> - if (de_iir)
> + de_iir = I915_READ(DEIIR);
> + if (de_iir) {
> ilk_display_irq_handler(dev, de_iir);
> + I915_WRITE(DEIIR, de_iir);
> + ret = IRQ_HANDLED;
> + }
>
> - if (pm_iir & GEN6_PM_RPS_EVENTS)
> - gen6_rps_irq_handler(dev_priv, pm_iir);
> -
> - I915_WRITE(GTIIR, gt_iir);
> - I915_WRITE(DEIIR, de_iir);
> - if (pm_iir)
> - I915_WRITE(GEN6_PMIIR, pm_iir);
> + if (IS_GEN6(dev)) {
> + u32 pm_iir = I915_READ(GEN6_PMIIR);
> + if (pm_iir) {
> + if (pm_iir & GEN6_PM_RPS_EVENTS)
> + gen6_rps_irq_handler(dev_priv, pm_iir);
> + I915_WRITE(GEN6_PMIIR, pm_iir);
> + ret = IRQ_HANDLED;
> + }
> + }
>
> -done:
> I915_WRITE(DEIER, de_ier);
> POSTING_READ(DEIER);
> I915_WRITE(SDEIER, sde_ier);
> --
> 1.8.1.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: don't read or write GEN6_PMIIR on Gen 5
Paulo Zanoni writes: > From: Paulo Zanoni > > The register doesn't exist on Gen 5. > > v2: Simplify checks since pm_iir is always 0 on Gen 5 (Chris) > > Signed-off-by: Paulo Zanoni Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_irq.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 9167219..c674dc3 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1389,7 +1389,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void > *arg) > struct drm_device *dev = (struct drm_device *) arg; > drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > int ret = IRQ_NONE; > - u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier; > + u32 de_iir, gt_iir, de_ier, pm_iir = 0, sde_ier; > > atomic_inc(&dev_priv->irq_received); > > @@ -1409,9 +1409,10 @@ static irqreturn_t ironlake_irq_handler(int irq, void > *arg) > > de_iir = I915_READ(DEIIR); > gt_iir = I915_READ(GTIIR); > - pm_iir = I915_READ(GEN6_PMIIR); > + if (IS_GEN6(dev)) > + pm_iir = I915_READ(GEN6_PMIIR); > > - if (de_iir == 0 && gt_iir == 0 && (!IS_GEN6(dev) || pm_iir == 0)) > + if (de_iir == 0 && gt_iir == 0 && pm_iir == 0) > goto done; > > ret = IRQ_HANDLED; > @@ -1424,12 +1425,13 @@ static irqreturn_t ironlake_irq_handler(int irq, void > *arg) > if (de_iir) > ilk_display_irq_handler(dev, de_iir); > > - if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS) > + if (pm_iir & GEN6_PM_RPS_EVENTS) > gen6_rps_irq_handler(dev_priv, pm_iir); > > I915_WRITE(GTIIR, gt_iir); > I915_WRITE(DEIIR, de_iir); > - I915_WRITE(GEN6_PMIIR, pm_iir); > + if (pm_iir) > + I915_WRITE(GEN6_PMIIR, pm_iir); > > done: > I915_WRITE(DEIER, de_ier); > -- > 1.8.1.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/10] drm/i915: extract ivb_display_irq_handler
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> Just like we did with ilk_display_irq_handler.
>
> Signed-off-by: Paulo Zanoni
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c | 63
> +++--
> 1 file changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 39160a2..9167219 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1256,13 +1256,46 @@ static void ilk_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
> ironlake_rps_change_irq_handler(dev);
> }
>
> +static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int i;
> +
> + if (de_iir & DE_ERR_INT_IVB)
> + ivb_err_int_handler(dev);
> +
> + if (de_iir & DE_AUX_CHANNEL_A_IVB)
> + dp_aux_irq_handler(dev);
> +
> + if (de_iir & DE_GSE_IVB)
> + intel_opregion_asle_intr(dev);
> +
> + for (i = 0; i < 3; i++) {
> + if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
> + drm_handle_vblank(dev, i);
> + if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
> + intel_prepare_page_flip(dev, i);
> + intel_finish_page_flip_plane(dev, i);
> + }
> + }
> +
> + /* check event from PCH */
> + if (!HAS_PCH_NOP(dev) && (de_iir & DE_PCH_EVENT_IVB)) {
> + u32 pch_iir = I915_READ(SDEIIR);
> +
> + cpt_irq_handler(dev, pch_iir);
> +
> + /* clear PCH hotplug event before clear CPU irq */
> + I915_WRITE(SDEIIR, pch_iir);
> + }
> +}
> +
> static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
> {
> struct drm_device *dev = (struct drm_device *) arg;
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier = 0;
> irqreturn_t ret = IRQ_NONE;
> - int i;
>
> atomic_inc(&dev_priv->irq_received);
>
> @@ -1307,33 +1340,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void
> *arg)
>
> de_iir = I915_READ(DEIIR);
> if (de_iir) {
> - if (de_iir & DE_ERR_INT_IVB)
> - ivb_err_int_handler(dev);
> -
> - if (de_iir & DE_AUX_CHANNEL_A_IVB)
> - dp_aux_irq_handler(dev);
> -
> - if (de_iir & DE_GSE_IVB)
> - intel_opregion_asle_intr(dev);
> -
> - for (i = 0; i < 3; i++) {
> - if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
> - drm_handle_vblank(dev, i);
> - if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
> - intel_prepare_page_flip(dev, i);
> - intel_finish_page_flip_plane(dev, i);
> - }
> - }
> -
> - /* check event from PCH */
> - if (!HAS_PCH_NOP(dev) && (de_iir & DE_PCH_EVENT_IVB)) {
> - u32 pch_iir = I915_READ(SDEIIR);
> -
> - cpt_irq_handler(dev, pch_iir);
> -
> - /* clear PCH hotplug event before clear CPU irq */
> - I915_WRITE(SDEIIR, pch_iir);
> - }
> + ivb_display_irq_handler(dev, de_iir);
>
> I915_WRITE(DEIIR, de_iir);
> ret = IRQ_HANDLED;
> --
> 1.8.1.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: extract ilk_display_irq_handler
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> It's the code that deals with de_iir.
>
> Signed-off-by: Paulo Zanoni
Minor observation on the whole irq stuff: ilk, ironlake and ivb,
ivybridge are both used and I couldn't figure out the pattern.
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_irq.c | 104
> +---
> 1 file changed, 56 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f397f9a..39160a2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1202,6 +1202,60 @@ static void cpt_irq_handler(struct drm_device *dev,
> u32 pch_iir)
> cpt_serr_int_handler(dev);
> }
>
> +static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (de_iir & DE_AUX_CHANNEL_A)
> + dp_aux_irq_handler(dev);
> +
> + if (de_iir & DE_GSE)
> + intel_opregion_asle_intr(dev);
> +
> + if (de_iir & DE_PIPEA_VBLANK)
> + drm_handle_vblank(dev, 0);
> +
> + if (de_iir & DE_PIPEB_VBLANK)
> + drm_handle_vblank(dev, 1);
> +
> + if (de_iir & DE_POISON)
> + DRM_ERROR("Poison interrupt\n");
> +
> + if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
> + if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
> + DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
> +
> + if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
> + if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
> + DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
> +
> + if (de_iir & DE_PLANEA_FLIP_DONE) {
> + intel_prepare_page_flip(dev, 0);
> + intel_finish_page_flip_plane(dev, 0);
> + }
> +
> + if (de_iir & DE_PLANEB_FLIP_DONE) {
> + intel_prepare_page_flip(dev, 1);
> + intel_finish_page_flip_plane(dev, 1);
> + }
> +
> + /* check event from PCH */
> + if (de_iir & DE_PCH_EVENT) {
> + u32 pch_iir = I915_READ(SDEIIR);
> +
> + if (HAS_PCH_CPT(dev))
> + cpt_irq_handler(dev, pch_iir);
> + else
> + ibx_irq_handler(dev, pch_iir);
> +
> + /* should clear PCH hotplug event before clear CPU irq */
> + I915_WRITE(SDEIIR, pch_iir);
> + }
> +
> + if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
> + ironlake_rps_change_irq_handler(dev);
> +}
> +
> static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
> {
> struct drm_device *dev = (struct drm_device *) arg;
> @@ -1360,54 +1414,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void
> *arg)
> else
> snb_gt_irq_handler(dev, dev_priv, gt_iir);
>
> - if (de_iir & DE_AUX_CHANNEL_A)
> - dp_aux_irq_handler(dev);
> -
> - if (de_iir & DE_GSE)
> - intel_opregion_asle_intr(dev);
> -
> - if (de_iir & DE_PIPEA_VBLANK)
> - drm_handle_vblank(dev, 0);
> -
> - if (de_iir & DE_PIPEB_VBLANK)
> - drm_handle_vblank(dev, 1);
> -
> - if (de_iir & DE_POISON)
> - DRM_ERROR("Poison interrupt\n");
> -
> - if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
> - if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
> - DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
> -
> - if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
> - if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
> - DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
> -
> - if (de_iir & DE_PLANEA_FLIP_DONE) {
> - intel_prepare_page_flip(dev, 0);
> - intel_finish_page_flip_plane(dev, 0);
> - }
> -
> - if (de_iir & DE_PLANEB_FLIP_DONE) {
> - intel_prepare_page_flip(dev, 1);
> - intel_finish_page_flip_plane(dev, 1);
> - }
> -
> - /* check event from PCH */
> - if (de_iir & DE_PCH_EVENT) {
> - u32 pch_iir = I915_READ(SDEIIR);
> -
> - if (HAS_PCH_CPT(dev))
> - cpt_irq_handler(dev, pch_iir);
> - else
> - ibx_irq_handler(dev, pch_iir);
> -
> - /* should clear PCH hotplug event before clear CPU irq */
> - I915_WRITE(SDEIIR, pch_iir);
> - }
> -
> - if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
> - ironlake_rps_change_irq_handler(dev);
> + if (de_iir)
> + ilk_display_irq_handler(dev, de_iir);
>
> if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
> gen6_rps_irq_handler(dev_priv, pm_iir);
> --
> 1.8.1.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
In
Re: [Intel-gfx] [PATCH 01/10] drm/i915: kill ivybridge_irq_preinstall
Paulo Zanoni writes:
> From: Paulo Zanoni
>
> After Daniel's latest changes it's now equal to
> ironlake_irq_preinstall.
>
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_irq.c | 21 +
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ca9df54..f397f9a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2090,25 +2090,6 @@ static void ironlake_irq_preinstall(struct drm_device
> *dev)
> ibx_irq_preinstall(dev);
> }
>
> -static void ivybridge_irq_preinstall(struct drm_device *dev)
> -{
> - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> -
> - atomic_set(&dev_priv->irq_received, 0);
> -
> - I915_WRITE(HWSTAM, 0xeffe);
> -
> - /* XXX hotplug from PCH */
This comment will be lost. I couldn't figure out if it is
is relevant anymore or not.
Reviewed-by: Mika Kuoppala
> - I915_WRITE(DEIMR, 0x);
> - I915_WRITE(DEIER, 0x0);
> - POSTING_READ(DEIER);
> -
> - gen5_gt_irq_preinstall(dev);
> -
> - ibx_irq_preinstall(dev);
> -}
> -
> static void valleyview_irq_preinstall(struct drm_device *dev)
> {
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> @@ -3116,7 +3097,7 @@ void intel_irq_init(struct drm_device *dev)
> } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> /* Share uninstall handlers with ILK/SNB */
> dev->driver->irq_handler = ivybridge_irq_handler;
> - dev->driver->irq_preinstall = ivybridge_irq_preinstall;
> + dev->driver->irq_preinstall = ironlake_irq_preinstall;
> dev->driver->irq_postinstall = ivybridge_irq_postinstall;
> dev->driver->irq_uninstall = ironlake_irq_uninstall;
> dev->driver->enable_vblank = ivybridge_enable_vblank;
> --
> 1.8.1.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Provide a simulation friendly test environment v4
On Thu, Jul 18, 2013 at 05:50:55PM +0200, Daniel Vetter wrote: > On Thu, Jul 18, 2013 at 04:19:05PM +0100, Damien Lespiau wrote: > > It was high time to follow up on: > > http://lists.freedesktop.org/archives/intel-gfx/2013-April/027361.html > > > > Changes from v2: > > • Take into account Daniel's comments and add the tests he listed > > • Actually runs on simulation now > > lgtm overall, please push. One thing we might want to do is add a generic > drmtest_dummy_blt_load function which auto-tunes for simulation runs. Ever > time I need to reliably hit a seqno wait in a test somewhere I just > copy-paste that little bit of lore so we'll have a constant game of > whack-a-mole for those. With a helper it would Just Work. Right, so pushed this to progress forward a bit, generally agreed to be torwards the right direction. i-g-t needs more work, but that'll probably be always the case. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] lib/drmtest: add drmtest_disable/enable_prefault() function
V2: add exit handler to enable prefault (Daniel)
Signed-off-by: Xiong Zhang
---
lib/drmtest.c | 50 ++
lib/drmtest.h | 3 +++
2 files changed, 53 insertions(+)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 011d8c1..980fa49 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -1593,3 +1593,53 @@ void kmstest_free_connector_config(struct
kmstest_connector_config *config)
drmModeFreeEncoder(config->encoder);
drmModeFreeConnector(config->connector);
}
+
+#define PREFAULT_DEBUGFS "/sys/module/i915/parameters/prefault_disable"
+static int drmtest_prefault_control(bool enable)
+{
+ char *name = PREFAULT_DEBUGFS;
+ int fd;
+ char buf[2] = {'Y', 'N'};
+ int index;
+ int result = 0;
+
+ fd = open(name, O_RDWR);
+ if (fd == -1) {
+ fprintf(stderr, "Couldn't open prefault_debugfs.%s\n",
+ strerror(errno));
+ return -1;
+ }
+
+ if (enable)
+ index = 1;
+ else
+ index = 0;
+
+ if (write(fd, &buf[index], 1) != 1) {
+ fprintf(stderr, "write prefault_debugfs error.%s\n",
+ strerror(errno));
+ result = -1;
+ }
+
+ close(fd);
+
+ return result;
+}
+
+static void enable_prefault_at_exit(int sig)
+{
+ drmtest_enable_prefault();
+}
+
+int drmtest_disable_prefault(void)
+{
+ drmtest_install_exit_handler(enable_prefault_at_exit);
+
+ return drmtest_prefault_control(false);
+}
+
+int drmtest_enable_prefault(void)
+{
+ return drmtest_prefault_control(true);
+}
+
diff --git a/lib/drmtest.h b/lib/drmtest.h
index e3a9275..80b344c 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -179,3 +179,6 @@ void drmtest_enable_exit_handler(void);
void drmtest_disable_exit_handler(void);
int drmtest_set_vt_graphics_mode(void);
+
+int drmtest_disable_prefault(void);
+int drmtest_enable_prefault(void);
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] tests: add reloc, pread, pwrite slow path subtest
the reloc, pread, pwrite slow path will be prevented by prefault,
these subtests will disable prefault first, then do reloc, pread,
pwrite, finally enable prefault.
Signed-off-by: Xiong Zhang
---
tests/gem_exec_faulting_reloc.c | 10 +-
tests/gem_mmap_gtt.c| 15 +++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/tests/gem_exec_faulting_reloc.c b/tests/gem_exec_faulting_reloc.c
index 863a1b0..c7f7b6b 100644
--- a/tests/gem_exec_faulting_reloc.c
+++ b/tests/gem_exec_faulting_reloc.c
@@ -220,7 +220,15 @@ static void run(int object_size)
int main(int argc, char **argv)
{
- run(OBJECT_SIZE);
+ drmtest_subtest_init(argc, argv);
+
+ if (drmtest_run_subtest("normal"))
+ run(OBJECT_SIZE);
+ if (drmtest_run_subtest("no-prefault")) {
+ drmtest_disable_prefault();
+ run(OBJECT_SIZE);
+ drmtest_enable_prefault();
+ }
return 0;
}
diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index d759340..0e1b195 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -144,6 +144,15 @@ test_read(int fd)
munmap(dst, OBJECT_SIZE);
}
+static void
+run_without_prefault(int fd,
+ void (*func)(int fd))
+{
+ drmtest_disable_prefault();
+ func(fd);
+ drmtest_enable_prefault();
+}
+
int main(int argc, char **argv)
{
int fd;
@@ -160,6 +169,12 @@ int main(int argc, char **argv)
test_write(fd);
if (drmtest_run_subtest("write-gtt"))
test_write_gtt(fd);
+ if (drmtest_run_subtest("read-no-prefault"))
+ run_without_prefault(fd, test_read);
+ if (drmtest_run_subtest("write-no-prefault"))
+ run_without_prefault(fd, test_write);
+ if (drmtest_run_subtest("write-gtt-no-prefault"))
+ run_without_prefault(fd, test_write_gtt);
close(fd);
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: run shmem_pread_fast() after page fault
On Fri, Jul 19, 2013 at 08:52:39AM +, Zhang, Xiong Y wrote: > It just influence one page. > During my test, I found one read slow path info when enable prefault. So I > add this patch. > Why don't move fault_in_multipages_writeable() from i915_gem_shmem_pread() to > the beginning of i915_gem_pread_ioctl() ? > So the pread_ioctl() is like pwrite_ioctl(). Is the cost of > fault_in_multipages_writeable() lager than fault_in_multipages_readable() ? The essence of the problem with fault_in_multipages_writeable() is that it writes a 0 without us guarranteeing to replace it with correct data. Real world usage (at least in the ddx) of pread is limited to UXA readbacks (x11perf -shmget would be the best test case), and more or less gen2 readbacks in SNA. (For SNA we first try to use CPU bo and various maps before resorting to pread.) You will want to extend the microbenchmarks such as i-g-t/gem_gtt_speed to cover cases which can be prefaulted and where prefaulting helps. If you can measure a difference between the paths in the current code, you are ready to see if you can measure an improvement from your patch. -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/3] drm/i915: run shmem_pread_fast() after page fault
On Fri, Jul 19, 2013 at 08:52:39AM +, Zhang, Xiong Y wrote: > It just influence one page. > During my test, I found one read slow path info when enable prefault. So > I add this patch. Why don't move fault_in_multipages_writeable() from > i915_gem_shmem_pread() to the beginning of i915_gem_pread_ioctl() ? So > the pread_ioctl() is like pwrite_ioctl(). Is the cost of > fault_in_multipages_writeable() lager than > fault_in_multipages_readable() ? See the attachment. It's a slight matter of correctness fixed in: commit 96d79b52701758404cf8701986891afc99ce810b Author: Daniel Vetter Date: Sun Mar 25 19:47:36 2012 +0200 drm/i915: don't clobber userspace memory before commiting to the pread The issue is that we can't prefault before we know that we'll do the write and prefaulting earlier will write garbage into userspace's memory. Hence the tricky ordering. I guess we should add a comment why this is so to the code. -Daniel > > thanks > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Friday, July 19, 2013 3:29 PM > To: Zhang, Xiong Y > Cc: intel-gfx@lists.freedesktop.org; Chris Wilson > Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: run shmem_pread_fast() after > page fault > > On Fri, Jul 19, 2013 at 01:51:26PM +0800, Xiong Zhang wrote: > > fault_in_multipages_writeable() will load fault page into physical > > memory, then pread can run fast path, no need to run slow path > > > > Signed-off-by: Xiong Zhang > > Hm, this avoids going through the slowpath for the first page. Does this have > an impact on micro-benchmarks (we have a few) or is this just something > you've spotted? I'm a bit vary of making the already complicated shmem > pread/pwrite paths even more complicated. Chris? > -Daniel > > > --- > > drivers/gpu/drm/i915/i915_gem.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c index f194d7e..982df1e 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -459,6 +459,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > > page_do_bit17_swizzling = obj_do_bit17_swizzling && > > (page_to_phys(page) & (1 << 17)) != 0; > > > > +read_again: > > ret = shmem_pread_fast(page, shmem_page_offset, page_length, > >user_data, page_do_bit17_swizzling, > >needs_clflush); > > @@ -475,6 +476,8 @@ i915_gem_shmem_pread(struct drm_device *dev, > > * and just continue. */ > > (void)ret; > > prefaulted = 1; > > + mutex_lock(&dev->struct_mutex); > > + goto read_again; > > } > > > > ret = shmem_pread_slow(page, shmem_page_offset, page_length, > > -- > > 1.8.3.2 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: run shmem_pread_fast() after page fault
It just influence one page. During my test, I found one read slow path info when enable prefault. So I add this patch. Why don't move fault_in_multipages_writeable() from i915_gem_shmem_pread() to the beginning of i915_gem_pread_ioctl() ? So the pread_ioctl() is like pwrite_ioctl(). Is the cost of fault_in_multipages_writeable() lager than fault_in_multipages_readable() ? See the attachment. thanks -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Friday, July 19, 2013 3:29 PM To: Zhang, Xiong Y Cc: intel-gfx@lists.freedesktop.org; Chris Wilson Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: run shmem_pread_fast() after page fault On Fri, Jul 19, 2013 at 01:51:26PM +0800, Xiong Zhang wrote: > fault_in_multipages_writeable() will load fault page into physical > memory, then pread can run fast path, no need to run slow path > > Signed-off-by: Xiong Zhang Hm, this avoids going through the slowpath for the first page. Does this have an impact on micro-benchmarks (we have a few) or is this just something you've spotted? I'm a bit vary of making the already complicated shmem pread/pwrite paths even more complicated. Chris? -Daniel > --- > drivers/gpu/drm/i915/i915_gem.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c index f194d7e..982df1e 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -459,6 +459,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > page_do_bit17_swizzling = obj_do_bit17_swizzling && > (page_to_phys(page) & (1 << 17)) != 0; > > +read_again: > ret = shmem_pread_fast(page, shmem_page_offset, page_length, > user_data, page_do_bit17_swizzling, > needs_clflush); > @@ -475,6 +476,8 @@ i915_gem_shmem_pread(struct drm_device *dev, >* and just continue. */ > (void)ret; > prefaulted = 1; > + mutex_lock(&dev->struct_mutex); > + goto read_again; > } > > ret = shmem_pread_slow(page, shmem_page_offset, page_length, > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch 0001-drm-i915-move-page-fault-handle-to-i915_gem_pread_io.patch Description: 0001-drm-i915-move-page-fault-handle-to-i915_gem_pread_io.patch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Move error uevent to detection time
On Thu, Jul 18, 2013 at 10:42:48PM -0700, Ben Widawsky wrote:
> We've recently deferred error handling with a workqueue for a number of
> reasons. However, when we're trying to pass the information to
> userspace, it's likely more interesting to know when the error was
> detected by the kernel, and not when we eventually get around to
> handling it in the workqueue.
>
> This was inspired as a result of the patch to move these events to the
> uapi.
>
> Signed-off-by: Ben Widawsky
> ---
> drivers/gpu/drm/i915/i915_irq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9910699..9fe430a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1435,13 +1435,10 @@ static void i915_error_work_func(struct work_struct
> *work)
> gpu_error);
> struct drm_device *dev = dev_priv->dev;
> struct intel_ring_buffer *ring;
> - char *error_event[] = { "ERROR=1", NULL };
> char *reset_event[] = { "RESET=1", NULL };
> char *reset_done_event[] = { "ERROR=0", NULL };
> int i, ret;
>
> - kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
> -
> /*
>* Note that there's only one work item which does gpu resets, so we
>* need not worry about concurrent gpu resets potentially incrementing
> @@ -1594,11 +1591,14 @@ void i915_handle_error(struct drm_device *dev, bool
> wedged)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_ring_buffer *ring;
> + char *error_event[] = { "ERROR=1", NULL };
> int i;
>
> i915_capture_error_state(dev);
> i915_report_and_clear_eir(dev);
>
> + kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
> +
> if (wedged) {
> atomic_set_mask(I915_RESET_IN_PROGRESS_FLAG,
> &dev_priv->gpu_error.reset_counter);
This leaves the reset counter unchanged prior to the uevent, if you move
the uevent signalling down to just before the queue_work() there would
be no userspace visible changes (other than timing, which is a moot
point).
However, kobject_uevent_env() is not suitable for calling from
irq-context.
-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/3] drm/i915: run shmem_pread_fast() after page fault
On Fri, Jul 19, 2013 at 01:51:26PM +0800, Xiong Zhang wrote: > fault_in_multipages_writeable() will load fault page into physical > memory, then pread can run fast path, no need to run slow path > > Signed-off-by: Xiong Zhang Hm, this avoids going through the slowpath for the first page. Does this have an impact on micro-benchmarks (we have a few) or is this just something you've spotted? I'm a bit vary of making the already complicated shmem pread/pwrite paths even more complicated. Chris? -Daniel > --- > drivers/gpu/drm/i915/i915_gem.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index f194d7e..982df1e 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -459,6 +459,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > page_do_bit17_swizzling = obj_do_bit17_swizzling && > (page_to_phys(page) & (1 << 17)) != 0; > > +read_again: > ret = shmem_pread_fast(page, shmem_page_offset, page_length, > user_data, page_do_bit17_swizzling, > needs_clflush); > @@ -475,6 +476,8 @@ i915_gem_shmem_pread(struct drm_device *dev, >* and just continue. */ > (void)ret; > prefaulted = 1; > + mutex_lock(&dev->struct_mutex); > + goto read_again; > } > > ret = shmem_pread_slow(page, shmem_page_offset, page_length, > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: add debug info in slow path
On Fri, Jul 19, 2013 at 01:51:25PM +0800, Xiong Zhang wrote:
> Signed-off-by: Xiong Zhang
I guess this was useful for debugging, but printks are rather expensive.
So I'll drop this - if we ever decide that this is useful information we
could expose it as a tracepoint, but since not even the core file i/o path
has that I don't think we need it.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c| 4
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index de59154..f194d7e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -376,6 +376,8 @@ shmem_pread_slow(struct page *page, int
> shmem_page_offset, int page_length,
> char *vaddr;
> int ret;
>
> + DRM_DEBUG("execute pread slow path\n");
> +
> vaddr = kmap(page);
> if (needs_clflush)
> shmem_clflush_swizzled_range(vaddr + shmem_page_offset,
> @@ -689,6 +691,8 @@ shmem_pwrite_slow(struct page *page, int
> shmem_page_offset, int page_length,
> char *vaddr;
> int ret;
>
> + DRM_DEBUG("execute pwrite slow path\n");
> +
> vaddr = kmap(page);
> if (unlikely(needs_clflush_before || page_do_bit17_swizzling))
> shmem_clflush_swizzled_range(vaddr + shmem_page_offset,
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 1734825..3065297 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -587,6 +587,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> int i, total, ret;
> int count = args->buffer_count;
>
> + DRM_DEBUG("Execute relocate slow path\n");
> /* We may process another execbuffer during the unlock... */
> while (!list_empty(&eb->objects)) {
> obj = list_first_entry(&eb->objects,
> --
> 1.8.3.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: add prefault_disable module option
On Fri, Jul 19, 2013 at 01:51:24PM +0800, Xiong Zhang wrote:
> prefault is stll enabled by default which prevent most of pwrite/pread/reloc
> from running slow path, in order to verify these slow pathes, prefault need
> to be disabled.
>
> Signed-off-by: Xiong Zhang
lgtm, queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c| 5 +
> drivers/gpu/drm/i915/i915_drv.h| 1 +
> drivers/gpu/drm/i915/i915_gem.c| 12 +++-
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 --
> 4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b07362f..dac6bd3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -137,6 +137,11 @@ module_param_named(fastboot, i915_fastboot, bool, 0600);
> MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time "
>"(default: false)");
>
> +bool i915_prefault_disable __read_mostly = false;
> +module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
> +MODULE_PARM_DESC(prefault_disable,
> + "Try to disable pre page fault for pread/pwrite/reloc
> (default:false)");
> +
> static struct drm_driver driver;
> extern int intel_agp_enabled;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7fdc8e3..9516e19 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1587,6 +1587,7 @@ extern unsigned int i915_preliminary_hw_support
> __read_mostly;
> extern int i915_disable_power_well __read_mostly;
> extern int i915_enable_ips __read_mostly;
> extern bool i915_fastboot __read_mostly;
> +extern bool i915_prefault_disable __read_mostly;
>
> extern int i915_suspend(struct drm_device *dev, pm_message_t state);
> extern int i915_resume(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c9d9d20..de59154 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -465,7 +465,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
>
> mutex_unlock(&dev->struct_mutex);
>
> - if (!prefaulted) {
> + if (likely(!i915_prefault_disable) && !prefaulted) {
> ret = fault_in_multipages_writeable(user_data, remain);
> /* Userspace is tricking us, but we've already clobbered
>* its pages with the prefault and promised to write the
> @@ -860,10 +860,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void
> *data,
> args->size))
> return -EFAULT;
>
> - ret = fault_in_multipages_readable(to_user_ptr(args->data_ptr),
> -args->size);
> - if (ret)
> - return -EFAULT;
> + if (likely(!i915_prefault_disable)) {
> + ret = fault_in_multipages_readable(to_user_ptr(args->data_ptr),
> +args->size);
> + if (ret)
> + return -EFAULT;
> + }
>
> ret = i915_mutex_lock_interruptible(dev);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 1b58694..1734825 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -759,8 +759,10 @@ validate_exec_list(struct drm_i915_gem_exec_object2
> *exec,
> if (!access_ok(VERIFY_WRITE, ptr, length))
> return -EFAULT;
>
> - if (fault_in_multipages_readable(ptr, length))
> - return -EFAULT;
> + if (likely(!i915_prefault_disable)) {
> + if (fault_in_multipages_readable(ptr, length))
> + return -EFAULT;
> + }
> }
>
> return 0;
> --
> 1.8.3.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] tests/gem_disable_prefault: add reloc slow path subtest
On Fri, Jul 19, 2013 at 01:53:10PM +0800, Xiong Zhang wrote:
> first disable prefault, then put relocate bo in gtt space and exec cmd,
> so it will run relocate_object_slow path, finally enable prefault
>
> Signed-off-by: Xiong Zhang
Hm, my idea was something much simpler like
diff --git a/tests/gem_exec_faulting_reloc.c b/tests/gem_exec_faulting_reloc.c
index 863a1b0..c7f7b6b 100644
--- a/tests/gem_exec_faulting_reloc.c
+++ b/tests/gem_exec_faulting_reloc.c
@@ -220,7 +220,15 @@ static void run(int object_size)
int main(int argc, char **argv)
{
- run(OBJECT_SIZE);
+ drmtest_subtest_init(argc, argv);
+
+ if (drmtest_run_subtest("normal"))
+ run(OBJECT_SIZE);
+ if (drmtest_run_subtest("no-prefault")) {
+ drmtest_disable_prefault();
+ run(OBJECT_SIZE);
+ drmtest_enable_prefault();
+ }
return 0;
}
diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index d759340..138476e 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -144,6 +144,14 @@ test_read(int fd)
munmap(dst, OBJECT_SIZE);
}
+static void
+run_without_prefault(int fd, void (*func)(int fd))
+{
+ drmtest_disable_prefault();
+ func(fd);
+ drmtest_enable_prefault();
+}
+
int main(int argc, char **argv)
{
int fd;
@@ -160,6 +168,12 @@ int main(int argc, char **argv)
test_write(fd);
if (drmtest_run_subtest("write-gtt"))
test_write_gtt(fd);
+ if (drmtest_run_subtest("read-no-prefault"))
+ run_without_prefault(fd, test_read);
+ if (drmtest_run_subtest("write-no-prefault"))
+ run_without_prefault(fd, test_write);
+ if (drmtest_run_subtest("write-gtt-no-prefault"))
+ run_without_prefault(fd, test_write_gtt);
close(fd);
Of course if that's not good enough to exercise the paths then we need to
add more tests. But it'd be a bit less code ;-)
Cheers, Daniel
> ---
> tests/Makefile.am| 1 +
> tests/gem_disable_prefault.c | 168
> +++
> 2 files changed, 169 insertions(+)
> create mode 100644 tests/gem_disable_prefault.c
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 3003aa0..f0804b3 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -106,6 +106,7 @@ TESTS_progs = \
> gem_reg_read \
> gem_tiling_max_stride \
> prime_udl \
> + gem_disable_prefault \
> $(NULL)
>
> # IMPORTANT: The ZZ_ tests need to be run last!
> diff --git a/tests/gem_disable_prefault.c b/tests/gem_disable_prefault.c
> new file mode 100644
> index 000..9e6f0ad
> --- /dev/null
> +++ b/tests/gem_disable_prefault.c
> @@ -0,0 +1,168 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Xiong Zhang
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "drm.h"
> +#include "i915_drm.h"
> +#include "drmtest.h"
> +#include "intel_chipset.h"
> +#include "intel_gpu_tools.h"
> +
> +#define OBJECT_SIZE (16*1024*1024)
> +
> +static void *
> +mmap_bo(int fd, uint32_t handle, int size)
> +{
> + void *ptr;
> +
> + ptr = gem_mmap(fd, handle, size, PROT_READ | PROT_WRITE);
> + assert(ptr != MAP_FAILED);
> +
> + return ptr;
> +}
> +
> +static void gem_exec(int fd, struct drm_i915_gem_execbuffer2 *execbuf)
> +{
> + int ret;
> +
> + ret = drmIoctl(fd,
> + DRM_IOCTL_I915_GEM_EXECBUFFER2,
> + execbuf);
> + assert(ret == 0);
> +}
> +
> +static void
> +test_disable_prefault_reloc(int fd)
> +{
> + struct drm_i915_gem_execbuffer2 execbuf;
> + struct drm_i915_gem_exec_object2 e
Re: [Intel-gfx] [PATCH 1/5] lib/drmtest: add drmtest_disable/enable_prefault() function
On Fri, Jul 19, 2013 at 01:53:08PM +0800, Xiong Zhang wrote:
> Signed-off-by: Xiong Zhang
> ---
> lib/drmtest.c | 43 +++
> lib/drmtest.h | 3 +++
> 2 files changed, 46 insertions(+)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 011d8c1..713c5ff 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -1593,3 +1593,46 @@ void kmstest_free_connector_config(struct
> kmstest_connector_config *config)
> drmModeFreeEncoder(config->encoder);
> drmModeFreeConnector(config->connector);
> }
> +
> +#define PREFAULT_DEBUGFS "/sys/module/i915/parameters/prefault_disable"
> +static int drmtest_prefault_control(bool enable)
> +{
> + char *name = PREFAULT_DEBUGFS;
> + int fd;
> + char buf[2] = {'Y', 'N'};
> + int index;
> + int result = 0;
> +
> + fd = open(name, O_RDWR);
> + if (fd == -1) {
> + fprintf(stderr, "Couldn't open prefault_debugfs.%s\n",
> + strerror(errno));
> + return -1;
> + }
> +
> + if (enable)
> + index = 1;
> + else
> + index = 0;
> +
> + if (write(fd, &buf[index], 1) != 1) {
> + fprintf(stderr, "write prefault_debugfs error.%s\n",
> + strerror(errno));
> + result = -1;
> + }
> +
> + close(fd);
> +
> + return result;
> +}
> +
> +int drmtest_disable_prefault(void)
> +{
> + return drmtest_prefault_control(false);
> +}
If our test crashes this will leave prefaulting disable, potentially
affecting future tests run before rebooting. To prevent such issues Imre
has added exit handler code to make sure that we can undo such things.
I think we should install an exit handler which calls
drmtest_enable_prefault again. See drmtest_install_exit_handler.
-Daniel
> +
> +int drmtest_enable_prefault(void)
> +{
> + return drmtest_prefault_control(true);
> +}
> +
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index e3a9275..80b344c 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -179,3 +179,6 @@ void drmtest_enable_exit_handler(void);
> void drmtest_disable_exit_handler(void);
>
> int drmtest_set_vt_graphics_mode(void);
> +
> +int drmtest_disable_prefault(void);
> +int drmtest_enable_prefault(void);
> --
> 1.8.3.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [patch 2/2] drm/i915: use after free on error path
On Fri, Jul 19, 2013 at 08:46:27AM +0300, Dan Carpenter wrote: > i915_gem_vma_destroy() frees its argument so we have to move the > drm_mm_remove_node() call up a few lines. > > Signed-off-by: Dan Carpenter Both applied, thanks for the patches. -Daniel > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 9a9a77a..f347ad5 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3161,9 +3161,9 @@ search_free: > return 0; > > err_out: > + drm_mm_remove_node(&vma->node); > i915_gem_vma_destroy(vma); > i915_gem_object_unpin_pages(obj); > - drm_mm_remove_node(&vma->node); > return ret; > } > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Change uevent for reset completion
On Thu, Jul 18, 2013 at 10:42:49PM -0700, Ben Widawsky wrote:
> Prior to this patch, we used ERROR=0 as a way of signifying the reset
> was complete. The functionality dates back quite a ways to:
>
> commit f316a42cc49eca73b33d85feb6177e32431747ff
> Author: Ben Gamari
> Date: Mon Sep 14 17:48:46 2009 -0400
>
> drm/i915: Hookup chip reset in error handler
>
> I'm not really sure what the motivation for this was originally, but to
> me it makes more sense to have a distinct event for error detection, and
> another event for reset start/finish (since reset is prone to failure).
>
> Signed-off-by: Ben Widawsky
> ---
> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9fe430a..03071d7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1436,7 +1436,7 @@ static void i915_error_work_func(struct work_struct
> *work)
> struct drm_device *dev = dev_priv->dev;
> struct intel_ring_buffer *ring;
> char *reset_event[] = { "RESET=1", NULL };
> - char *reset_done_event[] = { "ERROR=0", NULL };
> + char *reset_done_event[] = { "RESET=0", NULL };
It's a kernel api change so we need to check all distros and everything
else out there whether they don't use it. Imo not worth the effort since
signalling that the ERROR condition has passed is somewhat sensible, too.
Can you please respin with this part dropped?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: extract lpt_enable_clkout_dp from lpt_init_pch_refclk
On Fri, Jul 12, 2013 at 02:19:38PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > > The next step is to modify lpt_enable_clkout_dp to enable support for > "Sequence to enable CLKOUT_DP" and "Sequence to enable CLKOUT_DP > without spread". > > Signed-off-by: Paulo Zanoni I've merged the first three patches in this series to dinq, thanks for patches and review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
