Re: [Intel-gfx] [PATCH 1/3] drm/i915: remove bogus mutex_unlock from error-path

2013-02-12 Thread Ben Widawsky
On Tue, Feb 12, 2013 at 03:36:03PM +0100, Daniel Vetter wrote:
> This has been lost in the locking rework for intel_alloc_context_page:
> 
> commit 2c34b850ee1e9f86b41706149d0954eee58757a3
> Author: Ben Widawsky 
> Date:   Sat Mar 19 18:14:26 2011 -0700
> 
> drm/i915: fix ilk rc6 teardown locking
> 
> Cc: Ben Widawsky 
> Signed-off-by: Daniel Vetter 

I'm confused about how it got lost.
Reviewed-by: Ben Widawsky 

> ---
>  drivers/gpu/drm/i915/intel_pm.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3280cff..7658c39 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2286,7 +2286,6 @@ err_unpin:
>   i915_gem_object_unpin(ctx);
>  err_unref:
>   drm_gem_object_unreference(&ctx->base);
> - mutex_unlock(&dev->struct_mutex);
>   return NULL;
>  }
>  
> -- 
> 1.7.10.4
> 

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix PIPE_CONTROL DW/QW write through global GTT on IVB+

2013-02-12 Thread Ben Widawsky
On Tue, Feb 12, 2013 at 10:01:39PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> The bit controlling whether PIPE_CONTROL DW/QW write targets
> the global GTT or PPGTT moved moved from DW 2 bit 2 to
> DW 1 bit 24 on IVB.
> 
> TODO: need to test on IVB actually and make sure things don't
> explode.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 1 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d7542cd..69a95c6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -308,6 +308,7 @@
>  #define   DISPLAY_PLANE_A   (0<<20)
>  #define   DISPLAY_PLANE_B   (1<<20)
>  #define GFX_OP_PIPE_CONTROL(len) ((0x3<<29)|(0x3<<27)|(0x2<<24)|(len-2))
> +#define   PIPE_CONTROL_GLOBAL_GTT_IVB(1<<24) /* 
> gen7+ */
>  #define   PIPE_CONTROL_CS_STALL  (1<<20)
>  #define   PIPE_CONTROL_TLB_INVALIDATE(1<<18)
>  #define   PIPE_CONTROL_QW_WRITE  (1<<14)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 9b8b058..f397bd7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -330,8 +330,8 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>   return ret;
>  
>   intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
> - intel_ring_emit(ring, flags);
> - intel_ring_emit(ring, scratch_addr | PIPE_CONTROL_GLOBAL_GTT);
> + intel_ring_emit(ring, flags | PIPE_CONTROL_GLOBAL_GTT_IVB);
> + intel_ring_emit(ring, scratch_addr);
>   intel_ring_emit(ring, 0);
>   intel_ring_advance(ring);
>  
In the fashion of my favorite maintainer... bikeshed!
Can you move the PIPE_CONTROL_GLOBAL_GTT_IVB up to where we set
CS_STALL (or really anywhere else). That way we have a consistent way of
setting flags, ie. dword2


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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Print the pipe control page GTT address

2013-02-12 Thread Daniel Vetter
On Tue, Feb 12, 2013 at 10:01:38PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> We already print the HWS addresses during init, so do the same for the
> pipe control page. Reduces guesswork when looking at hex addresses
> later.
> 
> Signed-off-by: Ville Syrjälä 

Queued for -next, thanks for the patch. I'll hold off on patch 2 until the
report from the bomb squad is in ...
-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 2/2] drm/i915: Fix PIPE_CONTROL DW/QW write through global GTT on IVB+

2013-02-12 Thread ville . syrjala
From: Ville Syrjälä 

The bit controlling whether PIPE_CONTROL DW/QW write targets
the global GTT or PPGTT moved moved from DW 2 bit 2 to
DW 1 bit 24 on IVB.

TODO: need to test on IVB actually and make sure things don't
explode.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d7542cd..69a95c6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -308,6 +308,7 @@
 #define   DISPLAY_PLANE_A   (0<<20)
 #define   DISPLAY_PLANE_B   (1<<20)
 #define GFX_OP_PIPE_CONTROL(len)   ((0x3<<29)|(0x3<<27)|(0x2<<24)|(len-2))
+#define   PIPE_CONTROL_GLOBAL_GTT_IVB  (1<<24) /* gen7+ */
 #define   PIPE_CONTROL_CS_STALL(1<<20)
 #define   PIPE_CONTROL_TLB_INVALIDATE  (1<<18)
 #define   PIPE_CONTROL_QW_WRITE(1<<14)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9b8b058..f397bd7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -330,8 +330,8 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
return ret;
 
intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
-   intel_ring_emit(ring, flags);
-   intel_ring_emit(ring, scratch_addr | PIPE_CONTROL_GLOBAL_GTT);
+   intel_ring_emit(ring, flags | PIPE_CONTROL_GLOBAL_GTT_IVB);
+   intel_ring_emit(ring, scratch_addr);
intel_ring_emit(ring, 0);
intel_ring_advance(ring);
 
-- 
1.7.12.4

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


[Intel-gfx] [PATCH 1/2] drm/i915: Print the pipe control page GTT address

2013-02-12 Thread ville . syrjala
From: Ville Syrjälä 

We already print the HWS addresses during init, so do the same for the
pipe control page. Reduces guesswork when looking at hex addresses
later.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 00525ff..9b8b058 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -467,6 +467,9 @@ init_pipe_control(struct intel_ring_buffer *ring)
if (pc->cpu_page == NULL)
goto err_unpin;
 
+   DRM_DEBUG_DRIVER("%s pipe control offset: 0x%08x\n",
+ring->name, pc->gtt_offset);
+
pc->obj = obj;
ring->private = pc;
return 0;
-- 
1.7.12.4

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


Re: [Intel-gfx] i915 PCH backlight vs. Dell XPS13

2013-02-12 Thread Kamal Mostafa
On Fri, Feb 8, 2013 at 8:36 PM, Kamal Mostafa  wrote:
> > Daniel's backlight fixes which landed in 3.6 did enable working
> > backlight controls on the XPS13, but it appears that this commit from
> > Paolo broke it again:
> > a4f32fc drm/i915: don't forget the PCH backlight registers

On Fri, 2013-02-08 at 20:55 +0100, Daniel Vetter wrote:
> You might want to try the latest drm-intel-nightly git branch from
> http://cgit.freedesktop.org/~danvet/drm-intel This has a few more
> tricks which helped on some similar platforsm to yours (and similar
> resume issues).


Yes indeed, this commit from drm-intel-nightly does fix the problem:
"drm/i915: write backlight harder".

Thanks as always, for your help Daniel!

 -Kamal



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: move LVDS support check to output setup

2013-02-12 Thread Chris Wilson
On Tue, Feb 12, 2013 at 07:37:08PM +0200, Jani Nikula wrote:
> On Tue, 12 Feb 2013, Chris Wilson  wrote:
> > On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote:
> >> Keep all the platform output selection in intel_output_setup(), and don't
> >> scatter it around.
> >
> > I see this as doing the opposite. You are littering an already over
> > complicated routine with LVDS specific information.
> 
> I think it's fairly straightforward to follow what's happening there,
> and with all the platform dependent stuff in the same place you don't
> have to dig down from that complicated routine to look for *other*
> places where there are platform dependent ifs and checks. Especially if
> we need to add *more* of those deep down.

I consider the rest of those to be a mess of repetious code that would
have been simpler had the platform specific presence checks been pushed
down. The only critical aspect of that function is the ordering and more
importantly when to and when not to initialise certain outputs. That I
feel is hidden in the forest of ifs.

Just my 2 cents.
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915: move LVDS support check to output setup

2013-02-12 Thread Jani Nikula
On Tue, 12 Feb 2013, Chris Wilson  wrote:
> On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote:
>> Keep all the platform output selection in intel_output_setup(), and don't
>> scatter it around.
>
> I see this as doing the opposite. You are littering an already over
> complicated routine with LVDS specific information.

I think it's fairly straightforward to follow what's happening there,
and with all the platform dependent stuff in the same place you don't
have to dig down from that complicated routine to look for *other*
places where there are platform dependent ifs and checks. Especially if
we need to add *more* of those deep down.

>> As a useful side effect, do not try to enable LVDS on
>> HSW or VLV.
>
> But you wouldn't with the old arrangement either.

On HSW it would depend on I915_READ(PCH_LVDS) & LVDS_DETECTED, which I
believe it shouldn't, and VBT. On mobile VLV it depends on VBT only. And
I think we know better than the VBT. Unless I'm missing something and
you have to spell it out for me... :/

>> Some checks are done in a slightly different order than before, and on some
>> platforms VGA is now initialized before LVDS.
>
> Is that significant?

I don't think so, but I thought I'd write it down as a note just in case
it turns out to be significant.

> You have not sold me on the benefits of this change.

The main motivation was:
http://mid.gmane.org/cakmk7ugxfyuzcksbdkqqf15lgcp9vqgkwcu1-hhtq0vmrzv...@mail.gmail.com


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


Re: [Intel-gfx] [PATCH] drm/i915: move LVDS support check to output setup

2013-02-12 Thread Chris Wilson
On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote:
> Keep all the platform output selection in intel_output_setup(), and don't
> scatter it around.

I see this as doing the opposite. You are littering an already over
complicated routine with LVDS specific information.

> As a useful side effect, do not try to enable LVDS on
> HSW or VLV.

But you wouldn't with the old arrangement either.
 
> Some checks are done in a slightly different order than before, and on some
> platforms VGA is now initialized before LVDS.

Is that significant?

You have not sold me on the benefits of this change.
-Chris

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


[Intel-gfx] [PATCH] drm/i915: move LVDS support check to output setup

2013-02-12 Thread Jani Nikula
Keep all the platform output selection in intel_output_setup(), and don't
scatter it around. As a useful side effect, do not try to enable LVDS on
HSW or VLV.

Some checks are done in a slightly different order than before, and on some
platforms VGA is now initialized before LVDS.

Signed-off-by: Jani Nikula 

---

I took this out of the VLV series to give it the attention it needs.
---
 drivers/gpu/drm/i915/intel_display.c |   25 -
 drivers/gpu/drm/i915/intel_lvds.c|   24 
 2 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d75c6a0..a1fb320 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8198,12 +8198,19 @@ static void intel_setup_outputs(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *encoder;
bool dpd_is_edp = false;
-   bool has_lvds;
 
-   has_lvds = intel_lvds_init(dev);
-   if (!has_lvds && !HAS_PCH_SPLIT(dev)) {
-   /* disable the panel fitter on everything but LVDS */
-   I915_WRITE(PFIT_CONTROL, 0);
+   if (!HAS_PCH_SPLIT(dev)) {
+   bool has_lvds = false;
+
+   /* Prior to PCH split LVDS was only attached to mobile products,
+* except for the inglorious 830gm */
+   if (IS_MOBILE(dev) && !IS_I830(dev))
+   has_lvds = intel_lvds_init(dev);
+
+   if (!has_lvds) {
+   /* disable the panel fitter on everything but LVDS */
+   I915_WRITE(PFIT_CONTROL, 0);
+   }
}
 
if (!(HAS_DDI(dev) && (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)))
@@ -8230,6 +8237,14 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_ddi_init(dev, PORT_D);
} else if (HAS_PCH_SPLIT(dev)) {
int found;
+
+   if (I915_READ(PCH_LVDS) & LVDS_DETECTED) {
+   if (dev_priv->edp.support)
+   DRM_DEBUG_KMS("disable LVDS for eDP support\n");
+   else
+   intel_lvds_init(dev);
+   }
+
dpd_is_edp = intel_dpd_is_edp(dev);
 
if (has_edp_a(dev))
diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 7e4ec63..3c76e91 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1024,18 +1024,6 @@ static bool compute_is_dual_link_lvds(struct 
intel_lvds_encoder *lvds_encoder)
return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;
 }
 
-static bool intel_lvds_supported(struct drm_device *dev)
-{
-   /* With the introduction of the PCH we gained a dedicated
-* LVDS presence pin, use it. */
-   if (HAS_PCH_SPLIT(dev))
-   return true;
-
-   /* Otherwise LVDS was only attached to mobile products,
-* except for the inglorious 830gm */
-   return IS_MOBILE(dev) && !IS_I830(dev);
-}
-
 /**
  * intel_lvds_init - setup LVDS connectors on this device
  * @dev: drm device
@@ -1060,9 +1048,6 @@ bool intel_lvds_init(struct drm_device *dev)
int pipe;
u8 pin;
 
-   if (!intel_lvds_supported(dev))
-   return false;
-
/* Skip init on machines we know falsely report LVDS */
if (dmi_check_system(intel_no_lvds))
return false;
@@ -1073,15 +1058,6 @@ bool intel_lvds_init(struct drm_device *dev)
return false;
}
 
-   if (HAS_PCH_SPLIT(dev)) {
-   if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0)
-   return false;
-   if (dev_priv->edp.support) {
-   DRM_DEBUG_KMS("disable LVDS for eDP support\n");
-   return false;
-   }
-   }
-
lvds_encoder = kzalloc(sizeof(struct intel_lvds_encoder), GFP_KERNEL);
if (!lvds_encoder)
return false;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 3/3] drm/i915: push struct_mutex locking down to ironlake_enable_rc6

2013-02-12 Thread Daniel Vetter
Originally dev->struct_mutex protected all kinds of things for rps/ips
(debugfs access, driver state, ...). Nowadays we have
dev_priv->rps.lock for this, so only the ilk ips code needs it really.

Push the locking down. I haven't yet changed the cleanup side since
there we lock large parts of the code with struct_mutex.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c |2 --
 drivers/gpu/drm/i915/intel_pm.c  |   13 +
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index da1ad9c..91e99a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8945,9 +8945,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
 
intel_init_clock_gating(dev);
 
-   mutex_lock(&dev->struct_mutex);
intel_enable_gt_powersave(dev);
-   mutex_unlock(&dev->struct_mutex);
 }
 
 void intel_modeset_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7658c39..3ed7114 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2715,6 +2715,8 @@ void ironlake_teardown_rc6(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
if (dev_priv->ips.renderctx) {
i915_gem_object_unpin(dev_priv->ips.renderctx);
drm_gem_object_unreference(&dev_priv->ips.renderctx->base);
@@ -2778,11 +2780,11 @@ static void ironlake_enable_rc6(struct drm_device *dev)
if (!intel_enable_rc6(dev))
return;
 
-   WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+   mutex_lock(&dev->struct_mutex);
 
ret = ironlake_setup_rc6(dev);
if (ret)
-   return;
+   goto unlock;
 
was_interruptible = dev_priv->mm.interruptible;
dev_priv->mm.interruptible = false;
@@ -2795,7 +2797,7 @@ static void ironlake_enable_rc6(struct drm_device *dev)
if (ret) {
ironlake_teardown_rc6(dev);
dev_priv->mm.interruptible = was_interruptible;
-   return;
+   goto unlock;
}
 
intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
@@ -2820,11 +2822,14 @@ static void ironlake_enable_rc6(struct drm_device *dev)
if (ret) {
DRM_ERROR("failed to enable ironlake power power savings\n");
ironlake_teardown_rc6(dev);
-   return;
+   goto unlock;
}
 
I915_WRITE(PWRCTXA, dev_priv->ips.pwrctx->gtt_offset | PWRCTX_EN);
I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT);
+
+unlock:
+   mutex_lock(&dev->struct_mutex);
 }
 
 static unsigned long intel_pxfreq(u32 vidfreq)
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 2/3] drm/i915: cancel console_resume_work before destroying the fbdev

2013-02-12 Thread Daniel Vetter
Could lead to tears if the work indeed runs after we've destroyed the
fbdev ... Totally unlikely to happen though in reality.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_dma.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 99daa89..363d2d0 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1684,9 +1684,9 @@ int i915_driver_unload(struct drm_device *dev)
acpi_video_unregister();
 
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+   cancel_work_sync(&dev_priv->console_resume_work);
intel_fbdev_fini(dev);
intel_modeset_cleanup(dev);
-   cancel_work_sync(&dev_priv->console_resume_work);
 
/*
 * free the memory space allocated for the child device
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 1/3] drm/i915: remove bogus mutex_unlock from error-path

2013-02-12 Thread Daniel Vetter
This has been lost in the locking rework for intel_alloc_context_page:

commit 2c34b850ee1e9f86b41706149d0954eee58757a3
Author: Ben Widawsky 
Date:   Sat Mar 19 18:14:26 2011 -0700

drm/i915: fix ilk rc6 teardown locking

Cc: Ben Widawsky 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_pm.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3280cff..7658c39 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2286,7 +2286,6 @@ err_unpin:
i915_gem_object_unpin(ctx);
 err_unref:
drm_gem_object_unreference(&ctx->base);
-   mutex_unlock(&dev->struct_mutex);
return NULL;
 }
 
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 0/3] some locking clarifications

2013-02-12 Thread Daniel Vetter
Hi all,

Three patches for little things I've recently noticed. The long-term aim behind
the last one is to slowly reduce the scope of struct_mutex and clarify what's
actually protected by it.

Comments highly welcome.

Cheers, Daniel

Daniel Vetter (3):
  drm/i915: remove bogus mutex_unlock from error-path
  drm/i915: cancel console_resume_work before destroying the fbdev
  drm/i915: push struct_mutex locking down to ironlake_enable_rc6

 drivers/gpu/drm/i915/i915_dma.c  |2 +-
 drivers/gpu/drm/i915/intel_display.c |2 --
 drivers/gpu/drm/i915/intel_pm.c  |   14 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

-- 
1.7.10.4

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


[Intel-gfx] [PATCH i-g-t] tests: Forbid to run the blit tests with count of 1

2013-02-12 Thread Damien Lespiau
From: Damien Lespiau 

Invoking say,

  sudo ./tests/gem_render_linear_blits 1

does not make a lot of sense as we're creating a single bo. The test
does not yell at you and passes, even if the rendercopy function does
not do anything. This makes it quite harmful when trying to debug
rendercopy without realizing that count is the number of allocated bos
and must be >= 2.

Signed-off-by: Damien Lespiau 
---
 tests/gem_linear_blits.c| 4 
 tests/gem_render_linear_blits.c | 5 +
 tests/gem_render_tiled_blits.c  | 5 +
 tests/gem_tiled_blits.c | 3 +++
 4 files changed, 17 insertions(+)

diff --git a/tests/gem_linear_blits.c b/tests/gem_linear_blits.c
index fe15f1d..ec2dc56 100644
--- a/tests/gem_linear_blits.c
+++ b/tests/gem_linear_blits.c
@@ -189,6 +189,10 @@ int main(int argc, char **argv)
count = atoi(argv[1]);
if (count == 0)
count = 3 * gem_aperture_size(fd) / (1024*1024) / 2;
+   else if (count < 2) {
+   fprintf(stderr, "count must be >= 2\n");
+   return 1;
+   }
 
if (count > intel_get_total_ram_mb() * 9 / 10) {
count = intel_get_total_ram_mb() * 9 / 10;
diff --git a/tests/gem_render_linear_blits.c b/tests/gem_render_linear_blits.c
index 8ba24a3..a7e0189 100644
--- a/tests/gem_render_linear_blits.c
+++ b/tests/gem_render_linear_blits.c
@@ -85,6 +85,11 @@ int main(int argc, char **argv)
count = atoi(argv[1]);
if (count == 0)
count = 3 * gem_aperture_size(fd) / SIZE / 2;
+   else if (count < 2) {
+   fprintf(stderr, "count must be >= 2\n");
+   return 1;
+   }
+
printf("Using %d 1MiB buffers\n", count);
 
bo = malloc(sizeof(*bo)*count);
diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c
index 31b2ee1..626e652 100644
--- a/tests/gem_render_tiled_blits.c
+++ b/tests/gem_render_tiled_blits.c
@@ -88,6 +88,11 @@ int main(int argc, char **argv)
count = atoi(argv[1]);
if (count == 0)
count = 3 * gem_aperture_size(fd) / SIZE / 2;
+   else if (count < 2) {
+   fprintf(stderr, "count must be >= 2\n");
+   return 1;
+   }
+
printf("Using %d 1MiB buffers\n", count);
 
buf = malloc(sizeof(*buf)*count);
diff --git a/tests/gem_tiled_blits.c b/tests/gem_tiled_blits.c
index a6d..bb43976 100644
--- a/tests/gem_tiled_blits.c
+++ b/tests/gem_tiled_blits.c
@@ -132,6 +132,9 @@ int main(int argc, char **argv)
if (count == 0) {
count = 3 * gem_aperture_size(fd) / (1024*1024) / 2;
count += (count & 1) == 0;
+   } else if (count < 2) {
+   fprintf(stderr, "count must be >= 2\n");
+   return 1;
}
 
if (count > intel_get_total_ram_mb() * 9 / 10) {
-- 
1.7.11.7

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


[Intel-gfx] [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl

2013-02-12 Thread Chris Wilson
By exporting the ability to map user address and inserting PTEs
representing their backing pages into the GTT, we can exploit UMA in order
to utilize normal application data as a texture source or even as a
render target (depending upon the capabilities of the chipset). This has
a number of uses, with zero-copy downloads to the GPU and efficient
readback making the intermixed streaming of CPU and GPU operations
fairly efficient. This ability has many widespread implications from
faster rendering of client-side software rasterisers (chromium),
mitigation of stalls due to read back (firefox) and to faster pipelining
of texture data (such as pixel buffer objects in GL or data blobs in CL).

v2: Compile with CONFIG_MMU_NOTIFIER
v3: We can sleep while performing invalidate-range, which we can utilise
to drop our page references prior to the kernel manipulating the vma
(for either discard or cloning) and so protect normal users.
v4: Only run the invalidate notifier if the range intercepts the bo.
v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/Makefile  |1 +
 drivers/gpu/drm/i915/i915_dma.c|1 +
 drivers/gpu/drm/i915/i915_drv.h|   22 ++
 drivers/gpu/drm/i915/i915_gem.c|   31 ++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |7 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c|  329 
 include/uapi/drm/i915_drm.h|   16 ++
 7 files changed, 393 insertions(+), 14 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 91f3ac6..42858f6 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
  i915_gem_gtt.o \
  i915_gem_stolen.o \
  i915_gem_tiling.o \
+ i915_gem_userptr.o \
  i915_sysfs.o \
  i915_trace_points.o \
  i915_ums.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4fa6beb..9b1984c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1883,6 +1883,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, 
i915_gem_context_create_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, 
i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, 
DRM_UNLOCKED),
 };
 
 int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 923dc0a..90070f4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* General customization:
@@ -1076,6 +1077,7 @@ struct drm_i915_gem_object_ops {
 */
int (*get_pages)(struct drm_i915_gem_object *);
void (*put_pages)(struct drm_i915_gem_object *);
+   void (*release)(struct drm_i915_gem_object *);
 };
 
 struct drm_i915_gem_object {
@@ -1222,6 +1224,23 @@ struct drm_i915_gem_object {
 };
 #define to_gem_object(obj) (&((struct drm_i915_gem_object *)(obj))->base)
 
+struct i915_gem_userptr_object {
+   struct drm_i915_gem_object gem;
+   uintptr_t user_ptr;
+   size_t user_size;
+   int read_only;
+
+   struct mm_struct *mm;
+#if defined(CONFIG_MMU_NOTIFIER)
+   struct mmu_notifier mn;
+#endif
+};
+
+union drm_i915_gem_objects {
+   struct drm_i915_gem_object base;
+   struct i915_gem_userptr_object userptr;
+};
+
 #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
 
 /**
@@ -1501,6 +1520,8 @@ int i915_gem_entervt_ioctl(struct drm_device *dev, void 
*data,
   struct drm_file *file_priv);
 int i915_gem_leavevt_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_priv);
+int i915_gem_userptr_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file);
 int i915_gem_set_tiling(struct drm_device *dev, void *data,
struct drm_file *file_priv);
 int i915_gem_get_tiling(struct drm_device *dev, void *data,
@@ -1554,6 +1575,7 @@ static inline void i915_gem_object_unpin_pages(struct 
drm_i915_gem_object *obj)
BUG_ON(obj->pages_pin_count == 0);
obj->pages_pin_count--;
 }
+int i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
 
 int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
 int i915_gem_object_sync(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73b1e9e..65a36bf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/

Re: [Intel-gfx] [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds

2013-02-12 Thread Daniel Vetter
On Fri, Feb 08, 2013 at 04:35:38PM +0200, Mika Kuoppala wrote:
> commit c1d1f5aeda2033d96e872f416388653f05d4c16d
> Author: Mika Kuoppala 
> Date:   Tue Feb 5 17:26:52 2013 +0200
> 
> drm/i915: disable shared panel fitter for pipe
> 
> moved panel fit disabling to be crtc property. Thus
> the need to explicitly turn pfit off in encoder side
> became obsolete. Take advantage of that and get rid of
> pfit_dirty state handling in lvds.
> 
> Signed-off-by: Mika Kuoppala 

Merged both patches, with the bit of bikeshed applied as discussed on irc.
-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] drm/i915: unify HDMI/DP hpd definitions

2013-02-12 Thread Daniel Vetter
On Thu, Feb 07, 2013 at 11:15:26AM -0200, Paulo Zanoni wrote:
> 2013/2/7 Daniel Vetter :
> > They're physically the same pins and also the same bits, duplicating
> > only confuses the reader. This also makes it a bit obvious that we
> > have quite some code duplication going on here. Squashing that is for
> > a larger rework in our hpd handling though.
> >
> > Signed-off-by: Daniel Vetter 
> 
> Reviewed-by: Paulo Zanoni 
Queued for -next, thanks for the 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] drm/i915: detect wrong MCH watermark values

2013-02-12 Thread Daniel Vetter
On Sat, Feb 09, 2013 at 12:24:02PM -0800, Ben Widawsky wrote:
> On Sat, Feb 09, 2013 at 09:03:42PM +0100, Daniel Vetter wrote:
> > Some early bios versions seem to ship with the wrong tuning values for
> > the MCH, possible resulting in pipe underruns under load. Especially
> > on DP outputs this can lead to black screen, since DP really doesn't
> > like an occasional whack from an underrun.
> > 
> > Unfortunately the registers seem to be locked after boot, so the only
> > thing we can do is politely point out issues and suggest a BIOS
> > upgrade.
> > 
> > Arthur Runyan pointed us at this issue while discussion DP bugs - thus
> > far no confirmation from a bug report yet that it helps. But at least
> > some of my machines here have wrong values, so this might be useful in
> > understanding bug reports.
> > 
> > v2: After a bit more discussion with Art and Ben we've decided to only
> > the check the watermark values, since the OREF ones could be be a
> > notch more aggressive on certain machines.
> > 
> > Cc: Ben Widawsky 
> > Cc: Runyan, Arthur J 
> > Signed-off-by: Daniel Vetter 
> 
> I wouldn't mind if you printed the OREF values, even if you didn't use
> them to detect.

They're different already on my machine than the most conservative
recommended values, so I've thought it'll be of little value to us. If the
hw guys need them, we can always ask reporters for it.

> But,
> Reviewed-by: Ben Widawsky 

Thanks for the review, patch merged.
-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 v2] drm/i915/ctx: Remove bad invariant

2013-02-12 Thread Daniel Vetter
On Mon, Feb 11, 2013 at 01:31:27PM -0800, Ben Widawsky wrote:
> It's not that the assertion is incorrect, but rather that we can call
> do_destroy early in loading, and we will falsely BUG().
> 
> Since contexts have been in for a while now, and in the internal APIs
> are pretty stable, it should be fairly safe to remove this.
> 
> v2: Remove unused dev_priv, and dev
> 
> 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