Re: [Intel-gfx] [PATCH 0/5] refactor power management into intel_pm

2012-04-17 Thread Daniel Vetter
On Mon, Apr 16, 2012 at 10:20:33PM -0300, Eugeni Dodonov wrote:
 As Chris Wilson noticed, my previous patch that did the refactoring as one big
 patch which moved everything at once was extremely difficult to review and
 maintain. So I split the same refactoring into a series of smaller patches,
 which move one subsystem at a time.

Yeah, diffs are not actually readable, so looks like we have to do this
piece-wise. One thing that really bugs me is that we have all these
per-chip function declared as extern now, and the setup code for them is
somewhere completely different.

So can you add another patch add the end that moves all that setup code
intel intel_init_pm (at the end of intel_pm.c to avoid forward decl) and
removes all these chip-specific extern declarations?

If that patch doesn't result in only a few generic functinos exported from
intel_pm.c (and hence a simple interface) I think we need to reconsider
whether we've moved the right things in there (maybe it needs more/less).
But I can't easily judge that atm.
Yours, Daniel

 
 As previously, this reduces the intel_display.c module by around 2800 lines
 while also simplifying future power-related developments.
 
 
 Eugeni Dodonov (5):
   drm/i915: move fbc-related functionality into intel_pm module
   drm/i915: move watermarks settings into intel_pm module
   drm/i915: move drps, rps and rc6-related functions to intel_pm
   drm/i915: move emon functionality into intel_pm module
   drm/i915: move clock gating functionality into intel_pm module
 
  drivers/gpu/drm/i915/Makefile|1 +
  drivers/gpu/drm/i915/intel_display.c | 7803 
 +++---
  drivers/gpu/drm/i915/intel_drv.h |   72 +
  drivers/gpu/drm/i915/intel_pm.c  | 2898 +
  4 files changed, 5432 insertions(+), 5342 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/intel_pm.c
 
 -- 
 1.7.10
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Wait for all pending operations to the fb before disabling the pipe

2012-04-17 Thread Chris Wilson
During modeset we have to disable the pipe to reconfigure its timings
and maybe its size. Userspace may have queued up command buffers that
depend upon the pipe running in a certain configuration and so the
commands may become confused across the modeset. At the moment, we use a
less than satisfactory kick-scanline-waits should the GPU hang during
the modeset. It should be more reliable to wait for the pending
operations to complete first, even though we still have a window for
userspace to submit a broken command buffer during the modeset.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/intel_display.c |   27 ---
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8be3091..72980be 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2910,16 +2910,14 @@ static void intel_clear_scanline_wait(struct drm_device 
*dev)
 
 static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 {
-   struct drm_i915_gem_object *obj;
-   struct drm_i915_private *dev_priv;
+   struct drm_device *dev = crtc-dev;
 
if (crtc-fb == NULL)
return;
 
-   obj = to_intel_framebuffer(crtc-fb)-obj;
-   dev_priv = crtc-dev-dev_private;
-   wait_event(dev_priv-pending_flip_queue,
-  atomic_read(obj-pending_flip) == 0);
+   mutex_lock(dev-struct_mutex);
+   intel_finish_fb(crtc-fb);
+   mutex_unlock(dev-struct_mutex);
 }
 
 static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
@@ -3381,23 +3379,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
struct drm_crtc_helper_funcs *crtc_funcs = crtc-helper_private;
struct drm_device *dev = crtc-dev;
 
-   /* Flush any pending WAITs before we disable the pipe. Note that
-* we need to drop the struct_mutex in order to acquire it again
-* during the lowlevel dpms routines around a couple of the
-* operations. It does not look trivial nor desirable to move
-* that locking higher. So instead we leave a window for the
-* submission of further commands on the fb before we can actually
-* disable it. This race with userspace exists anyway, and we can
-* only rely on the pipe being disabled by userspace after it
-* receives the hotplug notification and has flushed any pending
-* batches.
-*/
-   if (crtc-fb) {
-   mutex_lock(dev-struct_mutex);
-   intel_finish_fb(crtc-fb);
-   mutex_unlock(dev-struct_mutex);
-   }
-
crtc_funcs-dpms(crtc, DRM_MODE_DPMS_OFF);
assert_plane_disabled(dev-dev_private, to_intel_crtc(crtc)-plane);
assert_pipe_disabled(dev-dev_private, to_intel_crtc(crtc)-pipe);
-- 
1.7.10

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


Re: [Intel-gfx] [PATCH 5/5] drm/i915: set stc evict disable lra evict w/a

2012-04-17 Thread Daniel Vetter
On Wed, Apr 11, 2012 at 08:42:42PM +0200, Daniel Vetter wrote:
 Our workaround list kindly lists that this new default value needs to
 be updated in Bspec. Naturally, this did not happen.
 
 Acked-by: Ben Widawsky b...@bwidawsk.net
 Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch

I've queued the entire series for -next.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: enable semaphores on gen6 if dmar is not active

2012-04-17 Thread Daniel Vetter
On Tue, Apr 03, 2012 at 12:32:05AM +0200, Daniel Vetter wrote:
 On Mon, Apr 02, 2012 at 02:44:14PM -0700, Andrew Lutomirski wrote:
  On Mon, Apr 2, 2012 at 1:52 PM, Daniel Vetter dan...@ffwll.ch wrote:
   On Mon, Apr 02, 2012 at 01:45:39PM -0700, Andrew Lutomirski wrote:
   On Mon, Apr 2, 2012 at 11:48 AM, Daniel Vetter daniel.vet...@ffwll.ch 
   wrote:
Inspired by the recent ppgtt regression report, where switching of
dmar only for the gpu seems to fix things completely, I've looked
again at the semaphores+vt-d situation.
   
Contrary to my earlier testing a few months back my system is now
stable with dmar disabled for the igd, and not only when disabling
dmar completely.
   
So I'm rather hopeful that all our recent fixes for snb have changed
things for code and it's time to try enabling semaphores again. We've
also had issues with enabling semaphores which are not vt-d related,
but I guess these are all fixed by the autoreport-disabling and lazy
request fix. And there's only one way to find out whether there are
still other issues ...
   
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
   
---
   
If no further vt-d regressions show up in the 3.4 cycle I plan to
merge this into -next for 3.5 (in a month or so). Comments about how
unfeasibly you deem this highly welcome.
-Daniel
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)
   
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8e0b686..ac52433 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -849,9 +849,11 @@ intel_enable_semaphores(struct drm_device *dev)
       if (i915_semaphores = 0)
               return i915_semaphores;
   
-       /* Disable semaphores on SNB */
-       if (INTEL_INFO(dev)-gen == 6)
-               return 0;
+#ifdef CONFIG_INTEL_IOMMU
+       /* Disable semaphores on SNB if VT-d is on. */
+       if (INTEL_INFO(dev)-gen == 6  intel_iommu_gfx_mapped)
+               return false;
  
   It might be nice to have a printk here giving instructions for how to 
   work
   around this.  For example:
  
   i915: Disabling semaphores to work around a DMAR issue.  As an
   alternative, boot with intel_iommu=igfx_off [or on, igfx_off, or
   whatever it's supposed to be].
  
   The documentation in kernel-parameters.txt is at best unclear to the
   uninitiated.
  
   If this really does work, I'll look into this. Atm it's still unclear
   where to exactly put the plain, we need to annoy internal hardware people
   to analyze this. Once we have enough evidence that the combination of gpu
   with various features enable plus VT-d just doesn't seem to work reliably.
  
   So can you check whether things do indeed work with this patch, atop
   kernel 3.4-rc1? Iirc you've been the one with the most trouble when
   semaphores are enabled ...
  
  I've had a hard time reproducing the problems lately.  The
  unconditional instant hard hang went away a few months ago, I think.
  I'll give it another shot when I get home to that machine.
 
 Well, I've managed to kill my machine shortly after login with semaphores,
 rc6 and dmar enabled. It hasn't died in the same configuration after a few
 hours of light usage (in my case that seems to be the best killer) with
 dmar disabled for just the gpu.
 
 So if you can give your machine some serious beating with the different
 options, that's really great.

Do you already have some preliminary results or has your machine not yet
died with this patch applied?

Thanks, Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Only free the unpin_work if cancelled before being run

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 10:29:38AM +0100, Chris Wilson wrote:
 The unpin worker frees it work struct and so during intel_crtc_disable
s/disable/destroy/

 we should only also free the work struct if cancel_work_sync() reports
 that it successfully cancelled the work prior to it being executed and
 thus avoid the double free.
 
 The impact is only for people unloading modules during a fullscreen game
 or movie playback, so extremely small.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch

 ---
  drivers/gpu/drm/i915/intel_display.c |4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 8298b72..78390e8 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -7602,10 +7602,8 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
   intel_crtc-unpin_work = NULL;
   spin_unlock_irqrestore(dev-event_lock, flags);
  
 - if (work) {
 - cancel_work_sync(work-work);
 + if (work  cancel_work_sync(work-work))
   kfree(work);
 - }
  
   drm_crtc_cleanup(crtc);
  
 -- 
 1.7.10
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Only free the unpin_work if cancelled before being run

2012-04-17 Thread Chris Wilson
On Tue, 17 Apr 2012 10:29:38 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:
 The unpin worker frees it work struct and so during intel_crtc_disable
 we should only also free the work struct if cancel_work_sync() reports
 that it successfully cancelled the work prior to it being executed and
 thus avoid the double free.
 
 The impact is only for people unloading modules during a fullscreen game
 or movie playback, so extremely small.

Futher review (hunting for some sign of workqueue corruption, cf
https://bugs.freedesktop.org/show_bug.cgi?id=48798) says that if work is
non-NULL here it will not have been scheduled so cancel_work_sync() will
always return true.

Which also means that we have no way of waiting upon the scheduled
unpin_work. :|
-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 0/5] refactor power management into intel_pm

2012-04-17 Thread Eugeni Dodonov
On Tue, Apr 17, 2012 at 05:57, Daniel Vetter dan...@ffwll.ch wrote:

 On Mon, Apr 16, 2012 at 10:20:33PM -0300, Eugeni Dodonov wrote:
  As Chris Wilson noticed, my previous patch that did the refactoring as
 one big
  patch which moved everything at once was extremely difficult to review
 and
  maintain. So I split the same refactoring into a series of smaller
 patches,
  which move one subsystem at a time.

 Yeah, diffs are not actually readable, so looks like we have to do this
 piece-wise.


Please, tell me that you meant 'diffs are *now* actually readable' with a
'not' being a typo instead of 'now' there by a chance...

Because otherwise I couldn't understand what you meant with piece-wise.
Should I move each function at a time? :)

I'll do the intel_pm_init (or intel_init_pm) and send it on top of this
series. It makes lots of sense and will avoid exposing all those
platform-specific stuff where we won't need it except for initialization.

-- 
Eugeni Dodonov
http://eugeni.dodonov.net/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/29] drm/i915: add Haswell DIP controls registers

2012-04-17 Thread Daniel Vetter
On Fri, Apr 13, 2012 at 05:08:39PM -0300, Eugeni Dodonov wrote:
 Haswell has different DIP control registers and offsets.
 
 Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com

I've read a bit through Bspec wrt dip writing and it looks like hsw is
rather differen from previous chips:
- with have a data reg for every type of dip
- the bits in the ctl reg moved around completely

... so I guess this patch and the follow-on one are pretty bogus. One
thing I've noticed is that intel_infoframe_index and intel_infoframe_flags
have way too generic names, they're only useful to frob the dip ctl reg on
pre-hsw afaics. I think we should rename them to i9xx_infoframe_ctl_inde
and _flags or something similar.

-Daniel
 ---
  drivers/gpu/drm/i915/i915_reg.h |   16 
  1 file changed, 16 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 05d98f2..8cc53fb 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -3488,6 +3488,22 @@
  #define VLV_TVIDEO_DIP_GCP(pipe) \
   _PIPE(pipe, VLV_VIDEO_DIP_GDCP_PAYLOAD_A, VLV_VIDEO_DIP_GDCP_PAYLOAD_B)
  
 +/* Haswell DIP controls */
 +#define HSW_VIDEO_DIP_CTL_A  0x60200
 +#define HSW_VIDEO_DIP_AVI_DATA_A 0x60220
 +#define HSW_VIDEO_DIP_GCP_A  0x60210
 +
 +#define HSW_VIDEO_DIP_CTL_B  0x61200
 +#define HSW_VIDEO_DIP_AVI_DATA_B 0x61220
 +#define HSW_VIDEO_DIP_GCP_B  0x61210
 +
 +#define HSW_TVIDEO_DIP_CTL(pipe) \
 +  _PIPE(pipe, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
 +#define HSW_TVIDEO_DIP_AVI_DATA(pipe) \
 +  _PIPE(pipe, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
 +#define HSW_TVIDEO_DIP_GCP(pipe) \
 + _PIPE(pipe, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
 +
  #define _TRANS_HTOTAL_B  0xe1000
  #define _TRANS_HBLANK_B  0xe1004
  #define _TRANS_HSYNC_B   0xe1008
 -- 
 1.7.10
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 13/29] drm/i915: show unknown sdvox registers on hdmi init

2012-04-17 Thread Daniel Vetter
On Fri, Apr 13, 2012 at 05:08:49PM -0300, Eugeni Dodonov wrote:
 Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com

This is pretty much a implementation bug, and with the new DDI split I'm
not too sure we'll need this. I'm ingoring this for now ;-)
-Daniel
 ---
  drivers/gpu/drm/i915/intel_hdmi.c |2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
 b/drivers/gpu/drm/i915/intel_hdmi.c
 index 700bd0b..0978fb7 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -614,6 +614,8 @@ void intel_hdmi_init(struct drm_device *dev, int 
 sdvox_reg)
   intel_encoder-clone_mask = (1  INTEL_HDMIF_CLONE_BIT);
   intel_hdmi-ddc_bus = GMBUS_PORT_DPD;
   dev_priv-hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
 + } else {
 + DRM_DEBUG_DRIVER(Unknown sdvox register %x\n, sdvox_reg);
   }
  
   intel_hdmi-sdvox_reg = sdvox_reg;
 -- 
 1.7.10
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 16/29] drm/i915: detect PCH encoders on Haswell

2012-04-17 Thread Daniel Vetter
On Fri, Apr 13, 2012 at 05:08:52PM -0300, Eugeni Dodonov wrote:
 On Haswell, the only PCH-connected output is the one driven by DDI E in
 FDI mode, used for VGA connection. All the others are handled by the CPU.
 
 Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c |7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index e4ebd39..b01afb0 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3098,6 +3098,13 @@ static bool intel_crtc_driving_pch(struct drm_crtc 
 *crtc)
   if (encoder-base.crtc != crtc)
   continue;
  
 + /* On Haswell, only DDI E can connect to PCH through FDI to 
 drive VGA */
 + if (IS_HASWELL(dev)  (encoder-type != DRM_MODE_ENCODER_DAC)) 
 {
 + DRM_DEBUG_KMS(Non-PCH encoder detected on Haswell. 
 Allowed: %d, detected: %d\n,
 + DRM_MODE_ENCODER_DAC, encoder-type);
 + return false;
 + }

Shouldn't we use a PCH_LTP check here, given that it looks like hsd can
be combined with ppt? Or is it really only DDI E that can be used for
fdi and hsw+ppt would only be able to drive 1 pch output?
-Daniel

 +
   switch (encoder-type) {
   case INTEL_OUTPUT_EDP:
   if (!intel_encoder_is_pch_edp(encoder-base))
 -- 
 1.7.10
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Only free the unpin_work if cancelled before being run

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 10:53:24AM +0100, Chris Wilson wrote:
 On Tue, 17 Apr 2012 10:29:38 +0100, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
  The unpin worker frees it work struct and so during intel_crtc_disable
  we should only also free the work struct if cancel_work_sync() reports
  that it successfully cancelled the work prior to it being executed and
  thus avoid the double free.
  
  The impact is only for people unloading modules during a fullscreen game
  or movie playback, so extremely small.
 
 Futher review (hunting for some sign of workqueue corruption, cf
 https://bugs.freedesktop.org/show_bug.cgi?id=48798) says that if work is
 non-NULL here it will not have been scheduled so cancel_work_sync() will
 always return true.

Well, I've failed to notice this while reviewing the unpin_work life-cycle
...

 Which also means that we have no way of waiting upon the scheduled
 unpin_work. :|

... but have noticed that we lose any reference from intel_crtc to the
unpin work once it's scheduled, and checked that we properly flush the
work queue: See the ordering of irq disable, flush workqueue, then crtc
destroy (in mode_config_cleanup).

We even have a flush_workqueu in i915_dma.c before tearing down gem, but
that won't work too well now that unpin also frobs around with the fbc
state (which is gone by now). But there's still the misleading comment
that this syncs up with unpin work.

Care to clean this up a bit by
- ditching the unnecessary flush_workqueu in i915_dma.c
- move the comment about syncing up with unpin_work to where we actually
  sync up
- and kill the superflous cancel_work?

Cheers, Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 11/29] drm/i915: share pipe count handling with Ivybridge

2012-04-17 Thread Chris Wilson
On Tue, 17 Apr 2012 12:19:16 +0200, Daniel Vetter dan...@ffwll.ch wrote:
 On Fri, Apr 13, 2012 at 05:08:47PM -0300, Eugeni Dodonov wrote:
  Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com
  Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c |2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 3d78686..5ee652d 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -2427,7 +2427,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int 
  y,
  case 1:
  break;
  case 2:
  -   if (IS_IVYBRIDGE(dev))
  +   if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
  break;
  /* fall through otherwise */
  default:
 
 Imo this code is a rather funky way to check for 3 plane support ... I
 think we should just replace this entire switch statement with a
 if(WARN_ON(intel_crtc-plane  dev_priv-num_pipes)) return -EINVAL;
 
 Or has there been another reason for this? Chris, git blame says you've
 originally added this in 5c3b82e2, any comments?

Yup, it's just a userspace (and internal consistency) validation check, so
if (pipe = dev_priv-num_pipes) return -EINVAL; would have sufficed.
-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 00/29] Haswell round 3

2012-04-17 Thread Daniel Vetter
On Fri, Apr 13, 2012 at 05:08:36PM -0300, Eugeni Dodonov wrote:
 Hi forks,
 
 Just in time for everyone's weekend, here comes the 3rd round of patches on
 Haswell.
 
 As major highlights, it also adds support for HDMI/DVI outputs and multi-head
 modes (I tried with VGA and HDMI).
 
 Other than that, it is the same patches with comments from the past round
 addressed (SBI locking support, proper WM calculation, better PCH items
 detection, and so on).
 
 Daniel, I think that the bits definitions, power wells, clocks programming, 
 and
 modesetting for both FDI and HDMI modes should be good to go unless someone
 spots additional issues with them - so please, all of you who have something 
 to
 say about them, say it now and bikeshed as you please :).

I've picked up a few more patches I couldn't find anything to bikeshed
about, the others grew some comments. I haven't looked too hard at the
actual modeset stuff given that you're already really busy reworking
things and moving stuff to intel_ddi.c

For the modeset stuff 2 general comments so you know from where the
bikeshed will hit you in the next review round:
- if possible, I'd like to hide most of the 'disable pch stuff on hsw'
  stuff behind crtc_driving_pch checks.
- I think we need to be careful about IS_HASWELL vs PCH_LPT checks,
  otherwise hsw+ppt will not work so great.

For the dip/infoframe stuff, maybe you can volunteer Paulo to help you
out, he's looking into this atm anyway. 

Cheers, Daniel

 
 Note that the DP and eDP modesetting support is not there yet - it will still
 require a considerable amount of patches.
 
 Also, there is one patch which fixes null pointer exceptions in gmbus code I
 was having with some of the drm-intel-next-queued iterations, but I don't 
 think
 it is necessary at the moment now that gmbus stuff was disabled again (patch
 5). I am not even sure if we'll hit those code paths with invalid values in
 real life, so I simple added some checks for cases when we don't have a valid
 adapter as it was looking too error-prone otherwise. Perhaps we could add a
 WARN into them as well.

 
 Eugeni Dodonov (29):
   drm/i915: add definition of LPT FDI port width registers
   drm/i915: add WRPLL divider programming bits
   drm/i915: add Haswell DIP controls registers
   drm/i915: support infoframes on Haswell
   drm/i915: prevent NULL pointer exception when using gmbus
   drm/i915: add support for SBI ops
   drm/i915: calculate same watermarks on Haswell as on Ivy Bridge
   drm/i915: share forcewaking code between IVB and HSW
   drm/i915: haswell has 3 pipes as well
   drm/i915: reuse Ivybridge interrupts code for Haswell
   drm/i915: share pipe count handling with Ivybridge
   drm/i915: share IVB cursor routine with Haswell
   drm/i915: show unknown sdvox registers on hdmi init
   drm/i915: do not use fdi_normal_train on haswell
   drm/i915: do not enable PCH PLL on pre-haswell
   drm/i915: detect PCH encoders on Haswell
   drm/i915: enable power wells on haswell init
   drm/i915: disable rc6 on haswell for now
   drm/i915: program WM_LINETIME on Haswell
   drm/i915: do not use old code paths on Haswell
   drm/i915: initialize DDI buffer translations
   drm/i915: perform Haswell DDI link training in FDI mode
   drm/i915: disable pipe DDI function when disabling pipe
   drm/i915: program iCLKIP on Lynx Point
   drm/i915: detect digital outputs on Haswell
   drm/i915: add support for DDI-controlled digital outputs
   drm/i915: add WR PLL programming table
   drm/i915: prepare HDMI link for Haswell
   drm/i915: hook Haswell devices in place
 
  drivers/char/agp/intel-agp.c |4 +
  drivers/gpu/drm/i915/i915_dma.c  |2 +-
  drivers/gpu/drm/i915/i915_drv.c  |7 +
  drivers/gpu/drm/i915/i915_irq.c  |6 +-
  drivers/gpu/drm/i915/i915_reg.h  |   23 +
  drivers/gpu/drm/i915/intel_display.c |  763 
 --
  drivers/gpu/drm/i915/intel_drv.h |1 +
  drivers/gpu/drm/i915/intel_hdmi.c|  602 ++-
  8 files changed, 1357 insertions(+), 51 deletions(-)
 
 -- 
 1.7.10
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 11/29] drm/i915: share pipe count handling with Ivybridge

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 11:36:33AM +0100, Chris Wilson wrote:
 On Tue, 17 Apr 2012 12:19:16 +0200, Daniel Vetter dan...@ffwll.ch wrote:
  On Fri, Apr 13, 2012 at 05:08:47PM -0300, Eugeni Dodonov wrote:
   Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com
   Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
   ---
drivers/gpu/drm/i915/intel_display.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_display.c 
   b/drivers/gpu/drm/i915/intel_display.c
   index 3d78686..5ee652d 100644
   --- a/drivers/gpu/drm/i915/intel_display.c
   +++ b/drivers/gpu/drm/i915/intel_display.c
   @@ -2427,7 +2427,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, 
   int y,
 case 1:
 break;
 case 2:
   - if (IS_IVYBRIDGE(dev))
   + if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
 break;
 /* fall through otherwise */
 default:
  
  Imo this code is a rather funky way to check for 3 plane support ... I
  think we should just replace this entire switch statement with a
  if(WARN_ON(intel_crtc-plane  dev_priv-num_pipes)) return -EINVAL;
  
  Or has there been another reason for this? Chris, git blame says you've
  originally added this in 5c3b82e2, any comments?
 
 Yup, it's just a userspace (and internal consistency) validation check, so
 if (pipe = dev_priv-num_pipes) return -EINVAL; would have sufficed.

Hm, how can userspace trigger this? It can only pass in crtc ids, which
the drm core validates. intel_crtc-plane is completely in our control,
hence why I think this can only be a driver bug. Or do I miss something?
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 11/29] drm/i915: share pipe count handling with Ivybridge

2012-04-17 Thread Chris Wilson
On Tue, 17 Apr 2012 13:26:19 +0200, Daniel Vetter dan...@ffwll.ch wrote:
 On Tue, Apr 17, 2012 at 11:36:33AM +0100, Chris Wilson wrote:
  On Tue, 17 Apr 2012 12:19:16 +0200, Daniel Vetter dan...@ffwll.ch wrote:
   On Fri, Apr 13, 2012 at 05:08:47PM -0300, Eugeni Dodonov wrote:
Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3d78686..5ee652d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2427,7 +2427,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, 
int y,
case 1:
break;
case 2:
-   if (IS_IVYBRIDGE(dev))
+   if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
break;
/* fall through otherwise */
default:
   
   Imo this code is a rather funky way to check for 3 plane support ... I
   think we should just replace this entire switch statement with a
   if(WARN_ON(intel_crtc-plane  dev_priv-num_pipes)) return -EINVAL;
   
   Or has there been another reason for this? Chris, git blame says you've
   originally added this in 5c3b82e2, any comments?
  
  Yup, it's just a userspace (and internal consistency) validation check, so
  if (pipe = dev_priv-num_pipes) return -EINVAL; would have sufficed.
 
 Hm, how can userspace trigger this? It can only pass in crtc ids, which
 the drm core validates. intel_crtc-plane is completely in our control,
 hence why I think this can only be a driver bug. Or do I miss something?

Shooting the messanger here. :-p

Right, userspace cannot assign pipes, that is purely an internal detail.
So this can be promoted to a if (WARN(pipe = num_pipes)) return -EINVAL;
or killed outright.

All I did in that commit was perform the existing consistency check
upfront, and return -EINVAL alongside the DRM_ERROR.
-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 00/29] Haswell round 3

2012-04-17 Thread Eugeni Dodonov
On Tue, Apr 17, 2012 at 07:49, Daniel Vetter dan...@ffwll.ch wrote:

 On Fri, Apr 13, 2012 at 05:08:36PM -0300, Eugeni Dodonov wrote:
  Hi forks,
 
  Just in time for everyone's weekend, here comes the 3rd round of patches
 on
  Haswell.
 
  As major highlights, it also adds support for HDMI/DVI outputs and
 multi-head
  modes (I tried with VGA and HDMI).
 
  Other than that, it is the same patches with comments from the past round
  addressed (SBI locking support, proper WM calculation, better PCH items
  detection, and so on).
 
  Daniel, I think that the bits definitions, power wells, clocks
 programming, and
  modesetting for both FDI and HDMI modes should be good to go unless
 someone
  spots additional issues with them - so please, all of you who have
 something to
  say about them, say it now and bikeshed as you please :).

 I've picked up a few more patches I couldn't find anything to bikeshed
 about, the others grew some comments. I haven't looked too hard at the
 actual modeset stuff given that you're already really busy reworking
 things and moving stuff to intel_ddi.c


...and into intel_pm.c as well when those patches land :).

At least the power wells-related part is much more welcome there.

So I'll address the bikeshedding^w comments on the remaining patches and
will resend them when intel_pm.c stuff lands.

-- 
Eugeni Dodonov
 http://eugeni.dodonov.net/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/29] drm/i915: add Haswell DIP controls registers

2012-04-17 Thread Eugeni Dodonov
On Tue, Apr 17, 2012 at 07:12, Daniel Vetter dan...@ffwll.ch wrote:

 On Fri, Apr 13, 2012 at 05:08:39PM -0300, Eugeni Dodonov wrote:
  Haswell has different DIP control registers and offsets.
 
  Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com

 I've read a bit through Bspec wrt dip writing and it looks like hsw is
 rather differen from previous chips:
 - with have a data reg for every type of dip
 - the bits in the ctl reg moved around completely

 ... so I guess this patch and the follow-on one are pretty bogus. One
 thing I've noticed is that intel_infoframe_index and intel_infoframe_flags
 have way too generic names, they're only useful to frob the dip ctl reg on
 pre-hsw afaics. I think we should rename them to i9xx_infoframe_ctl_inde
 and _flags or something similar.


Yep, I only did the minimally-required stuff to have HDMI output, so the
ones willing to use Haswell could have anything on their screen besides 'No
Signal' :).

But yes, those patches should receive more care for full hdmi and DIP
support going forward.

-- 
Eugeni Dodonov
http://eugeni.dodonov.net/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Only free the unpin_work if cancelled before being run

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 03:44:01PM +0200, Daniel Vetter wrote:
 On Tue, Apr 17, 2012 at 02:30:35PM +0100, Chris Wilson wrote:
  On Tue, 17 Apr 2012 12:33:46 +0200, Daniel Vetter dan...@ffwll.ch wrote:
   On Tue, Apr 17, 2012 at 10:53:24AM +0100, Chris Wilson wrote:
On Tue, 17 Apr 2012 10:29:38 +0100, Chris Wilson 
ch...@chris-wilson.co.uk wrote:
 The unpin worker frees it work struct and so during intel_crtc_disable
 we should only also free the work struct if cancel_work_sync() reports
 that it successfully cancelled the work prior to it being executed and
 thus avoid the double free.
 
 The impact is only for people unloading modules during a fullscreen 
 game
 or movie playback, so extremely small.

Futher review (hunting for some sign of workqueue corruption, cf
https://bugs.freedesktop.org/show_bug.cgi?id=48798) says that if work is
non-NULL here it will not have been scheduled so cancel_work_sync() will
always return true.
   
   Well, I've failed to notice this while reviewing the unpin_work life-cycle
   ...
   
Which also means that we have no way of waiting upon the scheduled
unpin_work. :|
   
   ... but have noticed that we lose any reference from intel_crtc to the
   unpin work once it's scheduled, and checked that we properly flush the
   work queue: See the ordering of irq disable, flush workqueue, then crtc
   destroy (in mode_config_cleanup).
   
   We even have a flush_workqueu in i915_dma.c before tearing down gem, but
   that won't work too well now that unpin also frobs around with the fbc
   state (which is gone by now). But there's still the misleading comment
   that this syncs up with unpin work.
   
   Care to clean this up a bit by
   - ditching the unnecessary flush_workqueu in i915_dma.c
  Indeed looks superfluous. If we consider the unlikelihood the cleanup
  code being well-tested, I'd prefer that we did do something like
  drain_workqueue() as the first step in unload().
 
 Sounds good.
 
   - move the comment about syncing up with unpin_work to where we actually
 sync up
  
  I'm not finding another point where we explicitly sync with outstanding
  unpin work. Probably due to the comment being in the wrong place...
 
 Well, I've just noticed that we call flush_scheduled_work instead of
 flush_workqueue (i.e. the global one instead of our own), but we also put
 the unpin work onto the global queue with schedule_work instead of our own
 with queue_work
 
   - and kill the superflous cancel_work?
  Which also reminds me that the unpin_work holds onto a few references
  that need to be released as well as the kfree. See above about these code
  paths being relatively untrod.
 
 Oops. The problem is that unpin_work also calls the fbc update, so we need
 to ensure that this is done before the fbc cleanup happened. Fun.

While we bitch around about this code: unpin_work is a bit misleading
given the fbc frobbing. So maybe we should call it finish_pageflip_work or
something like that ...
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/1] drm/i915: add generic power management initialization

2012-04-17 Thread Eugeni Dodonov
This adds intel_pm routine for generic power-related infrastructure
initialization.

Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |  125 +-
 drivers/gpu/drm/i915/intel_drv.h |   45 +--
 drivers/gpu/drm/i915/intel_pm.c  |  142 ++
 3 files changed, 145 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6649301..b08bccd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6369,23 +6369,6 @@ static void intel_init_display(struct drm_device *dev)
dev_priv-display.update_plane = i9xx_update_plane;
}
 
-   if (I915_HAS_FBC(dev)) {
-   if (HAS_PCH_SPLIT(dev)) {
-   dev_priv-display.fbc_enabled = ironlake_fbc_enabled;
-   dev_priv-display.enable_fbc = ironlake_enable_fbc;
-   dev_priv-display.disable_fbc = ironlake_disable_fbc;
-   } else if (IS_GM45(dev)) {
-   dev_priv-display.fbc_enabled = g4x_fbc_enabled;
-   dev_priv-display.enable_fbc = g4x_enable_fbc;
-   dev_priv-display.disable_fbc = g4x_disable_fbc;
-   } else if (IS_CRESTLINE(dev)) {
-   dev_priv-display.fbc_enabled = i8xx_fbc_enabled;
-   dev_priv-display.enable_fbc = i8xx_enable_fbc;
-   dev_priv-display.disable_fbc = i8xx_disable_fbc;
-   }
-   /* 855GM needs testing */
-   }
-
/* Returns the core display clock speed */
if (IS_VALLEYVIEW(dev))
dev_priv-display.get_display_clock_speed =
@@ -6412,130 +6395,24 @@ static void intel_init_display(struct drm_device *dev)
dev_priv-display.get_display_clock_speed =
i830_get_display_clock_speed;
 
-   /* For FIFO watermark updates */
if (HAS_PCH_SPLIT(dev)) {
-   dev_priv-display.force_wake_get = __gen6_gt_force_wake_get;
-   dev_priv-display.force_wake_put = __gen6_gt_force_wake_put;
-
-   /* IVB configs may use multi-threaded forcewake */
-   if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-   u32 ecobus;
-
-   /* A small trick here - if the bios hasn't configured 
MT forcewake,
-* and if the device is in RC6, then force_wake_mt_get 
will not wake
-* the device and the ECOBUS read will return zero. 
Which will be
-* (correctly) interpreted by the test below as MT 
forcewake being
-* disabled.
-*/
-   mutex_lock(dev-struct_mutex);
-   __gen6_gt_force_wake_mt_get(dev_priv);
-   ecobus = I915_READ_NOTRACE(ECOBUS);
-   __gen6_gt_force_wake_mt_put(dev_priv);
-   mutex_unlock(dev-struct_mutex);
-
-   if (ecobus  FORCEWAKE_MT_ENABLE) {
-   DRM_DEBUG_KMS(Using MT version of 
forcewake\n);
-   dev_priv-display.force_wake_get =
-   __gen6_gt_force_wake_mt_get;
-   dev_priv-display.force_wake_put =
-   __gen6_gt_force_wake_mt_put;
-   }
-   }
-
-   if (HAS_PCH_IBX(dev))
-   dev_priv-display.init_pch_clock_gating = 
ibx_init_clock_gating;
-   else if (HAS_PCH_CPT(dev))
-   dev_priv-display.init_pch_clock_gating = 
cpt_init_clock_gating;
-
if (IS_GEN5(dev)) {
-   if (I915_READ(MLTR_ILK)  ILK_SRLT_MASK)
-   dev_priv-display.update_wm = 
ironlake_update_wm;
-   else {
-   DRM_DEBUG_KMS(Failed to get proper latency. 
- Disable CxSR\n);
-   dev_priv-display.update_wm = NULL;
-   }
dev_priv-display.fdi_link_train = 
ironlake_fdi_link_train;
-   dev_priv-display.init_clock_gating = 
ironlake_init_clock_gating;
dev_priv-display.write_eld = ironlake_write_eld;
} else if (IS_GEN6(dev)) {
-   if (SNB_READ_WM0_LATENCY()) {
-   dev_priv-display.update_wm = 
sandybridge_update_wm;
-   dev_priv-display.update_sprite_wm = 
sandybridge_update_sprite_wm;
-   } else {
-   DRM_DEBUG_KMS(Failed to read display plane 
latency. 
- 

[Intel-gfx] [PATCH] drm/i915: intel_update_fbc() requires struct_mutex, so no longer atomic

2012-04-17 Thread Chris Wilson
As we need to manipulate our device structure and allocate queue a task,
it is no longer a simple atomic operation and cannot be performed along
the atomic modeset paths. Instead make sure that we disable FBC (which
must be therefore kept as a set of simple register writes) when
performing the atomic modeset and leave the heavy-weight
intel_update_fbc() for the normal modeset.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_display.c |   15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 39c13e3..1a0483f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2330,15 +2330,11 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, 
struct drm_framebuffer *fb,
 {
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   int ret;
-
-   ret = dev_priv-display.update_plane(crtc, fb, x, y);
-   if (ret)
-   return ret;
 
-   intel_update_fbc(dev);
+   if (dev_priv-display.disable_fbc)
+   dev_priv-display.disable_fbc(dev);
 
-   return 0;
+   return dev_priv-display.update_plane(crtc, fb, x, y);
 }
 
 static int
@@ -2373,6 +2369,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb)
 {
struct drm_device *dev = crtc-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_i915_master_private *master_priv;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
@@ -2409,8 +2406,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
if (old_fb)
intel_finish_fb(old_fb);
 
-   ret = intel_pipe_set_base_atomic(crtc, crtc-fb, x, y,
-LEAVE_ATOMIC_MODE_SET);
+   ret = dev_priv-display.update_plane(crtc, crtc-fb, x, y);
if (ret) {
intel_unpin_fb_obj(to_intel_framebuffer(crtc-fb)-obj);
mutex_unlock(dev-struct_mutex);
@@ -2423,6 +2419,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
intel_unpin_fb_obj(to_intel_framebuffer(old_fb)-obj);
}
 
+   intel_update_fbc(dev);
mutex_unlock(dev-struct_mutex);
 
intel_increase_pllclock(crtc);
-- 
1.7.10

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


[Intel-gfx] Kill unused fence pipelining

2012-04-17 Thread Chris Wilson
Since we have never succeeded in making the pipelined fence updates
stable and it appears we never will, delete the dead code.
-Chris

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


[Intel-gfx] [PATCH 06/12] drm/i915: Remove the unsightly optimisation from flush_fence()

2012-04-17 Thread Chris Wilson
As i915_wait_request() will first check for an already passed seqno,
doing it also in the caller is a waste of space for a cold path.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |   18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f7cd3461..bac3570 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2280,11 +2280,6 @@ static int i830_write_fence_reg(struct 
drm_i915_gem_object *obj)
return 0;
 }
 
-static bool ring_passed_seqno(struct intel_ring_buffer *ring, u32 seqno)
-{
-   return i915_seqno_passed(ring-get_seqno(ring), seqno);
-}
-
 static int
 i915_gem_object_flush_fence(struct drm_i915_gem_object *obj)
 {
@@ -2302,14 +2297,11 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object 
*obj)
}
 
if (obj-last_fenced_seqno) {
-   if (!ring_passed_seqno(obj-ring,
-  obj-last_fenced_seqno)) {
-   ret = i915_wait_request(obj-ring,
-   obj-last_fenced_seqno,
-   true);
-   if (ret)
-   return ret;
-   }
+   ret = i915_wait_request(obj-ring,
+   obj-last_fenced_seqno,
+   true);
+   if (ret)
+   return ret;
 
obj-last_fenced_seqno = 0;
}
-- 
1.7.10

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


[Intel-gfx] [PATCH 08/12] drm/i915: Refactor put_fence() to use the common fence writing routine

2012-04-17 Thread Chris Wilson
One clarification that we make is to the existing semantics of
obj-tiling_changed to only mean that we need to update an associated
fence register (including the NO_FENCE when executing an untiled but
fenced GPU command). If we do not have a fence register or pending
fenced GPU access for the object (after put_fence() for example), then
we can clear the tiling_changed flag as any fence will necessarily be
rewritten upon acquisition.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |   62 ---
 1 file changed, 51 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 199306d..3601b8b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -50,10 +50,28 @@ static int i915_gem_phys_pwrite(struct drm_device *dev,
struct drm_file *file);
 static void i915_gem_free_object_tail(struct drm_i915_gem_object *obj);
 
+static void i915_gem_write_fence(struct drm_device *dev, int reg,
+struct drm_i915_gem_object *obj);
+static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
+struct drm_i915_fence_reg *fence,
+bool enable);
+
 static int i915_gem_inactive_shrink(struct shrinker *shrinker,
struct shrink_control *sc);
 static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
 
+static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj)
+{
+   if (obj-tiling_mode)
+   i915_gem_release_mmap(obj);
+
+   /* As we do not have an associated fence register, we will force
+* a tiling change if we ever need to acquire one.
+*/
+   obj-tiling_changed = false;
+   obj-fence_reg = I915_FENCE_REG_NONE;
+}
+
 /* some bookkeeping */
 static void i915_gem_info_add_obj(struct drm_i915_private *dev_priv,
  size_t size)
@@ -2301,6 +2319,32 @@ static void i915_gem_write_fence(struct drm_device *dev, 
int reg,
}
 }
 
+static inline int fence_number(struct drm_i915_private *dev_priv,
+  struct drm_i915_fence_reg *fence)
+{
+   return fence - dev_priv-fence_regs;
+}
+
+static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
+struct drm_i915_fence_reg *fence,
+bool enable)
+{
+   struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
+   int reg = fence_number(dev_priv, fence);
+
+   i915_gem_write_fence(obj-base.dev, reg, enable ? obj : NULL);
+
+   if (enable) {
+   obj-fence_reg = reg;
+   fence-obj = obj;
+   list_move_tail(fence-lru_list, dev_priv-mm.fence_list);
+   } else {
+   obj-fence_reg = I915_FENCE_REG_NONE;
+   fence-obj = NULL;
+   list_del_init(fence-lru_list);
+   }
+}
+
 static int
 i915_gem_object_flush_fence(struct drm_i915_gem_object *obj)
 {
@@ -2339,24 +2383,20 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object 
*obj)
 int
 i915_gem_object_put_fence(struct drm_i915_gem_object *obj)
 {
+   struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
int ret;
 
-   if (obj-tiling_mode)
-   i915_gem_release_mmap(obj);
-
ret = i915_gem_object_flush_fence(obj);
if (ret)
return ret;
 
-   if (obj-fence_reg != I915_FENCE_REG_NONE) {
-   struct drm_i915_private *dev_priv = obj-base.dev-dev_private;
-
-   WARN_ON(dev_priv-fence_regs[obj-fence_reg].pin_count);
-   i915_gem_clear_fence_reg(obj-base.dev,
-dev_priv-fence_regs[obj-fence_reg]);
+   if (obj-fence_reg == I915_FENCE_REG_NONE)
+   return 0;
 
-   obj-fence_reg = I915_FENCE_REG_NONE;
-   }
+   i915_gem_object_update_fence(obj,
+dev_priv-fence_regs[obj-fence_reg],
+false);
+   i915_gem_object_fence_lost(obj);
 
return 0;
 }
-- 
1.7.10

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


[Intel-gfx] [PATCH 11/12] drm/i915: Clarify the semantics of tiling-changed by renaming it

2012-04-17 Thread Chris Wilson
Rename obj-tiling_changed to obj-fence_change so that it is clear that
it flags when the parameters for an active fence (including the
no-fence) register are changed.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_drv.h|7 ++-
 drivers/gpu/drm/i915/i915_gem.c|8 
 drivers/gpu/drm/i915/i915_gem_tiling.c |5 -
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c32ada..3d7ad9b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -869,7 +869,12 @@ struct drm_i915_gem_object {
 * Current tiling mode for the object.
 */
unsigned int tiling_mode:2;
-   unsigned int tiling_changed:1;
+   /**
+* Whether the tiling parameters for the currently associated fence
+* register have changed. Note that for the purposes of tracking
+* tiling changes we also treat the unfenced register as a fence.
+*/
+   unsigned int fence_changed:1;
 
/** How many users have pinned this object in GTT space. The following
 * users can each hold at most one reference: pwrite/pread, pin_ioctl
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7bc4a40..5edab3f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -66,7 +66,7 @@ static inline void i915_gem_object_fence_lost(struct 
drm_i915_gem_object *obj)
/* As we do not have an associated fence register, we will force
 * a tiling change if we ever need to acquire one.
 */
-   obj-tiling_changed = false;
+   obj-fence_changed = false;
obj-fence_reg = I915_FENCE_REG_NONE;
 }
 
@@ -2456,7 +2456,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
/* Have we updated the tiling parameters upon the object and so
 * will need to serialise the write to the associated fence register?
 */
-   if (obj-tiling_changed) {
+   if (obj-fence_changed) {
ret = i915_gem_object_flush_fence(obj);
if (ret)
return ret;
@@ -2465,7 +2465,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
/* Just update our place in the LRU if our fence is getting reused. */
if (obj-fence_reg != I915_FENCE_REG_NONE) {
reg = dev_priv-fence_regs[obj-fence_reg];
-   if (!obj-tiling_changed) {
+   if (!obj-fence_changed) {
list_move_tail(reg-lru_list,
   dev_priv-mm.fence_list);
return 0;
@@ -2488,7 +2488,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
return 0;
 
i915_gem_object_update_fence(obj, reg, enable);
-   obj-tiling_changed = false;
+   obj-fence_changed = false;
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 1a93066..e51d892 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -374,7 +374,10 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
}
 
if (ret == 0) {
-   obj-tiling_changed = true;
+   obj-fence_changed =
+   obj-fenced_gpu_access ||
+   obj-fence_reg != I915_FENCE_REG_NONE;
+
obj-tiling_mode = args-tiling_mode;
obj-stride = args-stride;
}
-- 
1.7.10

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


[Intel-gfx] [PATCH 09/12] drm/i915: Refactor fence clearing to use the common fence writing routine

2012-04-17 Thread Chris Wilson
Now that we have a routine that is able to clear the fences as well as
setup up the register for a tiled object, remove the surplus routines to
clear the fences.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |   62 ++-
 1 file changed, 9 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3601b8b..e09ac3a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -42,8 +42,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct 
drm_i915_gem_object *o
 static __must_check int i915_gem_object_bind_to_gtt(struct drm_i915_gem_object 
*obj,
unsigned alignment,
bool map_and_fenceable);
-static void i915_gem_clear_fence_reg(struct drm_device *dev,
-struct drm_i915_fence_reg *reg);
 static int i915_gem_phys_pwrite(struct drm_device *dev,
struct drm_i915_gem_object *obj,
struct drm_i915_gem_pwrite *args,
@@ -1655,19 +1653,18 @@ static void i915_gem_reset_fences(struct drm_device 
*dev)
 
for (i = 0; i  dev_priv-num_fence_regs; i++) {
struct drm_i915_fence_reg *reg = dev_priv-fence_regs[i];
-   struct drm_i915_gem_object *obj = reg-obj;
 
-   if (!obj)
-   continue;
+   i915_gem_write_fence(dev, i, NULL);
 
-   if (obj-tiling_mode)
-   i915_gem_release_mmap(obj);
+   if (reg-obj)
+   i915_gem_object_fence_lost(reg-obj);
 
-   reg-obj-fence_reg = I915_FENCE_REG_NONE;
-   reg-obj-fenced_gpu_access = false;
-   reg-obj-last_fenced_seqno = 0;
-   i915_gem_clear_fence_reg(dev, reg);
+   reg-pin_count = 0;
+   reg-obj = NULL;
+   INIT_LIST_HEAD(reg-lru_list);
}
+
+   INIT_LIST_HEAD(dev_priv-mm.fence_list);
 }
 
 void i915_gem_reset(struct drm_device *dev)
@@ -2513,45 +2510,6 @@ update:
 }
 
 /**
- * i915_gem_clear_fence_reg - clear out fence register info
- * @obj: object to clear
- *
- * Zeroes out the fence register itself and clears out the associated
- * data structures in dev_priv and obj.
- */
-static void
-i915_gem_clear_fence_reg(struct drm_device *dev,
-struct drm_i915_fence_reg *reg)
-{
-   drm_i915_private_t *dev_priv = dev-dev_private;
-   uint32_t fence_reg = reg - dev_priv-fence_regs;
-
-   switch (INTEL_INFO(dev)-gen) {
-   case 7:
-   case 6:
-   I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + fence_reg*8, 0);
-   break;
-   case 5:
-   case 4:
-   I915_WRITE64(FENCE_REG_965_0 + fence_reg*8, 0);
-   break;
-   case 3:
-   if (fence_reg = 8)
-   fence_reg = FENCE_REG_945_8 + (fence_reg - 8) * 4;
-   else
-   case 2:
-   fence_reg = FENCE_REG_830_0 + fence_reg * 4;
-
-   I915_WRITE(fence_reg, 0);
-   break;
-   }
-
-   list_del_init(reg-lru_list);
-   reg-obj = NULL;
-   reg-pin_count = 0;
-}
-
-/**
  * Finds free space in the GTT aperture and binds the object there.
  */
 static int
@@ -3788,9 +3746,7 @@ i915_gem_load(struct drm_device *dev)
dev_priv-num_fence_regs = 8;
 
/* Initialize fence registers to zero */
-   for (i = 0; i  dev_priv-num_fence_regs; i++) {
-   i915_gem_clear_fence_reg(dev, dev_priv-fence_regs[i]);
-   }
+   i915_gem_reset_fences(dev);
 
i915_gem_detect_bit_6_swizzle(dev);
init_waitqueue_head(dev_priv-pending_flip_queue);
-- 
1.7.10

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


[Intel-gfx] [PATCH 07/12] drm/i915: Prepare to consolidate fence writing

2012-04-17 Thread Chris Wilson
Update the existing architecture specific fence writing routines to
either update the fence to point to a tiled object or to clear them in
preparation to remove the other fence writing routes.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |  211 ---
 1 file changed, 108 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bac3570..199306d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2163,121 +2163,142 @@ int i915_gpu_idle(struct drm_device *dev, bool 
do_retire)
return 0;
 }
 
-static int sandybridge_write_fence_reg(struct drm_i915_gem_object *obj)
+static void sandybridge_write_fence_reg(struct drm_device *dev, int reg,
+   struct drm_i915_gem_object *obj)
 {
-   struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
-   u32 size = obj-gtt_space-size;
-   int regnum = obj-fence_reg;
uint64_t val;
 
-   val = (uint64_t)((obj-gtt_offset + size - 4096) 
-0xf000)  32;
-   val |= obj-gtt_offset  0xf000;
-   val |= (uint64_t)((obj-stride / 128) - 1) 
-   SANDYBRIDGE_FENCE_PITCH_SHIFT;
+   if (obj) {
+   u32 size = obj-gtt_space-size;
 
-   if (obj-tiling_mode == I915_TILING_Y)
-   val |= 1  I965_FENCE_TILING_Y_SHIFT;
-   val |= I965_FENCE_REG_VALID;
+   val = (uint64_t)((obj-gtt_offset + size - 4096) 
+0xf000)  32;
+   val |= obj-gtt_offset  0xf000;
+   val |= (uint64_t)((obj-stride / 128) - 1) 
+   SANDYBRIDGE_FENCE_PITCH_SHIFT;
 
-   I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + regnum * 8, val);
+   if (obj-tiling_mode == I915_TILING_Y)
+   val |= 1  I965_FENCE_TILING_Y_SHIFT;
+   val |= I965_FENCE_REG_VALID;
+   } else
+   val = 0;
 
-   return 0;
+   I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + reg * 8, val);
+   POSTING_READ(FENCE_REG_SANDYBRIDGE_0 + reg * 8);
 }
 
-static int i965_write_fence_reg(struct drm_i915_gem_object *obj)
+static void i965_write_fence_reg(struct drm_device *dev, int reg,
+struct drm_i915_gem_object *obj)
 {
-   struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
-   u32 size = obj-gtt_space-size;
-   int regnum = obj-fence_reg;
uint64_t val;
 
-   val = (uint64_t)((obj-gtt_offset + size - 4096) 
-   0xf000)  32;
-   val |= obj-gtt_offset  0xf000;
-   val |= ((obj-stride / 128) - 1)  I965_FENCE_PITCH_SHIFT;
-   if (obj-tiling_mode == I915_TILING_Y)
-   val |= 1  I965_FENCE_TILING_Y_SHIFT;
-   val |= I965_FENCE_REG_VALID;
+   if (obj) {
+   u32 size = obj-gtt_space-size;
 
-   I915_WRITE64(FENCE_REG_965_0 + regnum * 8, val);
+   val = (uint64_t)((obj-gtt_offset + size - 4096) 
+0xf000)  32;
+   val |= obj-gtt_offset  0xf000;
+   val |= ((obj-stride / 128) - 1)  I965_FENCE_PITCH_SHIFT;
+   if (obj-tiling_mode == I915_TILING_Y)
+   val |= 1  I965_FENCE_TILING_Y_SHIFT;
+   val |= I965_FENCE_REG_VALID;
+   } else
+   val = 0;
 
-   return 0;
+   I915_WRITE64(FENCE_REG_965_0 + reg * 8, val);
+   POSTING_READ(FENCE_REG_965_0 + reg * 8);
 }
 
-static int i915_write_fence_reg(struct drm_i915_gem_object *obj)
+static void i915_write_fence_reg(struct drm_device *dev, int reg,
+struct drm_i915_gem_object *obj)
 {
-   struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
-   u32 size = obj-gtt_space-size;
-   u32 fence_reg, val, pitch_val;
-   int tile_width;
-
-   if (WARN((obj-gtt_offset  ~I915_FENCE_START_MASK) ||
-(size  -size) != size ||
-(obj-gtt_offset  (size - 1)),
-object 0x%08x [fenceable? %d] not 1M or pot-size (0x%08x) 
aligned\n,
-obj-gtt_offset, obj-map_and_fenceable, size))
-   return -EINVAL;
+   u32 val;
 
-   if (obj-tiling_mode == I915_TILING_Y  HAS_128_BYTE_Y_TILING(dev))
-   tile_width = 128;
-   else
-   tile_width = 512;
-
-   /* Note: pitch better be a power of two tile widths */
-   pitch_val = obj-stride / tile_width;
-   pitch_val = ffs(pitch_val) - 1;
-
-   val = obj-gtt_offset;
-   if (obj-tiling_mode == I915_TILING_Y)
-   val |= 1  I830_FENCE_TILING_Y_SHIFT;
-   val |= I915_FENCE_SIZE_BITS(size);
-   val |= pitch_val  I830_FENCE_PITCH_SHIFT;
-   val |= I830_FENCE_REG_VALID;

[Intel-gfx] [PATCH 05/12] drm/i915: Simplify fence finding

2012-04-17 Thread Chris Wilson
As the fences are stored in LRU order, we can simply reuse the oldest if
we do not have an unused register.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |   16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b25d229..f7cd3461 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2352,7 +2352,7 @@ static struct drm_i915_fence_reg *
 i915_find_fence_reg(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-   struct drm_i915_fence_reg *reg, *first, *avail;
+   struct drm_i915_fence_reg *reg, *avail;
int i;
 
/* First try to find a free reg */
@@ -2370,24 +2370,14 @@ i915_find_fence_reg(struct drm_device *dev)
return NULL;
 
/* None available, try to steal one or wait for a user to finish */
-   avail = first = NULL;
list_for_each_entry(reg, dev_priv-mm.fence_list, lru_list) {
if (reg-pin_count)
continue;
 
-   if (first == NULL)
-   first = reg;
-
-   if (reg-obj-last_fenced_seqno == 0) {
-   avail = reg;
-   break;
-   }
+   return reg;
}
 
-   if (avail == NULL)
-   avail = first;
-
-   return avail;
+   return NULL;
 }
 
 /**
-- 
1.7.10

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


[Intel-gfx] [PATCH 12/12] drm/i915: Only the zap the VMA after updating the tiling parameters

2012-04-17 Thread Chris Wilson
If we fail to unbind and so abort the change in tiling, we will have
removed the VMA for the object for no reason. The likelihood of unbind
failing is slim (other than ERESTARTSYS which will cause userspace to
try again), so the change is mostly for the principle.

Also improve the slightly stale comment.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem_tiling.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index e51d892..6a2bd54 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -354,9 +354,10 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
/* We need to rebind the object if its current allocation
 * no longer meets the alignment restrictions for its new
 * tiling mode. Otherwise we can just leave it alone, but
-* need to ensure that any fence register is cleared.
+* need to ensure that any fence register is updated before
+* the next fenced (either through the GTT or by the BLT unit
+* on older GPUs) access.
 */
-   i915_gem_release_mmap(obj);
 
obj-map_and_fenceable =
obj-gtt_space == NULL ||
@@ -380,6 +381,9 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 
obj-tiling_mode = args-tiling_mode;
obj-stride = args-stride;
+
+   /* Force the fence to be reacquired for GTT access */
+   i915_gem_release_mmap(obj);
}
}
/* we have to maintain this existing ABI... */
-- 
1.7.10

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


[Intel-gfx] [PATCH 10/12] drm/i915: Refactor get_fence() to use the common fence writing routine

2012-04-17 Thread Chris Wilson
We can also take advantage of the new 'no retire' mode for seqno waiting
to avoid having to take a reference on the old fence object whilst
flushing an existing fence.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |   70 +++
 1 file changed, 27 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e09ac3a..7bc4a40 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2361,7 +2361,7 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object 
*obj)
if (obj-last_fenced_seqno) {
ret = i915_wait_request(obj-ring,
obj-last_fenced_seqno,
-   true);
+   false);
if (ret)
return ret;
 
@@ -2449,63 +2449,47 @@ i915_gem_object_get_fence(struct drm_i915_gem_object 
*obj)
 {
struct drm_device *dev = obj-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   bool enable = obj-tiling_mode != I915_TILING_NONE;
struct drm_i915_fence_reg *reg;
int ret;
 
-   if (obj-tiling_mode == I915_TILING_NONE)
-   return i915_gem_object_put_fence(obj);
+   /* Have we updated the tiling parameters upon the object and so
+* will need to serialise the write to the associated fence register?
+*/
+   if (obj-tiling_changed) {
+   ret = i915_gem_object_flush_fence(obj);
+   if (ret)
+   return ret;
+   }
 
/* Just update our place in the LRU if our fence is getting reused. */
if (obj-fence_reg != I915_FENCE_REG_NONE) {
reg = dev_priv-fence_regs[obj-fence_reg];
-   list_move_tail(reg-lru_list, dev_priv-mm.fence_list);
+   if (!obj-tiling_changed) {
+   list_move_tail(reg-lru_list,
+  dev_priv-mm.fence_list);
+   return 0;
+   }
+   } else if (enable) {
+   reg = i915_find_fence_reg(dev);
+   if (reg == NULL)
+   return -EDEADLK;
+
+   if (reg-obj) {
+   struct drm_i915_gem_object *old = reg-obj;
 
-   if (obj-tiling_changed) {
-   ret = i915_gem_object_flush_fence(obj);
+   ret = i915_gem_object_flush_fence(old);
if (ret)
return ret;
 
-   goto update;
+   i915_gem_object_fence_lost(old);
}
-
+   } else
return 0;
-   }
-
-   reg = i915_find_fence_reg(dev);
-   if (reg == NULL)
-   return -EDEADLK;
-
-   ret = i915_gem_object_flush_fence(obj);
-   if (ret)
-   return ret;
-
-   if (reg-obj) {
-   struct drm_i915_gem_object *old = reg-obj;
 
-   drm_gem_object_reference(old-base);
-
-   if (old-tiling_mode)
-   i915_gem_release_mmap(old);
-
-   ret = i915_gem_object_flush_fence(old);
-   if (ret) {
-   drm_gem_object_unreference(old-base);
-   return ret;
-   }
-
-   old-fence_reg = I915_FENCE_REG_NONE;
-   old-last_fenced_seqno = 0;
-
-   drm_gem_object_unreference(old-base);
-   }
-
-   reg-obj = obj;
-   list_move_tail(reg-lru_list, dev_priv-mm.fence_list);
-   obj-fence_reg = reg - dev_priv-fence_regs;
-
-update:
+   i915_gem_object_update_fence(obj, reg, enable);
obj-tiling_changed = false;
-   i915_gem_write_fence(dev, reg - dev_priv-fence_regs, obj);
+
return 0;
 }
 
-- 
1.7.10

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


[Intel-gfx] [PATCH 04/12] drm/i915: Discard the unused obj-last_fenced_ring

2012-04-17 Thread Chris Wilson
As we now never pipeline a fence update, obj-last_fenced_ring is always
the same as the obj-ring whenever obj-last_fenced_seqno is active, so
remove it.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_drv.h |5 ++---
 drivers/gpu/drm/i915/i915_gem.c |   16 +---
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 735af61..8c32ada 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -930,13 +930,12 @@ struct drm_i915_gem_object {
 */
uint32_t gtt_offset;
 
-   /** Breadcrumb of last rendering to the buffer. */
-   uint32_t last_rendering_seqno;
struct intel_ring_buffer *ring;
 
+   /** Breadcrumb of last rendering to the buffer. */
+   uint32_t last_rendering_seqno;
/** Breadcrumb of last fenced GPU access to the buffer. */
uint32_t last_fenced_seqno;
-   struct intel_ring_buffer *last_fenced_ring;
 
/** Current tiling stride for the object, if it's tiled. */
uint32_t stride;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3a091f5..b25d229 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1398,7 +1398,6 @@ i915_gem_object_move_to_active(struct drm_i915_gem_object 
*obj,
 
if (obj-fenced_gpu_access) {
obj-last_fenced_seqno = seqno;
-   obj-last_fenced_ring = ring;
 
/* Bump MRU to take account of the delayed flush */
if (obj-fence_reg != I915_FENCE_REG_NONE) {
@@ -1445,7 +1444,6 @@ i915_gem_object_move_to_inactive(struct 
drm_i915_gem_object *obj)
BUG_ON(!list_empty(obj-gpu_write_list));
BUG_ON(!obj-active);
obj-ring = NULL;
-   obj-last_fenced_ring = NULL;
 
i915_gem_object_move_off_active(obj);
obj-fenced_gpu_access = false;
@@ -1650,7 +1648,6 @@ static void i915_gem_reset_fences(struct drm_device *dev)
reg-obj-fence_reg = I915_FENCE_REG_NONE;
reg-obj-fenced_gpu_access = false;
reg-obj-last_fenced_seqno = 0;
-   reg-obj-last_fenced_ring = NULL;
i915_gem_clear_fence_reg(dev, reg);
}
 }
@@ -2295,7 +2292,7 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object 
*obj)
 
if (obj-fenced_gpu_access) {
if (obj-base.write_domain  I915_GEM_GPU_DOMAINS) {
-   ret = i915_gem_flush_ring(obj-last_fenced_ring,
+   ret = i915_gem_flush_ring(obj-ring,
  0, obj-base.write_domain);
if (ret)
return ret;
@@ -2304,10 +2301,10 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object 
*obj)
obj-fenced_gpu_access = false;
}
 
-   if (obj-last_fenced_seqno  NULL != obj-last_fenced_ring) {
-   if (!ring_passed_seqno(obj-last_fenced_ring,
+   if (obj-last_fenced_seqno) {
+   if (!ring_passed_seqno(obj-ring,
   obj-last_fenced_seqno)) {
-   ret = i915_wait_request(obj-last_fenced_ring,
+   ret = i915_wait_request(obj-ring,
obj-last_fenced_seqno,
true);
if (ret)
@@ -2315,7 +2312,6 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object 
*obj)
}
 
obj-last_fenced_seqno = 0;
-   obj-last_fenced_ring = NULL;
}
 
/* Ensure that all CPU reads are completed before installing a fence
@@ -2382,7 +2378,7 @@ i915_find_fence_reg(struct drm_device *dev)
if (first == NULL)
first = reg;
 
-   if (reg-obj-last_fenced_ring == NULL) {
+   if (reg-obj-last_fenced_seqno == 0) {
avail = reg;
break;
}
@@ -2458,7 +2454,6 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
}
 
old-fence_reg = I915_FENCE_REG_NONE;
-   old-last_fenced_ring = NULL;
old-last_fenced_seqno = 0;
 
drm_gem_object_unreference(old-base);
@@ -2467,7 +2462,6 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
reg-obj = obj;
list_move_tail(reg-lru_list, dev_priv-mm.fence_list);
obj-fence_reg = reg - dev_priv-fence_regs;
-   obj-last_fenced_ring = NULL;
 
 update:
obj-tiling_changed = false;
-- 
1.7.10

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


[Intel-gfx] [PATCH 02/12] drm/i915: Remove fence pipelining

2012-04-17 Thread Chris Wilson
Step 2 is then to replace the pipelined parameter with NULL and perform
constant folding to remove dead code.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |  155 +--
 1 file changed, 36 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 40e0808..5a9d90f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2166,8 +2166,7 @@ int i915_gpu_idle(struct drm_device *dev, bool do_retire)
return 0;
 }
 
-static int sandybridge_write_fence_reg(struct drm_i915_gem_object *obj,
-  struct intel_ring_buffer *pipelined)
+static int sandybridge_write_fence_reg(struct drm_i915_gem_object *obj)
 {
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
@@ -2185,26 +2184,12 @@ static int sandybridge_write_fence_reg(struct 
drm_i915_gem_object *obj,
val |= 1  I965_FENCE_TILING_Y_SHIFT;
val |= I965_FENCE_REG_VALID;
 
-   if (pipelined) {
-   int ret = intel_ring_begin(pipelined, 6);
-   if (ret)
-   return ret;
-
-   intel_ring_emit(pipelined, MI_NOOP);
-   intel_ring_emit(pipelined, MI_LOAD_REGISTER_IMM(2));
-   intel_ring_emit(pipelined, FENCE_REG_SANDYBRIDGE_0 + regnum*8);
-   intel_ring_emit(pipelined, (u32)val);
-   intel_ring_emit(pipelined, FENCE_REG_SANDYBRIDGE_0 + regnum*8 + 
4);
-   intel_ring_emit(pipelined, (u32)(val  32));
-   intel_ring_advance(pipelined);
-   } else
-   I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + regnum * 8, val);
+   I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + regnum * 8, val);
 
return 0;
 }
 
-static int i965_write_fence_reg(struct drm_i915_gem_object *obj,
-   struct intel_ring_buffer *pipelined)
+static int i965_write_fence_reg(struct drm_i915_gem_object *obj)
 {
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
@@ -2220,26 +2205,12 @@ static int i965_write_fence_reg(struct 
drm_i915_gem_object *obj,
val |= 1  I965_FENCE_TILING_Y_SHIFT;
val |= I965_FENCE_REG_VALID;
 
-   if (pipelined) {
-   int ret = intel_ring_begin(pipelined, 6);
-   if (ret)
-   return ret;
-
-   intel_ring_emit(pipelined, MI_NOOP);
-   intel_ring_emit(pipelined, MI_LOAD_REGISTER_IMM(2));
-   intel_ring_emit(pipelined, FENCE_REG_965_0 + regnum*8);
-   intel_ring_emit(pipelined, (u32)val);
-   intel_ring_emit(pipelined, FENCE_REG_965_0 + regnum*8 + 4);
-   intel_ring_emit(pipelined, (u32)(val  32));
-   intel_ring_advance(pipelined);
-   } else
-   I915_WRITE64(FENCE_REG_965_0 + regnum * 8, val);
+   I915_WRITE64(FENCE_REG_965_0 + regnum * 8, val);
 
return 0;
 }
 
-static int i915_write_fence_reg(struct drm_i915_gem_object *obj,
-   struct intel_ring_buffer *pipelined)
+static int i915_write_fence_reg(struct drm_i915_gem_object *obj)
 {
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
@@ -2276,24 +2247,12 @@ static int i915_write_fence_reg(struct 
drm_i915_gem_object *obj,
else
fence_reg = FENCE_REG_945_8 + (fence_reg - 8) * 4;
 
-   if (pipelined) {
-   int ret = intel_ring_begin(pipelined, 4);
-   if (ret)
-   return ret;
-
-   intel_ring_emit(pipelined, MI_NOOP);
-   intel_ring_emit(pipelined, MI_LOAD_REGISTER_IMM(1));
-   intel_ring_emit(pipelined, fence_reg);
-   intel_ring_emit(pipelined, val);
-   intel_ring_advance(pipelined);
-   } else
-   I915_WRITE(fence_reg, val);
+   I915_WRITE(fence_reg, val);
 
return 0;
 }
 
-static int i830_write_fence_reg(struct drm_i915_gem_object *obj,
-   struct intel_ring_buffer *pipelined)
+static int i830_write_fence_reg(struct drm_i915_gem_object *obj)
 {
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
@@ -2319,18 +2278,7 @@ static int i830_write_fence_reg(struct 
drm_i915_gem_object *obj,
val |= pitch_val  I830_FENCE_PITCH_SHIFT;
val |= I830_FENCE_REG_VALID;
 
-   if (pipelined) {
-   int ret = intel_ring_begin(pipelined, 4);
-   if (ret)
-   return ret;
-
-   intel_ring_emit(pipelined, MI_NOOP);
-   intel_ring_emit(pipelined, MI_LOAD_REGISTER_IMM(1));
-   intel_ring_emit(pipelined, FENCE_REG_830_0 + regnum*4);
-   intel_ring_emit(pipelined, val);

[Intel-gfx] [PATCH 01/12] drm/i915: Remove the pipelined parameter from get_fence()

2012-04-17 Thread Chris Wilson
We never succeeded in getting pipelined fencing to work (unresolved
spurious GPU hangs), so begin the process of dismantling and removal
the broken code.

Step 1 is the removal of the pipeline parameter to get_fence().

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_drv.h|3 +--
 drivers/gpu/drm/i915/i915_gem.c|7 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
 drivers/gpu/drm/i915/intel_display.c   |2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 422f424..e99a02a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1255,8 +1255,7 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2)
 
 u32 i915_gem_next_request_seqno(struct intel_ring_buffer *ring);
 
-int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
-  struct intel_ring_buffer *pipelined);
+int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
 int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
 
 static inline bool
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index aa44ff2..40e0808 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1079,7 +1079,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)
if (!obj-has_global_gtt_mapping)
i915_gem_gtt_bind_object(obj, obj-cache_level);
 
-   ret = i915_gem_object_get_fence(obj, NULL);
+   ret = i915_gem_object_get_fence(obj);
if (ret)
goto unlock;
 
@@ -2453,7 +2453,6 @@ i915_find_fence_reg(struct drm_device *dev,
 /**
  * i915_gem_object_get_fence - set up fencing for an object
  * @obj: object to map through a fence reg
- * @pipelined: ring on which to queue the change, or NULL for CPU access
  *
  * When mapping objects through the GTT, userspace wants to be able to write
  * to them without having to worry about swizzling if the object is tiled.
@@ -2466,11 +2465,11 @@ i915_find_fence_reg(struct drm_device *dev,
  * For an untiled surface, this removes any existing fence.
  */
 int
-i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
- struct intel_ring_buffer *pipelined)
+i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
 {
struct drm_device *dev = obj-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   struct intel_ring_buffer *pipelined;
struct drm_i915_fence_reg *reg;
int ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2a24d0c..b5262fd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -530,7 +530,7 @@ pin_and_fence_object(struct drm_i915_gem_object *obj,
 
if (has_fenced_gpu_access) {
if (entry-flags  EXEC_OBJECT_NEEDS_FENCE) {
-   ret = i915_gem_object_get_fence(obj, ring);
+   ret = i915_gem_object_get_fence(obj);
if (ret)
goto err_unpin;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7506a72..95d7713 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2151,7 +2151,7 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
 * framebuffer compression.  For simplicity, we always install
 * a fence as the cost is not that onerous.
 */
-   ret = i915_gem_object_get_fence(obj, pipelined);
+   ret = i915_gem_object_get_fence(obj);
if (ret)
goto err_unpin;
 
-- 
1.7.10

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


Re: [Intel-gfx] [PATCH 09/12] drm/edid: Update range descriptor struct for EDID 1.4

2012-04-17 Thread Takashi Iwai
At Fri, 13 Apr 2012 16:33:37 -0400,
Adam Jackson wrote:
 
 Signed-off-by: Adam Jackson a...@redhat.com
 ---
  include/drm/drm_edid.h |   26 --
  1 files changed, 20 insertions(+), 6 deletions(-)
 
 diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
 index bcb9a66..8cefbbe 100644
 --- a/include/drm/drm_edid.h
 +++ b/include/drm/drm_edid.h
 @@ -90,12 +90,26 @@ struct detailed_data_monitor_range {
   u8 min_hfreq_khz;
   u8 max_hfreq_khz;
   u8 pixel_clock_mhz; /* need to multiply by 10 */
 - __le16 sec_gtf_toggle; /* A000=use above, 20=use below */
 - u8 hfreq_start_khz; /* need to multiply by 2 */
 - u8 c; /* need to divide by 2 */
 - __le16 m;
 - u8 k;
 - u8 j; /* need to divide by 2 */
 + u8 flags;
 + union {
 + struct {
 + u8 reserved;
 + u8 hfreq_start_khz; /* need to multiply by 2 */
 + u8 c; /* need to divide by 2 */
 + __le16 m;
 + u8 k;
 + u8 j; /* need to divide by 2 */
 + } gtf2;
 + struct {
 + u8 version;
 + u8 data1; /* high 6 bits: extra clock resolution */
 + u8 data2; /* plus low 2 of above: max hactive */
 + u8 supported_aspects;
 + u8 flags; /* preferred aspect and blanking support */
 + u8 supported_scalings;
 + u8 preferred_refresh;
 + } cvt;

These new structs must be marked with __attribute__((packed)) although
the struct detailed_data_monitor_range itself is already marked.  At
least, with gcc 4.6 and x86-64 here, they get unaligned.


thanks,

Takashi

 + } formula;
  } __attribute__((packed));
  
  struct detailed_data_wpindex {
 -- 
 1.7.7.6
 
 ___
 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 00/12] Add more DMT and common modes

2012-04-17 Thread Takashi Iwai
At Fri, 13 Apr 2012 16:56:26 -0400,
Adam Jackson wrote:
 
 On 4/13/12 4:33 PM, Adam Jackson wrote:
  Incorporates some feedback from Rodrigo and Takashi.  Major themes of the
  series:
 
  - Fix the DMT list to include reduced-blanking modes
  - Add modes from DMT for any range descriptor type
  - Add an extra modes list for things not in DMT
  - For ranges that specify a formula, generate timings from the extra modes
 
  This still doesn't address the driver policy decision of I know I have
  a scaler, add modes as if there were a range descriptor even if there's
  not one.  But it should at least make clear what such a helper function
  should do.
 
 One minor buglet in this series that's not obvious from inspection (and 
 that I didn't realize until just now) is that putting 1366x768 in the 
 minimode list won't do what you want, since the mode you get back will 
 be 1368x768.  Probably we should move the manual-underscan hack into the 
 timing generators themselves.

Sounds like a good compromise.  We have already hacks in drm_edid.c
for HDMI TV, so one exception more isn't that bad ;)


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


Re: [Intel-gfx] [PATCH 00/12] Add more DMT and common modes

2012-04-17 Thread Takashi Iwai
At Fri, 13 Apr 2012 16:33:28 -0400,
Adam Jackson wrote:
 
 Incorporates some feedback from Rodrigo and Takashi.  Major themes of the
 series:
 
 - Fix the DMT list to include reduced-blanking modes
 - Add modes from DMT for any range descriptor type
 - Add an extra modes list for things not in DMT
 - For ranges that specify a formula, generate timings from the extra modes
 
 This still doesn't address the driver policy decision of I know I have
 a scaler, add modes as if there were a range descriptor even if there's
 not one.  But it should at least make clear what such a helper function
 should do.

I tried these patches now with a few monitors here and it looks
working well (after fixing the alignment as I posted in another
mail).  I got new 1600x900 output on two monitors.  Yay!

I guess this will solve major of problems we've face for HD+ panel.
The rest is the 1366x768 mode, but it's often covered by 1360x768.
So, once when LVDS gets additional de facto standard modes, it'll be
fixed as well.


thanks,

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


[Intel-gfx] [PATCH] drm/i915: Replace open coded MI_BATCH_GTT

2012-04-17 Thread Chris Wilson
The (26) virtual memory space selector harks back to gen3 and is
mandatory given our use of GTT space for batchbuffers. On gen4+, use of
the GTT became mandatory and bit6 marked reserved. However the code must
now explicitly set (17), which conveniently is also (26).

To clarify the meaning for future readers, replace the open coded (26)
with MI_BATCH_GTT.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_reg.h |1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b5f937a..f617437 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -231,6 +231,7 @@
 #define   MI_BATCH_NON_SECURE  (1)
 #define   MI_BATCH_NON_SECURE_I965 (18)
 #define MI_BATCH_BUFFER_START  MI_INSTR(0x31, 0)
+#define   MI_BATCH_GTT (26) /* aliased with (17) on gen4 */
 #define MI_SEMAPHORE_MBOX  MI_INSTR(0x16, 1) /* gen6+ */
 #define  MI_SEMAPHORE_GLOBAL_GTT(122)
 #define  MI_SEMAPHORE_UPDATE   (121)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 90eddd3..7270d06 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -817,7 +817,8 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, 
u32 offset, u32 length)
return ret;
 
intel_ring_emit(ring,
-   MI_BATCH_BUFFER_START | (2  6) |
+   MI_BATCH_BUFFER_START |
+   MI_BATCH_GTT |
MI_BATCH_NON_SECURE_I965);
intel_ring_emit(ring, offset);
intel_ring_advance(ring);
@@ -854,7 +855,7 @@ i915_dispatch_execbuffer(struct intel_ring_buffer *ring,
if (ret)
return ret;
 
-   intel_ring_emit(ring, MI_BATCH_BUFFER_START | (2  6));
+   intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT);
intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
intel_ring_advance(ring);
 
-- 
1.7.10

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


Re: [Intel-gfx] [PATCH] drm/i915: intel_update_fbc() requires struct_mutex, so no longer atomic

2012-04-17 Thread Jesse Barnes
On Tue, 17 Apr 2012 15:08:19 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 As we need to manipulate our device structure and allocate queue a task,
 it is no longer a simple atomic operation and cannot be performed along
 the atomic modeset paths. Instead make sure that we disable FBC (which
 must be therefore kept as a set of simple register writes) when
 performing the atomic modeset and leave the heavy-weight
 intel_update_fbc() for the normal modeset.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 ---

Looks good.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 00/12] Add more DMT and common modes

2012-04-17 Thread Takashi Iwai
At Tue, 17 Apr 2012 17:33:17 +0200,
Takashi Iwai wrote:
 
 At Fri, 13 Apr 2012 16:33:28 -0400,
 Adam Jackson wrote:
  
  Incorporates some feedback from Rodrigo and Takashi.  Major themes of the
  series:
  
  - Fix the DMT list to include reduced-blanking modes
  - Add modes from DMT for any range descriptor type
  - Add an extra modes list for things not in DMT
  - For ranges that specify a formula, generate timings from the extra modes
  
  This still doesn't address the driver policy decision of I know I have
  a scaler, add modes as if there were a range descriptor even if there's
  not one.  But it should at least make clear what such a helper function
  should do.
 
 I tried these patches now with a few monitors here and it looks
 working well (after fixing the alignment as I posted in another
 mail).  I got new 1600x900 output on two monitors.  Yay!

Oh, feel free to add my tested-by tag:

Tested-by: Takashi Iwai ti...@suse.de


thanks,

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


Re: [Intel-gfx] [PATCH] drm/i915: Mask reserved bits in display/sprite address registers

2012-04-17 Thread Jesse Barnes
On Mon, 16 Apr 2012 21:02:41 +
Reese, Armin C armin.c.re...@intel.com wrote:

 Corrected my name in the patch (acreese - Armin Reese) ...
 
 The attached patch file was updated to reflect reviewer's comments.  The only 
 change I did not make was using PAGE_MASK instead of DISP_BASEADDR_MASK.  
 PAGE_MASK is CPU architecture dependent and I didn't want to tie our GPU 
 addressing arch to that of the CPU.

Looks good, thanks Armin.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, 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 00/12] Add more DMT and common modes

2012-04-17 Thread Rodrigo Vivi
Thanks for the patches ajax

Feel free to use
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com

On Tue, Apr 17, 2012 at 12:50 PM, Takashi Iwai ti...@suse.de wrote:
 At Tue, 17 Apr 2012 17:33:17 +0200,
 Takashi Iwai wrote:

 At Fri, 13 Apr 2012 16:33:28 -0400,
 Adam Jackson wrote:
 
  Incorporates some feedback from Rodrigo and Takashi.  Major themes of the
  series:
 
  - Fix the DMT list to include reduced-blanking modes
  - Add modes from DMT for any range descriptor type
  - Add an extra modes list for things not in DMT
  - For ranges that specify a formula, generate timings from the extra modes
 
  This still doesn't address the driver policy decision of I know I have
  a scaler, add modes as if there were a range descriptor even if there's
  not one.  But it should at least make clear what such a helper function
  should do.

 I tried these patches now with a few monitors here and it looks
 working well (after fixing the alignment as I posted in another
 mail).  I got new 1600x900 output on two monitors.  Yay!

 Oh, feel free to add my tested-by tag:

        Tested-by: Takashi Iwai ti...@suse.de


 thanks,

 Takashi
 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
GPG: 0x905BE242 @ wwwkeys.pgp.net
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/1] drm/i915: split power-related items into intel_pm module

2012-04-17 Thread Jesse Barnes
On Mon, 16 Apr 2012 20:06:01 -0300
Eugeni Dodonov eugeni.dodo...@intel.com wrote:

 As previously discussed on irc with Daniel, Ben and Jesse, This patch
 moves the power-related functionality into intel_pm module, aiming at
 simplifying the intel_display code and make it less cluttered.
 
 The functionality affected by this move are: clock gating, rc6 and rps,
 display watermarks, display fifo-related items, cxsr and FBC.
 
 This simplifies intel_display by +/- 2800 lines of code and centralizes
 most of the power-related items in one place to simplify future hardware
 enablement and subsystems interaction.
 
 CC: Jesse Barnes jbar...@virtuousgeek.org
 CC: Ben Widawsky b...@bwidawsk.net
 CC: Daniel Vetter daniel.vet...@ffwll.ch
 CC: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com

Acked-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, 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 09/12] drm/edid: Update range descriptor struct for EDID 1.4

2012-04-17 Thread Adam Jackson
On Tue, 2012-04-17 at 17:25 +0200, Takashi Iwai wrote:
 At Fri, 13 Apr 2012 16:33:37 -0400, Adam Jackson wrote:
  +   u8 flags;
  +   union {
  +   struct {
  +   u8 reserved;
  +   u8 hfreq_start_khz; /* need to multiply by 2 */
  +   u8 c; /* need to divide by 2 */
  +   __le16 m;
  +   u8 k;
  +   u8 j; /* need to divide by 2 */
  +   } gtf2;
  +   struct {
  +   u8 version;
  +   u8 data1; /* high 6 bits: extra clock resolution */
  +   u8 data2; /* plus low 2 of above: max hactive */
  +   u8 supported_aspects;
  +   u8 flags; /* preferred aspect and blanking support */
  +   u8 supported_scalings;
  +   u8 preferred_refresh;
  +   } cvt;
 
 These new structs must be marked with __attribute__((packed)) although
 the struct detailed_data_monitor_range itself is already marked.  At
 least, with gcc 4.6 and x86-64 here, they get unaligned.

Eek.  Good catch, thanks.

- ajax


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


[Intel-gfx] [PATCH] drm/i915: Unpin the flip target if we fail to queue the flip

2012-04-17 Thread Chris Wilson
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_display.c |   50 --
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 85f1814..b2c29a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7761,14 +7761,14 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
 
ret = intel_pin_and_fence_fb_obj(dev, obj, LP_RING(dev_priv));
if (ret)
-   goto out;
+   goto err;
 
/* Offset into the new buffer for cases of shared fbs between CRTCs */
offset = crtc-y * fb-pitches[0] + crtc-x * fb-bits_per_pixel/8;
 
ret = BEGIN_LP_RING(6);
if (ret)
-   goto out;
+   goto err_unpin;
 
/* Can't queue multiple flips, so wait for the previous
 * one to finish before executing the next.
@@ -7785,7 +7785,11 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
OUT_RING(obj-gtt_offset + offset);
OUT_RING(0); /* aux display base address, unused */
ADVANCE_LP_RING();
-out:
+   return 0;
+
+err_unpin:
+   intel_unpin_fb_obj(obj);
+err:
return ret;
 }
 
@@ -7802,14 +7806,14 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
 
ret = intel_pin_and_fence_fb_obj(dev, obj, LP_RING(dev_priv));
if (ret)
-   goto out;
+   goto err;
 
/* Offset into the new buffer for cases of shared fbs between CRTCs */
offset = crtc-y * fb-pitches[0] + crtc-x * fb-bits_per_pixel/8;
 
ret = BEGIN_LP_RING(6);
if (ret)
-   goto out;
+   goto err_unpin;
 
if (intel_crtc-plane)
flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
@@ -7824,7 +7828,11 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
OUT_RING(MI_NOOP);
 
ADVANCE_LP_RING();
-out:
+   return 0;
+
+err_unpin:
+   intel_unpin_fb_obj(obj);
+err:
return ret;
 }
 
@@ -7840,11 +7848,11 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
 
ret = intel_pin_and_fence_fb_obj(dev, obj, LP_RING(dev_priv));
if (ret)
-   goto out;
+   goto err;
 
ret = BEGIN_LP_RING(4);
if (ret)
-   goto out;
+   goto err_unpin;
 
/* i965+ uses the linear or tiled offsets from the
 * Display Registers (which do not change across a page-flip)
@@ -7863,7 +7871,11 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
pipesrc = I915_READ(PIPESRC(intel_crtc-pipe))  0x0fff0fff;
OUT_RING(pf | pipesrc);
ADVANCE_LP_RING();
-out:
+   return 0;
+
+err_unpin:
+   intel_unpin_fb_obj(obj);
+err:
return ret;
 }
 
@@ -7879,11 +7891,11 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
 
ret = intel_pin_and_fence_fb_obj(dev, obj, LP_RING(dev_priv));
if (ret)
-   goto out;
+   goto err;
 
ret = BEGIN_LP_RING(4);
if (ret)
-   goto out;
+   goto err_unpin;
 
OUT_RING(MI_DISPLAY_FLIP |
 MI_DISPLAY_FLIP_PLANE(intel_crtc-plane));
@@ -7894,7 +7906,11 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
pipesrc = I915_READ(PIPESRC(intel_crtc-pipe))  0x0fff0fff;
OUT_RING(pf | pipesrc);
ADVANCE_LP_RING();
-out:
+   return 0;
+
+err_unpin:
+   intel_unpin_fb_obj(obj);
+err:
return ret;
 }
 
@@ -7920,18 +7936,22 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 
ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
if (ret)
-   goto out;
+   goto err;
 
ret = intel_ring_begin(ring, 4);
if (ret)
-   goto out;
+   goto err_unpin;
 
intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | (intel_crtc-plane  19));
intel_ring_emit(ring, (fb-pitches[0] | obj-tiling_mode));
intel_ring_emit(ring, (obj-gtt_offset));
intel_ring_emit(ring, (MI_NOOP));
intel_ring_advance(ring);
-out:
+   return 0;
+
+err_unpin:
+   intel_unpin_fb_obj(obj);
+err:
return ret;
 }
 
-- 
1.7.10

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


[Intel-gfx] TIMESTAMP register

2012-04-17 Thread Lawrynowicz, Jacek
Hi,

 

Starting from IVB the main TIMESTAMP register is longer than 32 bits (it’s 36 
bits long).

How should we pass its 36 bit value from i915 to user space?

GET_PARAM ioctls supports only 32 bit params. Should we add GET_PARAM64 or 
create separate ioctl to access the TIMESTAMP?

Any suggestions?

 

This functionality is required for any OpenGL driver which implements 
GL_ARB_timer_query.

 

Regards,

Jacek

 



smime.p7s
Description: S/MIME cryptographic signature
-
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Do not set Enable Panel Fitter on SNB pageflips

2012-04-17 Thread Chris Wilson
Not only do the pageflip work without it at non-native modes (i.e. with
the panel fitter enabled), it also causes normal (non-pageflipped)
modesets to fail.

Reported-by: Adam Jackson a...@redhat.com
Cc: Adam Jackson a...@redhat.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/intel_display.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b2c29a5..2319838 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7902,7 +7902,13 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
OUT_RING(fb-pitches[0] | obj-tiling_mode);
OUT_RING(obj-gtt_offset);
 
-   pf = I915_READ(PF_CTL(intel_crtc-pipe))  PF_ENABLE;
+   /* Contrary to the suggestions in the documentation,
+* Enable Panel Fitter does not seem to be required when page
+* flipping with a non-native mode, and worse causes a normal
+* modeset to fail.
+* pf = I915_READ(PF_CTL(intel_crtc-pipe))  PF_ENABLE;
+*/
+   pf = 0;
pipesrc = I915_READ(PIPESRC(intel_crtc-pipe))  0x0fff0fff;
OUT_RING(pf | pipesrc);
ADVANCE_LP_RING();
-- 
1.7.10

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


Re: [Intel-gfx] TIMESTAMP register

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 07:12:40PM +, Lawrynowicz, Jacek wrote:
 Starting from IVB the main TIMESTAMP register is longer than 32 bits
 (it’s 36 bits long).
 
 How should we pass its 36 bit value from i915 to user space?
 
 GET_PARAM ioctls supports only 32 bit params. Should we add GET_PARAM64
 or create separate ioctl to access the TIMESTAMP?
 
 Any suggestions?
 
  
 
 This functionality is required for any OpenGL driver which implements 
 GL_ARB_timer_query.

Like occlusion queries I presume this should be done with a pipe control
qword write issued from a batch. GETPARAM makes absolutely no sense, that
is for driver features, not hw features and just used so that a new driver
also can work on an older kernel.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] TIMESTAMP register

2012-04-17 Thread Lawrynowicz, Jacek
ARB_timer_query allows client read TIMESTAMP both asynchronously and 
synchronously.
The former can be implemented as you said but the latter requires support from 
the KMD.
This must be a simple MMIO read as this is the only way to report current GPU 
time.
Implementing synchronous TIMESTAMP query using pipe control would render the 
third example from ARB_timer_query spec useless.

Regards,
Jacek

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, April 17, 2012 9:40 PM
To: Lawrynowicz, Jacek
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] TIMESTAMP register

On Tue, Apr 17, 2012 at 07:12:40PM +, Lawrynowicz, Jacek wrote:
 Starting from IVB the main TIMESTAMP register is longer than 32 bits 
 (it’s 36 bits long).
 
 How should we pass its 36 bit value from i915 to user space?
 
 GET_PARAM ioctls supports only 32 bit params. Should we add 
 GET_PARAM64 or create separate ioctl to access the TIMESTAMP?
 
 Any suggestions?
 
  
 
 This functionality is required for any OpenGL driver which implements 
 GL_ARB_timer_query.

Like occlusion queries I presume this should be done with a pipe control qword 
write issued from a batch. GETPARAM makes absolutely no sense, that is for 
driver features, not hw features and just used so that a new driver also can 
work on an older kernel.

-Daniel
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48


smime.p7s
Description: S/MIME cryptographic signature
-
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] TIMESTAMP register

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 08:34:11PM +, Lawrynowicz, Jacek wrote:
 ARB_timer_query allows client read TIMESTAMP both asynchronously and 
 synchronously.
 The former can be implemented as you said but the latter requires support 
 from the KMD.
 This must be a simple MMIO read as this is the only way to report current 
 GPU time.
 Implementing synchronous TIMESTAMP query using pipe control would render the 
 third example from ARB_timer_query spec useless.

Ok, I've looked like a dofus again, but now I've read the spec and we
indeed seem to need a synchronous readout of the TIMESTAMP register. I
guess a new register will do, together with some fixed-point integer that
tells userspace how to convert it to nanoseconds.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Do not set Enable Panel Fitter on SNB pageflips

2012-04-17 Thread Adam Jackson
On Tue, 2012-04-17 at 20:37 +0100, Chris Wilson wrote:
 Not only do the pageflip work without it at non-native modes (i.e. with
 the panel fitter enabled), it also causes normal (non-pageflipped)
 modesets to fail.

Testcase: enable a GLX compositor like gnome-shell, resize X to
something other than LVDS native size, vt switch to text console, frown
at black screen of doom.

Fixes the above testcase on a Lenovo X220.

Tested-by: Adam Jackson a...@redhat.com

- ajax


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: Do not set Enable Panel Fitter on SNB pageflips

2012-04-17 Thread Daniel Vetter
On Tue, Apr 17, 2012 at 05:19:45PM -0400, Adam Jackson wrote:
 On Tue, 2012-04-17 at 20:37 +0100, Chris Wilson wrote:
  Not only do the pageflip work without it at non-native modes (i.e. with
  the panel fitter enabled), it also causes normal (non-pageflipped)
  modesets to fail.
 
 Testcase: enable a GLX compositor like gnome-shell, resize X to
 something other than LVDS native size, vt switch to text console, frown
 at black screen of doom.
 
 Fixes the above testcase on a Lenovo X220.
 
 Tested-by: Adam Jackson a...@redhat.com
Queued for -next, thanks for the patch and testing.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/4] drm/i915: IBX+ doesn't have separate vsync/hsync controls on the VGA DAC

2012-04-17 Thread Jesse Barnes
Like this?

From fee9f6fbf2230f96b9195baf32fd67b243969a14 Mon Sep 17 00:00:00 2001
From: Jesse Barnes jbar...@virtuousgeek.org
Date: Wed, 11 Apr 2012 09:23:34 -0700
Subject: [PATCH] drm/i915: IBX+ doesn't have separate vsync/hsync controls on
 the VGA DAC

When the PCH split occurred, we dropped support for separate hsync and
vsync disable in the VGA DAC.  So add a PCH specific DPMS function that
just uses the port enable bit for controlling DPMS states.

Before this fix, when anything other than a full DPMS off occurred,
the VGA port would be left enabled and scanning out while all the other
heads would turn off as expected.

v2: duplicate encoder helper vtable into pch and gmch versions (Daniel)

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_crt.c |   54 ++---
 1 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 70b0f1a..fb516e9 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -55,18 +55,36 @@ static struct intel_crt *intel_attached_crt(struct 
drm_connector *connector)
struct intel_crt, base);
 }
 
-static void intel_crt_dpms(struct drm_encoder *encoder, int mode)
+static void pch_crt_dpms(struct drm_encoder *encoder, int mode)
 {
struct drm_device *dev = encoder-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   u32 temp, reg;
+   u32 temp;
 
-   if (HAS_PCH_SPLIT(dev))
-   reg = PCH_ADPA;
-   else
-   reg = ADPA;
+   temp = I915_READ(PCH_ADPA);
+   temp = ~ADPA_DAC_ENABLE;
+
+   switch (mode) {
+   case DRM_MODE_DPMS_ON:
+   temp |= ADPA_DAC_ENABLE;
+   break;
+   case DRM_MODE_DPMS_STANDBY:
+   case DRM_MODE_DPMS_SUSPEND:
+   case DRM_MODE_DPMS_OFF:
+   /* Just leave port enable cleared */
+   break;
+   }
+
+   I915_WRITE(PCH_ADPA, temp);
+}
 
-   temp = I915_READ(reg);
+static void intel_crt_dpms(struct drm_encoder *encoder, int mode)
+{
+   struct drm_device *dev = encoder-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 temp;
+
+   temp = I915_READ(ADPA);
temp = ~(ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE);
temp = ~ADPA_DAC_ENABLE;
 
@@ -85,7 +103,7 @@ static void intel_crt_dpms(struct drm_encoder *encoder, int 
mode)
break;
}
 
-   I915_WRITE(reg, temp);
+   I915_WRITE(ADPA, temp);
 }
 
 static int intel_crt_mode_valid(struct drm_connector *connector,
@@ -516,12 +534,20 @@ static void intel_crt_reset(struct drm_connector 
*connector)
  * Routines for controlling stuff on the analog port
  */
 
-static const struct drm_encoder_helper_funcs intel_crt_helper_funcs = {
-   .dpms = intel_crt_dpms,
+static const struct drm_encoder_helper_funcs pch_encoder_funcs = {
+   .mode_fixup = intel_crt_mode_fixup,
+   .prepare = intel_encoder_prepare,
+   .commit = intel_encoder_commit,
+   .mode_set = intel_crt_mode_set,
+   .dpms = pch_crt_dpms,
+};
+
+static const struct drm_encoder_helper_funcs gmch_encoder_funcs = {
.mode_fixup = intel_crt_mode_fixup,
.prepare = intel_encoder_prepare,
.commit = intel_encoder_commit,
.mode_set = intel_crt_mode_set,
+   .dpms = intel_crt_dpms,
 };
 
 static const struct drm_connector_funcs intel_crt_connector_funcs = {
@@ -567,6 +593,7 @@ void intel_crt_init(struct drm_device *dev)
struct intel_crt *crt;
struct intel_connector *intel_connector;
struct drm_i915_private *dev_priv = dev-dev_private;
+   const struct drm_encoder_helper_funcs *encoder_helper_funcs;
 
/* Skip machines without VGA that falsely report hotplug events */
if (dmi_check_system(intel_no_crt))
@@ -602,7 +629,12 @@ void intel_crt_init(struct drm_device *dev)
connector-interlace_allowed = 1;
connector-doublescan_allowed = 0;
 
-   drm_encoder_helper_add(crt-base.base, intel_crt_helper_funcs);
+   if (HAS_PCH_SPLIT(dev))
+   encoder_helper_funcs = pch_encoder_funcs;
+   else
+   encoder_helper_funcs = gmch_encoder_funcs;
+
+   drm_encoder_helper_add(crt-base.base, encoder_helper_funcs);
drm_connector_helper_add(connector, intel_crt_connector_helper_funcs);
 
drm_sysfs_connector_add(connector);
-- 
1.7.5.4


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


Re: [Intel-gfx] [PATCH 2/4] drm/i915: IBX+ doesn't have separate vsync/hsync controls on the VGA DAC

2012-04-17 Thread Chris Wilson
On Tue, 17 Apr 2012 15:06:33 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:
 Like this?
 
 From fee9f6fbf2230f96b9195baf32fd67b243969a14 Mon Sep 17 00:00:00 2001
 From: Jesse Barnes jbar...@virtuousgeek.org
 Date: Wed, 11 Apr 2012 09:23:34 -0700
 Subject: [PATCH] drm/i915: IBX+ doesn't have separate vsync/hsync controls on
  the VGA DAC
 
 When the PCH split occurred, we dropped support for separate hsync and
 vsync disable in the VGA DAC.  So add a PCH specific DPMS function that
 just uses the port enable bit for controlling DPMS states.
 
 Before this fix, when anything other than a full DPMS off occurred,
 the VGA port would be left enabled and scanning out while all the other
 heads would turn off as expected.
 
 v2: duplicate encoder helper vtable into pch and gmch versions (Daniel)
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org

With s/intel_crt_dpms/gmch_crt_dpms/ and
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48491

Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

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


Re: [Intel-gfx] TIMESTAMP register

2012-04-17 Thread Ben Widawsky
On Tue, 17 Apr 2012 23:04:18 +0200
Daniel Vetter dan...@ffwll.ch wrote:

 On Tue, Apr 17, 2012 at 08:34:11PM +, Lawrynowicz, Jacek wrote:
  ARB_timer_query allows client read TIMESTAMP both asynchronously and 
  synchronously.
  The former can be implemented as you said but the latter requires support 
  from the KMD.
  This must be a simple MMIO read as this is the only way to report current 
  GPU time.
  Implementing synchronous TIMESTAMP query using pipe control would render 
  the third example from ARB_timer_query spec useless.
 
 Ok, I've looked like a dofus again, but now I've read the spec and we
 indeed seem to need a synchronous readout of the TIMESTAMP register. I
 guess a new register will do, together with some fixed-point integer that
 tells userspace how to convert it to nanoseconds.
 -Daniel

I've not read the spec, but synchronous and current doesn't mean the
exact same thing to me. I assume the spec doesn't allow getting the
value in a batch and then just waiting for rendering to complete?

If we go with the register read approach, I vote sysfs.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays

2012-04-17 Thread Paulo Zanoni
2012/4/16 Ben Widawsky b...@bwidawsk.net:
 Sparse doesn't like:
 error: bad constant expression


bikeshedding
I know you'll hate me for asking, but: how difficult is it to fix sparse?
Adding those mallocs/frees increases the code complexity, making it
harder to read...
/bikeshedding

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