[Intel-gfx] [PATCH 5/7 v2] drm/i915: Add setters for min/max frequency

2012-09-06 Thread Ben Widawsky
Provide a standardized sysfs interface for setting min, and max
frequencies.  The code which reads the limits were lifted from the
debugfs files. As a brief explanation, the limits are similar to the CPU
p-states. We have 3 states:

RP0 - ie. max frequency
RP1 - ie. "preferred min" frequency
RPn - seriously lowest frequency

Initially Daniel asked me to clamp the writes to supported values, but
in conforming to the way the cpufreq drivers seem to work, instead
return -EINVAL (noticed by Jesse in discussion).

The values can be used by userspace wishing to control the limits of the
GPU (see the CC list for people who care).

v2: add the dropped mutex_unlock in error cases (Chris)
EINVAL on both too min, or too max (Daniel)

CC: Arjan van de Ven 
CC: Jacob Pan 
CC: Daniel Vetter 
CC: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 84 ++-
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index 6c7f905..4f470cf 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -238,6 +238,46 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, 
struct device_attribute
return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_max_freq_mhz_store(struct device *kdev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val, rp_state_cap, hw_max, hw_min;
+   ssize_t ret;
+
+   ret = kstrtou32(buf, 0, &val);
+   if (ret)
+   return ret;
+
+   ret = mutex_lock_interruptible(&dev->struct_mutex);
+   if (ret)
+   return ret;
+
+   rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+   hw_max = (rp_state_cap & 0xff) * GT_FREQUENCY_MULTIPLIER;
+   hw_min = ((rp_state_cap & 0xff) >> 16) * GT_FREQUENCY_MULTIPLIER;
+
+   if (val < hw_min || val > hw_max) {
+   mutex_unlock(&dev->struct_mutex);
+   return -EINVAL;
+   }
+
+   if (val < dev_priv->rps.min_delay)
+   val = dev_priv->rps.min_delay;
+
+   if (dev_priv->rps.cur_delay > val)
+   gen6_set_rps(dev_priv->dev, val);
+
+   dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+   mutex_unlock(&dev->struct_mutex);
+
+   return count;
+}
+
 static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)
 {
struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
@@ -255,9 +295,49 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, 
struct device_attribute
return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_min_freq_mhz_store(struct device *kdev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val, rp_state_cap, hw_max, hw_min;
+   ssize_t ret;
+
+   ret = kstrtou32(buf, 0, &val);
+   if (ret)
+   return ret;
+
+   ret = mutex_lock_interruptible(&dev->struct_mutex);
+   if (ret)
+   return ret;
+
+   rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+   hw_max = (rp_state_cap & 0xff) * GT_FREQUENCY_MULTIPLIER;
+   hw_min = ((rp_state_cap & 0xff) >> 16) * GT_FREQUENCY_MULTIPLIER;
+
+   if (val < hw_min || val > hw_max) {
+   mutex_unlock(&dev->struct_mutex);
+   return -EINVAL;
+   }
+
+   if (val > dev_priv->rps.max_delay)
+   val = dev_priv->rps.max_delay;
+
+   if (dev_priv->rps.cur_delay < val)
+   gen6_set_rps(dev_priv->dev, val);
+
+   dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+   mutex_unlock(&dev->struct_mutex);
+
+   return count;
+}
+
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
NULL);
-static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
gt_max_freq_mhz_store);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
gt_min_freq_mhz_store);
 
 static const struct attribute *gen6_attrs[] = {
&dev_attr_gt_cur_freq_mhz.attr,
-- 
1.7.12

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


Re: [Intel-gfx] [PATCH 05/24] drm/i915: Replace the array of pages with a scatterlist

2012-09-06 Thread Ben Widawsky
On Tue,  4 Sep 2012 21:02:57 +0100
Chris Wilson  wrote:

> Rather than have multiple data structures for describing our page layout
> in conjunction with the array of pages, we can migrate all users over to
> a scatterlist.
> 
> One major advantage, other than unifying the page tracking structures,
> this offers is that we replace the vmalloc'ed array (which can be up to
> a megabyte in size) with a chain of individual pages which helps reduce
> memory pressure.
> 
> The disadvantage is that we then do not have a simple array to iterate,
> or to access randomly. The common case for this is in the relocation
> processing, which will typically fit within a single scatterlist page
> and so be almost the same cost as the simple array. For iterating over
> the array, the extra function call could be optimised away, but in
> reality is an insignificant cost of either binding the pages, or
> performing the pwrite/pread.
> 
> Signed-off-by: Chris Wilson 


Now that my eyes are done bleeding, easy ones:

ERROR: space required after that ',' (ctx:VxV)
#69: FILE: drivers/char/agp/intel-gtt.c:99:
+   for_each_sg(st->sgl, sg, num_entries,i)
^

WARNING: Prefer pr_err(... to printk(KERN_ERR, ...
#189: FILE: drivers/gpu/drm/drm_cache.c:117:
+   printk(KERN_ERR "Timed out waiting for cache
flush.\n");

WARNING: Prefer pr_err(... to printk(KERN_ERR, ...
#191: FILE: drivers/gpu/drm/drm_cache.c:119:
+   printk(KERN_ERR "Architecture has no drm_cache.c support\n");

In addition to the inline comments, it would have been even slightly
easier to review without the s/page/i since it seems to just be for no
compelling reason anyway.



> ---
>  drivers/char/agp/intel-gtt.c   |   51 +---
>  drivers/gpu/drm/drm_cache.c|   23 ++
>  drivers/gpu/drm/i915/i915_drv.h|   18 +++--
>  drivers/gpu/drm/i915/i915_gem.c|   79 --
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c |   99 +++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |3 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c|  121 
> ++--
>  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 ++--
>  drivers/gpu/drm/i915/i915_irq.c|   25 +++---
>  drivers/gpu/drm/i915/intel_ringbuffer.c|9 ++-
>  include/drm/drmP.h |1 +
>  include/drm/intel-gtt.h|   10 +--
>  12 files changed, 236 insertions(+), 219 deletions(-)
> 
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 58e32f7..511d1b1 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -84,40 +84,33 @@ static struct _intel_private {
>  #define IS_IRONLAKE  intel_private.driver->is_ironlake
>  #define HAS_PGTBL_EN intel_private.driver->has_pgtbl_enable
>  
> -int intel_gtt_map_memory(struct page **pages, unsigned int num_entries,
> -  struct scatterlist **sg_list, int *num_sg)
> +static int intel_gtt_map_memory(struct page **pages,
> + unsigned int num_entries,
> + struct sg_table *st)
>  {
> - struct sg_table st;
>   struct scatterlist *sg;
>   int i;
>  
> - if (*sg_list)
> - return 0; /* already mapped (for e.g. resume */
> -
>   DBG("try mapping %lu pages\n", (unsigned long)num_entries);
>  
> - if (sg_alloc_table(&st, num_entries, GFP_KERNEL))
> + if (sg_alloc_table(st, num_entries, GFP_KERNEL))
>   goto err;
>  
> - *sg_list = sg = st.sgl;
> -
> - for (i = 0 ; i < num_entries; i++, sg = sg_next(sg))
> + for_each_sg(st->sgl, sg, num_entries,i)
>   sg_set_page(sg, pages[i], PAGE_SIZE, 0);
>  
> - *num_sg = pci_map_sg(intel_private.pcidev, *sg_list,
> -  num_entries, PCI_DMA_BIDIRECTIONAL);
> - if (unlikely(!*num_sg))
> + if (!pci_map_sg(intel_private.pcidev,
> + st->sgl, st->nents, PCI_DMA_BIDIRECTIONAL))
>   goto err;
>  
>   return 0;
>  
>  err:
> - sg_free_table(&st);
> + sg_free_table(st);
>   return -ENOMEM;
>  }
> -EXPORT_SYMBOL(intel_gtt_map_memory);
>  
> -void intel_gtt_unmap_memory(struct scatterlist *sg_list, int num_sg)
> +static void intel_gtt_unmap_memory(struct scatterlist *sg_list, int num_sg)
>  {
>   struct sg_table st;
>   DBG("try unmapping %lu pages\n", (unsigned long)mem->page_count);
> @@ -130,7 +123,6 @@ void intel_gtt_unmap_memory(struct scatterlist *sg_list, 
> int num_sg)
>  
>   sg_free_table(&st);
>  }
> -EXPORT_SYMBOL(intel_gtt_unmap_memory);
>  
>  static void intel_fake_agp_enable(struct agp_bridge_data *bridge, u32 mode)
>  {
> @@ -879,8 +871,7 @@ static bool i830_check_flags(unsigned int flags)
>   return false;
>  }
>  
> -void intel_gtt_insert_sg_entries(struct scatterlist *sg_list,
> -  unsigned int sg_le

Re: [Intel-gfx] [PATCH 00/58] modeset-rework, the basic conversion

2012-09-06 Thread Jesse Barnes
On Wed, 5 Sep 2012 16:23:55 -0700
Jesse Barnes  wrote:

> On Sun, 19 Aug 2012 21:12:17 +0200
> Daniel Vetter  wrote:
> 
> > Hi all,
> > 
> > Changes since last time around:
> > - The prep patches are all merged now.
> > - I've left out the actual DP fixes/cleanups, I think we should merge those 
> > in a
> >   separte step.
> > - A few bugfixes (thanks to Paulo, Jani and Chris).
> > - I've also applied a few bikesheds for naming that Paulo suggested (but 
> > I'm not
> >   sure whether I've sent those out already in a previous patchbomb).
> > 
> > Essentially this is just the core rework, which addes the new get_hw_state 
> > code,
> > refactors all the encoders to use the new functions and finally reworks the
> > modeset logic to disable/enable entire pipes, always (and with a 
> > deterministic
> > order).
> > 
> > For merging to -next, I plan to pull in everything with a real merge 
> > commit. For
> > that reason I've put up a modeset-rework-base branch onto my private fdo 
> > repo[1].
> > That way I can put a short documentation for the new modeset design into the
> > merge commit (stichted together from the previous patchbomb cover letters),
> > documenting my folly assumptions for eternity.
> > 
> > I'll also plan to put tags for the entire series in the merge commit, so if 
> > you
> > have tested this on a few machines, read through and agree with the new 
> > designs,
> > please reply with your tested-by/acked-by/reviewed-by tags.
> > 
> > Flames, comments and test reports highly welcome.
> 
> Ok I've tested on Ironlake, Cantiga, Crestline, and Pineview so far and
> things look good.  I ran testdisplay both with and without VGA
> attached (the ILK has a eDP panel), and tried S3 and S4 both with and
> without VGA both in the console and in X.
> 
> There was one issue on Pineview where the VGA seemed to get
> "forgotten", but I haven't isolated it yet.  I canceled testdisplay
> part way through and that seemed to confuse fbcon about what was there.
> 
> I also see an issue with 1280x800 modes across all platforms, but that
> may just be the monitor, I need to test more.
> 
> I'm testing Montara (that's 855 for you youngsters) now, but it's slow
> so not all my builds have completed there yet.
> 
> Overall though:
> Tested-by: Jesse Barnes 
> 

Looks like I just found an issue with DPMS on fbcon on SNB though.
Leaving the system for awhile either results in corruption (part of the
fbcon on the screen) or just the screen left on.

This is on LVDS.

-- 
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 04/24] drm/i915: Pin backing pages for pread

2012-09-06 Thread Ben Widawsky
On Tue,  4 Sep 2012 21:02:56 +0100
Chris Wilson  wrote:

> By using the recently introduced pinning of pages, we can safely drop
> the mutex in the knowledge that the pages are not going to disappear
> beneath us, and so we can simplify the code for iterating over the pages.
> 
> Signed-off-by: Chris Wilson 
If you fix patch3 to explain, it explain here too. Write is always
scarier for me anyway. With fixed patch 3, it's
Reviewed-by: Ben Widawsky 
-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/24] drm/i915: Pin backing pages for pwrite

2012-09-06 Thread Ben Widawsky
On Tue,  4 Sep 2012 21:02:55 +0100
Chris Wilson  wrote:

> By using the recently introduced pinning of pages, we can safely drop
> the mutex in the knowledge that the pages are not going to disappear
> beneath us, and so we can simplify the code for iterating over the pages.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   37 +
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index aa088ef..8a4eac0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -690,7 +690,7 @@ shmem_pwrite_fast(struct page *page, int 
> shmem_page_offset, int page_length,
>  page_length);
>   kunmap_atomic(vaddr);
>  
> - return ret;
> + return ret ? -EFAULT : 0;
>  }
>  
>  /* Only difference to the fast-path function is that this can handle bit17
> @@ -724,7 +724,7 @@ shmem_pwrite_slow(struct page *page, int 
> shmem_page_offset, int page_length,
>page_do_bit17_swizzling);
>   kunmap(page);
>  
> - return ret;
> + return ret ? -EFAULT : 0;
>  }
>  
>  static int
> @@ -733,7 +733,6 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
> struct drm_i915_gem_pwrite *args,
> struct drm_file *file)
>  {
> - struct address_space *mapping = 
> obj->base.filp->f_path.dentry->d_inode->i_mapping;
>   ssize_t remain;
>   loff_t offset;
>   char __user *user_data;

Without digging to deep to see if you looked already. It would be nice
if we can get a DRM_INFO or something for cases where return isn't
actually EFAULT.

> @@ -742,7 +741,6 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>   int hit_slowpath = 0;
>   int needs_clflush_after = 0;
>   int needs_clflush_before = 0;
> - int release_page;
>  
>   user_data = (char __user *) (uintptr_t) args->data_ptr;
>   remain = args->size;
> @@ -768,6 +766,12 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>   && obj->cache_level == I915_CACHE_NONE)
>   needs_clflush_before = 1;
>  
> + ret = i915_gem_object_get_pages(obj);
> + if (ret)
> + return ret;
> +
> + i915_gem_object_pin_pages(obj);
> +
>   offset = args->offset;
>   obj->dirty = 1;
>  
> @@ -793,18 +797,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>   ((shmem_page_offset | page_length)
>   & (boot_cpu_data.x86_clflush_size - 1));
>  
> - if (obj->pages) {
> - page = obj->pages[offset >> PAGE_SHIFT];
> - release_page = 0;
> - } else {
> - page = shmem_read_mapping_page(mapping, offset >> 
> PAGE_SHIFT);
> - if (IS_ERR(page)) {
> - ret = PTR_ERR(page);
> - goto out;
> - }
> - release_page = 1;
> - }
> -
> + page = obj->pages[offset >> PAGE_SHIFT];
>   page_do_bit17_swizzling = obj_do_bit17_swizzling &&
>   (page_to_phys(page) & (1 << 17)) != 0;
>  

So the obvious question is what about the page caching? Can you add to
the commit message for my edification why previously the shmem page is
released from the page cache and now it isn't?

> @@ -816,26 +809,20 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>   goto next_page;
>  
>   hit_slowpath = 1;
> - page_cache_get(page);
>   mutex_unlock(&dev->struct_mutex);
> -
>   ret = shmem_pwrite_slow(page, shmem_page_offset, page_length,
>   user_data, page_do_bit17_swizzling,
>   partial_cacheline_write,
>   needs_clflush_after);
>  
>   mutex_lock(&dev->struct_mutex);
> - page_cache_release(page);
> +
>  next_page:
>   set_page_dirty(page);
>   mark_page_accessed(page);
> - if (release_page)
> - page_cache_release(page);
>  
> - if (ret) {
> - ret = -EFAULT;
> + if (ret)
>   goto out;
> - }
>  
>   remain -= page_length;
>   user_data += page_length;
> @@ -843,6 +830,8 @@ next_page:
>   }
>  
>  out:
> + i915_gem_object_unpin_pages(obj);
> +
>   if (hit_slowpath) {
>   /* Fixup: Kill any reinstated backing storage pages */
>   if (obj->madv == __I915_MADV_PURGED)



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


Re: [Intel-gfx] [PATCH 4/7 v2] drm/i915: Add current GPU freq to sysfs

2012-09-06 Thread Ben Widawsky

On 2012-09-06 14:09, Michael Larabel wrote:

Have you discussed this with Radeon/Nouveau to see if they would
adopt a common sysfs interface? In the past, Eugeni Dodonov and I had
talked about this with a desire to have the open-source drivers 
expose

the useful information for monitoring in a similar manner for helping
userspace (current/max/min clock speeds, voltage, etc). [My personal
interest is from monitoring this information in a more standardized
way from the Phoronix Test Suite system monitoring components rather
than writing driver-specific paths like it is now.]

Martin (CCed) has also expressed interest from the Nouveau side in
calling for some standardization.

-- Michael


We can... I did very briefly look at what Nouveau does with the hwmon 
interface, and what DRM provided, and thought it'd be more work than I 
wanted to sign up for (which is just about nothing sysfs other than 
display). Thanks for bringing him into the thread though.




On 09/06/2012 03:54 PM, Ben Widawsky wrote:
Userspace applications such as PowerTOP are interesting in being 
able to
read the current GPU frequency. The patch itself sets up a generic 
array
for gen6 attributes so we can easily add other items in the future 
(and

it also happens to be just about the cleanest way to do this).

The patch is a nice addition to
commit 1ac02185dff3afac146d745ba220dc6672d1d162
Author: Daniel Vetter 
Date:   Thu Aug 30 13:26:48 2012 +0200

 drm/i915: add a tracepoint for gpu frequency changes

Reading the GPU frequncy can be done by reading a file like:
/sys/class/drm/card0/render_frequency_mhz

v2: rename the sysfs file to gt_cur_freq_mhz (Daniel)
Use kdev naming (Chris)
Use DEVICE_ATTR over __ATTR (Ben)
Add min/max freq (Ben/Daniel)
user the new #define for frequency multiplier

CC: Arjan van de Ven 
CC: Jacob Pan 
Signed-off-by: Ben Widawsky 
---
  drivers/gpu/drm/i915/i915_sysfs.c | 71 
+++

  1 file changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c

index c701a17..6c7f905 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -203,6 +203,70 @@ static struct bin_attribute dpf_attrs = {
.mmap = NULL
  };
  +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
+   struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, 
kdev);

+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
+   mutex_unlock(&dev->struct_mutex);
+
+   return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)

+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, 
kdev);

+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
+   mutex_unlock(&dev->struct_mutex);
+
+   return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)

+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, 
kdev);

+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ret = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
+   mutex_unlock(&dev->struct_mutex);
+
+   return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, 
NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, 
gt_max_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, 
gt_min_freq_mhz_show, NULL);

+
+static const struct attribute *gen6_attrs[] = {
+   &dev_attr_gt_cur_freq_mhz.attr,
+   &dev_attr_gt_max_freq_mhz.attr,
+   &dev_attr_gt_min_freq_mhz.attr,
+   NULL,
+};
+
+
  void i915_setup_sysfs(struct drm_device *dev)
  {
int ret;
@@ -220,10 +284,17 @@ void i915_setup_sysfs(struct drm_device *dev)
if (ret)
DRM_ERROR("l3 parity sysfs setup failed\n");
}
+
+   if (INTEL_INFO(dev)->gen >= 6) {
+   ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs);
+   if (ret)
+   DRM_ERROR("gen6 sysfs setup failed\n");
+   }
  }
  void i915_teardown_sysfs(struct drm_device *dev)
  {
+   sysfs_remove_file

Re: [Intel-gfx] [PATCH 02/24] drm/i915: Pin backing pages whilst exporting through a dmabuf vmap

2012-09-06 Thread Ben Widawsky
On Tue,  4 Sep 2012 21:02:54 +0100
Chris Wilson  wrote:

> We need to refcount our pages in order to prevent reaping them at
> inopportune times, such as when they currently vmapped or exported to
> another driver. However, we also wish to keep the lazy deallocation of
> our pages so we need to take a pin/unpinned approach rather than a
> simple refcount.

I've not followed the dmabuf development much but is there no interface
to map partial objects, ie. have some pages of an object pinned, but not
all?

> 
> Signed-off-by: Chris Wilson 

With comment below addressed or not it's:
Reviewed-by: Ben Widawsky 

> ---
>  drivers/gpu/drm/i915/i915_drv.h|   12 
>  drivers/gpu/drm/i915/i915_gem.c|   11 +--
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c |8 ++--
>  3 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f180874..0747472 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -994,6 +994,7 @@ struct drm_i915_gem_object {
>   unsigned int has_global_gtt_mapping:1;
>  
>   struct page **pages;
> + int pages_pin_count;
>  
>   /**
>* DMAR support
> @@ -1327,6 +1328,17 @@ void i915_gem_release_mmap(struct drm_i915_gem_object 
> *obj);
>  void i915_gem_lastclose(struct drm_device *dev);
>  
>  int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
> +static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
> +{
> + BUG_ON(obj->pages == NULL);
> + obj->pages_pin_count++;
> +}
> +static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object 
> *obj)
> +{
> + BUG_ON(obj->pages_pin_count == 0);
> + obj->pages_pin_count--;
> +}
> +

Big fan of BUG_ON(!mutex_is_locked()) here.

>  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
>  int i915_gem_object_sync(struct drm_i915_gem_object *obj,
>struct intel_ring_buffer *to);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 66fbd9f..aa088ef 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1699,6 +1699,9 @@ i915_gem_object_put_pages(struct drm_i915_gem_object 
> *obj)
>  
>   BUG_ON(obj->gtt_space);
>  
> + if (obj->pages_pin_count)
> + return -EBUSY;
> +
>   ops->put_pages(obj);
>  
>   list_del(&obj->gtt_list);
> @@ -1830,6 +1833,8 @@ i915_gem_object_get_pages(struct drm_i915_gem_object 
> *obj)
>   if (obj->sg_table || obj->pages)
>   return 0;
>  
> + BUG_ON(obj->pages_pin_count);
> +
>   ret = ops->get_pages(obj);
>   if (ret)
>   return ret;
> @@ -3736,6 +3741,7 @@ void i915_gem_free_object(struct drm_gem_object 
> *gem_obj)
>   dev_priv->mm.interruptible = was_interruptible;
>   }
>  
> + obj->pages_pin_count = 0;
>   i915_gem_object_put_pages(obj);
>   i915_gem_object_free_mmap_offset(obj);
>  
> @@ -4395,9 +4401,10 @@ i915_gem_inactive_shrink(struct shrinker *shrinker, 
> struct shrink_control *sc)
>  
>   cnt = 0;
>   list_for_each_entry(obj, &dev_priv->mm.unbound_list, gtt_list)
> - cnt += obj->base.size >> PAGE_SHIFT;
> + if (obj->pages_pin_count == 0)
> + cnt += obj->base.size >> PAGE_SHIFT;
>   list_for_each_entry(obj, &dev_priv->mm.bound_list, gtt_list)
> - if (obj->pin_count == 0)
> + if (obj->pin_count == 0 && obj->pages_pin_count == 0)
>   cnt += obj->base.size >> PAGE_SHIFT;
>  
>   mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
> b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index e4f1141..eca4726 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -50,6 +50,8 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
> dma_buf_attachment *attachme
>   /* link the pages into an SG then map the sg */
>   sg = drm_prime_pages_to_sg(obj->pages, npages);
>   nents = dma_map_sg(attachment->dev, sg->sgl, sg->nents, dir);
> + i915_gem_object_pin_pages(obj);
> +
>  out:
>   mutex_unlock(&dev->struct_mutex);
>   return sg;
> @@ -102,6 +104,7 @@ static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
>   }
>  
>   obj->vmapping_count = 1;
> + i915_gem_object_pin_pages(obj);
>  out_unlock:
>   mutex_unlock(&dev->struct_mutex);
>   return obj->dma_buf_vmapping;
> @@ -117,10 +120,11 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf 
> *dma_buf, void *vaddr)
>   if (ret)
>   return;
>  
> - --obj->vmapping_count;
> - if (obj->vmapping_count == 0) {
> + if (--obj->vmapping_count == 0) {
>   vunmap(obj->dma_buf_vmapping);
>   obj->dma_buf_vmapping = NULL;
> +
> + i915_gem_object_unpin_pag

Re: [Intel-gfx] [PATCH 01/24] drm/i915: Introduce drm_i915_gem_object_ops

2012-09-06 Thread Ben Widawsky
On Tue,  4 Sep 2012 21:02:53 +0100
Chris Wilson  wrote:

> In order to specialise functions depending upon the type of object, we
> can attach vfuncs to each object via a new ->ops pointer.
> 
> For instance, this will be used in future patches to only bind pages from
> a dma-buf for the duration that the object is used by the GPU - and so
> prevent them from pinning those pages for the entire of the object.
> 
> Signed-off-by: Chris Wilson 
With comments below (addressed or not) this is:
Reviewed-by: Ben Widawsky 

> ---
>  drivers/gpu/drm/i915/i915_drv.h|   12 +-
>  drivers/gpu/drm/i915/i915_gem.c|   71 
> +---
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c |4 +-
>  3 files changed, 60 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f16ab5e..f180874 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -898,9 +898,16 @@ enum i915_cache_level {
>   I915_CACHE_LLC_MLC, /* gen6+, in docs at least! */
>  };
>  
> +struct drm_i915_gem_object_ops {
> + int (*get_pages)(struct drm_i915_gem_object *);
> + void (*put_pages)(struct drm_i915_gem_object *);
> +};
> +

For the sake of "namespace" can we name this gem_get_pages() or
something similar?

I would love comments describing what get/put should do for when people
other than you want to add new ops. If you'd prefer to put them in the
definitions of the get/put functions, that's even better.

>  struct drm_i915_gem_object {
>   struct drm_gem_object base;
>  
> + const struct drm_i915_gem_object_ops *ops;
> +
>   /** Current space allocated to this object in the GTT, if any. */
>   struct drm_mm_node *gtt_space;
>   struct list_head gtt_list;
> @@ -1305,7 +1312,8 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void 
> *data,
>   struct drm_file *file_priv);
>  void i915_gem_load(struct drm_device *dev);
>  int i915_gem_init_object(struct drm_gem_object *obj);
> -void i915_gem_object_init(struct drm_i915_gem_object *obj);
> +void i915_gem_object_init(struct drm_i915_gem_object *obj,
> +  const struct drm_i915_gem_object_ops *ops);
>  struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> size_t size);
>  void i915_gem_free_object(struct drm_gem_object *obj);
> @@ -1318,7 +1326,7 @@ int __must_check i915_gem_object_unbind(struct 
> drm_i915_gem_object *obj);
>  void i915_gem_release_mmap(struct drm_i915_gem_object *obj);
>  void i915_gem_lastclose(struct drm_device *dev);
>  
> -int __must_check i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
> *obj);
> +int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
>  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
>  int i915_gem_object_sync(struct drm_i915_gem_object *obj,
>struct intel_ring_buffer *to);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 87a64e5..66fbd9f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1650,18 +1650,12 @@ i915_gem_object_is_purgeable(struct 
> drm_i915_gem_object *obj)
>   return obj->madv == I915_MADV_DONTNEED;
>  }
>  
> -static int
> +static void
>  i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj)
>  {
>   int page_count = obj->base.size / PAGE_SIZE;
>   int ret, i;
>  
> - BUG_ON(obj->gtt_space);
> -
> - if (obj->pages == NULL)
> - return 0;
> -
> - BUG_ON(obj->gtt_space);
>   BUG_ON(obj->madv == __I915_MADV_PURGED);
>  
>   ret = i915_gem_object_set_to_cpu_domain(obj, true);
> @@ -1693,9 +1687,21 @@ i915_gem_object_put_pages_gtt(struct 
> drm_i915_gem_object *obj)
>  
>   drm_free_large(obj->pages);
>   obj->pages = NULL;
> +}
>  
> - list_del(&obj->gtt_list);
> +static int
> +i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
> +{
> + const struct drm_i915_gem_object_ops *ops = obj->ops;
> +
> + if (obj->sg_table || obj->pages == NULL)
> + return 0;
> +
> + BUG_ON(obj->gtt_space);
>  
> + ops->put_pages(obj);
> +
> + list_del(&obj->gtt_list);
>   if (i915_gem_object_is_purgeable(obj))
>   i915_gem_object_truncate(obj);
>  
> @@ -1712,7 +1718,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long 
> target)
>&dev_priv->mm.unbound_list,
>gtt_list) {
>   if (i915_gem_object_is_purgeable(obj) &&
> - i915_gem_object_put_pages_gtt(obj) == 0) {
> + i915_gem_object_put_pages(obj) == 0) {
>   count += obj->base.size >> PAGE_SHIFT;
>   if (count >= target)
>   return count;
> @@ -1724,7 +1730,7 @@ i915_gem_purge(struct drm_i915_p

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Show render P state thresholds in sysfs

2012-09-06 Thread Jesse Barnes
On Thu,  6 Sep 2012 13:54:09 -0700
Ben Widawsky  wrote:

> This is useful for userspace utilities which wish to use the previous
> interface, specifically for micromanaging the increase/decrease steps by
> setting min == max.
> 
> CC: Jacob Pan 
> CC: Jesse Barnes 
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 36 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
> b/drivers/gpu/drm/i915/i915_sysfs.c
> index 5a5e610..e3a31ae 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -333,10 +333,46 @@ static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, 
> gt_cur_freq_mhz_show, NULL);
>  static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
> gt_max_freq_mhz_store);
>  static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
> gt_min_freq_mhz_store);
>  
> +
> +static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute 
> *attr, char *buf);
> +static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> +static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> +static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> +
> +/* For now we have a static number of RP states */
> +static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute 
> *attr, char *buf)
> +{
> + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
> + struct drm_device *dev = minor->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 val, rp_state_cap;
> + ssize_t ret;
> +
> + ret = mutex_lock_interruptible(&dev->struct_mutex);
> + if (ret)
> + return ret;
> + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> + mutex_unlock(&dev->struct_mutex);
> +
> + if (attr == &dev_attr_gt_RP0_freq_mhz) {
> + val = ((rp_state_cap & 0xff) >> 0) * 
> GT_FREQUENCY_MULTIPLIER;
> + } else if (attr == &dev_attr_gt_RP1_freq_mhz) {
> + val = ((rp_state_cap & 0x00ff00) >> 8) * 
> GT_FREQUENCY_MULTIPLIER;
> + } else if (attr == &dev_attr_gt_RPn_freq_mhz) {
> + val = ((rp_state_cap & 0xff) >> 16) * 
> GT_FREQUENCY_MULTIPLIER;
> + } else {
> + BUG();
> + }
> + return snprintf(buf, PAGE_SIZE, "%d", val);
> +}
> +
>  static const struct attribute *gen6_attrs[] = {
>   &dev_attr_gt_cur_freq_mhz.attr,
>   &dev_attr_gt_max_freq_mhz.attr,
>   &dev_attr_gt_min_freq_mhz.attr,
> + &dev_attr_gt_RP0_freq_mhz.attr,
> + &dev_attr_gt_RP1_freq_mhz.attr,
> + &dev_attr_gt_RPn_freq_mhz.attr,
>   NULL,
>  };
>  

If you add documentation:
Reviewed-by: Jesse Barnes 

-- 
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 5/7] drm/i915: Add setters for min/max frequency

2012-09-06 Thread Jesse Barnes
On Thu,  6 Sep 2012 13:54:08 -0700
Ben Widawsky  wrote:

> Provide a standardized sysfs interface for setting min, and max
> frequencies.  The code which reads the limits were lifted from the
> debugfs files. As a brief explanation, the limits are similar to the CPU
> p-states. We have 3 states:
> 
> RP0 - ie. max frequency
> RP1 - ie. "local min" frequency

I'd say "preferred min" or "vmin" here instead.

> RPn - seriously lowest frequency
> 
> Initially Daniel asked me to clamp the writes to supported values, but
> in conforming to the way the cpufreq drivers seem to work, instead
> return -EINVAL (noticed by Jesse in discussion).
> 
> The values can be used by userspace wishing to control the limits of the
> GPU (see the CC list for people who care).

Can you add this and any other i915 bits that are missing to the sysfs
documentation under Documentation/?

Thanks for moving to -EINVAL, it makes a lot more sense to me.

-- 
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 3/7] drm/i915: #define gpu freq multipler

2012-09-06 Thread Jesse Barnes
On Thu,  6 Sep 2012 13:54:06 -0700
Ben Widawsky  wrote:

> Magic numbers are bad mmmkay. In this case in particular the value is
> especially weird because the docs say multiple things. We'll need this
> value for sysfs, so extracting it is useful for that as well.

Reviewed-by: Jesse Barnes 

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


[Intel-gfx] [PATCH 5/5] drm/i915: disable the cpu edp port after the cpu pipe

2012-09-06 Thread Daniel Vetter
See bspec, Vol3 Part2, Section 1.1.3 "Display Mode Set Sequence". This
applies to all platforms where we currently support eDP on, i.e. ilk,
snb & ivb.

Without this change we fail to light up the eDP port on previously
unused crtcs (likely because something is stuck on the old pipe), and
we also fail to properly disable the old pipe (i.e. bit 30 in the
PIPECONF register is stuck as set until the next reboot).

v2: Rebased on top of the edp panel off sequence changes in 3.6-rc2.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=44001
Signed-Off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_dp.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 07f9521..f28353d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1315,15 +1315,20 @@ static void intel_disable_dp(struct intel_encoder 
*encoder)
ironlake_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
ironlake_edp_panel_off(intel_dp);
-   intel_dp_link_down(intel_dp);
+
+   /* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */
+   if (!is_cpu_edp(intel_dp))
+   intel_dp_link_down(intel_dp);
 }
 
 static void intel_post_disable_dp(struct intel_encoder *encoder)
 {
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
-   if (is_cpu_edp(intel_dp))
+   if (is_cpu_edp(intel_dp)) {
+   intel_dp_link_down(intel_dp);
ironlake_edp_pll_off(intel_dp);
+   }
 }
 
 static void intel_enable_dp(struct intel_encoder *encoder)
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 4/5] drm/i915: rip out dp port enabling cludges^Wchecks

2012-09-06 Thread Daniel Vetter
These have been added because dp links are fiddle things and don't
like it when we try to re-train an enabled output (or disable a
disabled output harder). And because the crtc helper code is
ridiculously bad add tracking the modeset state.

But with the new code in place it is simply a bug to disable a disabled
encoder or to enable an enabled encoder again. Hence convert these to
WARNs (and bail out for safety), but flatten all conditionals in the
code itself.

Signed-Off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_dp.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7c746d1..07f9521 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1333,15 +1333,15 @@ static void intel_enable_dp(struct intel_encoder 
*encoder)
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t dp_reg = I915_READ(intel_dp->output_reg);
 
+   if (WARN_ON(dp_reg & DP_PORT_EN))
+   return;
+
ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
-   if (!(dp_reg & DP_PORT_EN)) {
-   intel_dp_start_link_train(intel_dp);
-   ironlake_edp_panel_on(intel_dp);
-   ironlake_edp_panel_vdd_off(intel_dp, true);
-   intel_dp_complete_link_train(intel_dp);
-   } else
-   ironlake_edp_panel_vdd_off(intel_dp, false);
+   intel_dp_start_link_train(intel_dp);
+   ironlake_edp_panel_on(intel_dp);
+   ironlake_edp_panel_vdd_off(intel_dp, true);
+   intel_dp_complete_link_train(intel_dp);
ironlake_edp_backlight_on(intel_dp);
 }
 
@@ -1913,7 +1913,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t DP = intel_dp->DP;
 
-   if ((I915_READ(intel_dp->output_reg) & DP_PORT_EN) == 0)
+   if (WARN_ON((I915_READ(intel_dp->output_reg) & DP_PORT_EN) == 0))
return;
 
DRM_DEBUG_KMS("\n");
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 3/5] drm/i915: robustify edp_pll_on/off

2012-09-06 Thread Daniel Vetter
With the previous patch to clean up where exactly these two functions
are getting called, this patch can tackle the enable/disable code
itself:

- WARN if the port enable bit is in the wrong state or if the edp pll
  bit is in the wrong state, just for paranoia's sake.
- Don't disable the edp pll harder in the modeset functions just for
  fun.
- Don't set the edp pll enable flag in intel_dp->DP in modeset, do
  that while changing the actual hw state. We do the same with the
  actual port enable bit, so this is a bit more consistent.
- Track the current DP register value when setting things up and add
  some comments how intel_dp->DP is used in the disable code.

v2: Be more careful with resetting intel_dp->DP - otherwise dpms
off->on will fail spectacularly, becuase we enable the eDP port when
we should only enable the eDP pll.

Signed-Off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_dp.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c72d4f3..7c746d1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -887,7 +887,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
intel_dp->DP |= intel_crtc->pipe << 29;
 
/* don't miss out required setting for eDP */
-   intel_dp->DP |= DP_PLL_ENABLE;
if (adjusted_mode->clock < 20)
intel_dp->DP |= DP_PLL_FREQ_160MHZ;
else
@@ -909,7 +908,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
 
if (is_cpu_edp(intel_dp)) {
/* don't miss out required setting for eDP */
-   intel_dp->DP |= DP_PLL_ENABLE;
if (adjusted_mode->clock < 20)
intel_dp->DP |= DP_PLL_FREQ_160MHZ;
else
@@ -1192,8 +1190,15 @@ static void ironlake_edp_pll_on(struct intel_dp 
*intel_dp)
 
DRM_DEBUG_KMS("\n");
dpa_ctl = I915_READ(DP_A);
-   dpa_ctl |= DP_PLL_ENABLE;
-   I915_WRITE(DP_A, dpa_ctl);
+   WARN(dpa_ctl & DP_PLL_ENABLE, "dp pll on, should be off\n");
+   WARN(dpa_ctl & DP_PORT_EN, "dp port still on, should be off\n");
+
+   /* We don't adjust intel_dp->DP while tearing down the link, to
+* facilitate link retraining (e.g. after hotplug). Hence clear all
+* enable bits here to ensure that we don't enable too much. */
+   intel_dp->DP &= ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
+   intel_dp->DP |= DP_PLL_ENABLE;
+   I915_WRITE(DP_A, intel_dp->DP);
POSTING_READ(DP_A);
udelay(200);
 }
@@ -1209,6 +1214,13 @@ static void ironlake_edp_pll_off(struct intel_dp 
*intel_dp)
 to_intel_crtc(crtc)->pipe);
 
dpa_ctl = I915_READ(DP_A);
+   WARN((dpa_ctl & DP_PLL_ENABLE) == 0,
+"dp pll off, should be on\n");
+   WARN(dpa_ctl & DP_PORT_EN, "dp port still on, should be off\n");
+
+   /* We can't rely on the value tracked for the DP register in
+* intel_dp->DP because link_down must not change that (otherwise link
+* re-training will fail. */
dpa_ctl &= ~DP_PLL_ENABLE;
I915_WRITE(DP_A, dpa_ctl);
POSTING_READ(DP_A);
@@ -1906,13 +1918,6 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 
DRM_DEBUG_KMS("\n");
 
-   if (is_edp(intel_dp)) {
-   DP &= ~DP_PLL_ENABLE;
-   I915_WRITE(intel_dp->output_reg, DP);
-   POSTING_READ(intel_dp->output_reg);
-   udelay(100);
-   }
-
if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp))) {
DP &= ~DP_LINK_TRAIN_MASK_CPT;
I915_WRITE(intel_dp->output_reg, DP | 
DP_LINK_TRAIN_PAT_IDLE_CPT);
@@ -2456,6 +2461,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, 
enum port port)
 
intel_dp->output_reg = output_reg;
intel_dp->port = port;
+   /* Preserve the current hw state. */
+   intel_dp->DP = I915_READ(intel_dp->output_reg);
 
intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
if (!intel_connector) {
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 2/5] drm/i915: clean up the cpu edp pll special case

2012-09-06 Thread Daniel Vetter
By using the new pre_enabel/post_disable functions.

To ensure that we only frob the cpu edp pll while the pipe is off add
the relevant asserts. Thanks to the new output state staging, this is
now really easy.

Signed-Off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_dp.c | 74 +++--
 1 file changed, 27 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c59710d..c72d4f3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -808,9 +808,6 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct 
drm_display_mode *mode,
}
 }
 
-static void ironlake_edp_pll_on(struct drm_encoder *encoder);
-static void ironlake_edp_pll_off(struct drm_encoder *encoder);
-
 static void
 intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
  struct drm_display_mode *adjusted_mode)
@@ -821,14 +818,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
struct drm_crtc *crtc = intel_dp->base.base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-   /* Turn on the eDP PLL if needed */
-   if (is_edp(intel_dp)) {
-   if (!is_pch_edp(intel_dp))
-   ironlake_edp_pll_on(encoder);
-   else
-   ironlake_edp_pll_off(encoder);
-   }
-
/*
 * There are four kinds of DP registers:
 *
@@ -1191,12 +1180,16 @@ static void ironlake_edp_backlight_off(struct intel_dp 
*intel_dp)
msleep(intel_dp->backlight_off_delay);
 }
 
-static void ironlake_edp_pll_on(struct drm_encoder *encoder)
+static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
 {
-   struct drm_device *dev = encoder->dev;
+   struct drm_device *dev = intel_dp->base.base.dev;
+   struct drm_crtc *crtc = intel_dp->base.base.crtc;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 dpa_ctl;
 
+   assert_pipe_disabled(dev_priv,
+to_intel_crtc(crtc)->pipe);
+
DRM_DEBUG_KMS("\n");
dpa_ctl = I915_READ(DP_A);
dpa_ctl |= DP_PLL_ENABLE;
@@ -1205,12 +1198,16 @@ static void ironlake_edp_pll_on(struct drm_encoder 
*encoder)
udelay(200);
 }
 
-static void ironlake_edp_pll_off(struct drm_encoder *encoder)
+static void ironlake_edp_pll_off(struct intel_dp *intel_dp)
 {
-   struct drm_device *dev = encoder->dev;
+   struct drm_device *dev = intel_dp->base.base.dev;
+   struct drm_crtc *crtc = intel_dp->base.base.crtc;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 dpa_ctl;
 
+   assert_pipe_disabled(dev_priv,
+to_intel_crtc(crtc)->pipe);
+
dpa_ctl = I915_READ(DP_A);
dpa_ctl &= ~DP_PLL_ENABLE;
I915_WRITE(DP_A, dpa_ctl);
@@ -1309,6 +1306,14 @@ static void intel_disable_dp(struct intel_encoder 
*encoder)
intel_dp_link_down(intel_dp);
 }
 
+static void intel_post_disable_dp(struct intel_encoder *encoder)
+{
+   struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+   if (is_cpu_edp(intel_dp))
+   ironlake_edp_pll_off(intel_dp);
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -1328,39 +1333,12 @@ static void intel_enable_dp(struct intel_encoder 
*encoder)
ironlake_edp_backlight_on(intel_dp);
 }
 
-static void
-intel_dp_dpms(struct drm_connector *connector, int mode)
+static void intel_pre_enable_dp(struct intel_encoder *encoder)
 {
-   struct intel_dp *intel_dp = intel_attached_dp(connector);
-
-   /* DP supports only 2 dpms states. */
-   if (mode != DRM_MODE_DPMS_ON)
-   mode = DRM_MODE_DPMS_OFF;
-
-   if (mode == connector->dpms)
-   return;
-
-   connector->dpms = mode;
-
-   /* Only need to change hw state when actually enabled */
-   if (!intel_dp->base.base.crtc) {
-   intel_dp->base.connectors_active = false;
-   return;
-   }
-
-   if (mode != DRM_MODE_DPMS_ON) {
-   intel_encoder_dpms(&intel_dp->base, mode);
-
-   if (is_cpu_edp(intel_dp))
-   ironlake_edp_pll_off(&intel_dp->base.base);
-   } else {
-   if (is_cpu_edp(intel_dp))
-   ironlake_edp_pll_on(&intel_dp->base.base);
-
-   intel_encoder_dpms(&intel_dp->base, mode);
-   }
+   struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
-   intel_modeset_check_state(connector->dev);
+   if (is_cpu_edp(intel_dp))
+   ironlake_edp_pll_on(intel_dp);
 }
 
 /*
@@ -2391,7 +2369,7 @@ static const struct drm_encoder_helper_funcs 
intel_dp_helper_funcs = {
 };
 
 static const struct drm_connector_funcs intel_dp_connector_funcs = {
-   .dpms = intel_dp_dpms,
+   .dpms = intel_co

[Intel-gfx] [PATCH 1/5] drm/i915: add encoder->pre_enable/post_disable

2012-09-06 Thread Daniel Vetter
The cpu eDP encoder has some horrible hacks to set up the DP pll at
the right time. To be able to move them to the right place, add some
more encoder callbacks so that this can happen at the right time.

LVDS has some similar funky hacks, but that would require more work
(we need to move around the pll setup a bit). Hence for now only
wire these new callbacks up for ilk+ - we only have cpu eDP on these
platforms.

Signed-Off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 8 
 drivers/gpu/drm/i915/intel_drv.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bff0936..1d31364 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3189,6 +3189,10 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
assert_fdi_rx_disabled(dev_priv, pipe);
}
 
+   for_each_encoder_on_crtc(dev, crtc, encoder)
+   if (encoder->pre_enable)
+   encoder->pre_enable(encoder);
+
/* Enable panel fitting for LVDS */
if (dev_priv->pch_pf_size &&
(intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) || HAS_eDP)) {
@@ -3258,6 +3262,10 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
I915_WRITE(PF_CTL(pipe), 0);
I915_WRITE(PF_WIN_SZ(pipe), 0);
 
+   for_each_encoder_on_crtc(dev, crtc, encoder)
+   if (encoder->post_disable)
+   encoder->post_disable(encoder);
+
ironlake_fdi_disable(crtc);
 
intel_disable_transcoder(dev_priv, pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4f2b2d6..1306f05 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -153,7 +153,9 @@ struct intel_encoder {
bool connectors_active;
void (*hot_plug)(struct intel_encoder *);
void (*enable)(struct intel_encoder *);
+   void (*pre_enable)(struct intel_encoder *);
void (*disable)(struct intel_encoder *);
+   void (*post_disable)(struct intel_encoder *);
/* Read out the current hw state of this connector, returning true if
 * the encoder is active. If the encoder is enabled it also set the pipe
 * it is connected to in the pipe parameter. */
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 0/5] cpu edp fixes

2012-09-06 Thread Daniel Vetter
Hi all!

Finally.

Now that the modeset-rework is merge I can finally dump the cpu edp fixes and
cleanups. Avoids my ivb edp from getting angry and leaving random pipes stuck in
the active state and the panel in the passive state.

Again, has been part of the modeset-rework branch since a longer time, so has
seen tons of testing.

Flames, comments & review highly welcome.

Cheers, Daniel

Daniel Vetter (5):
  drm/i915: add encoder->pre_enable/post_disable
  drm/i915: clean up the cpu edp pll special case
  drm/i915: robustify edp_pll_on/off
  drm/i915: rip out dp port enabling cludges^Wchecks
  drm/i915: disable the cpu edp port after the cpu pipe

 drivers/gpu/drm/i915/intel_display.c |   8 +++
 drivers/gpu/drm/i915/intel_dp.c  | 126 ---
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 3 files changed, 69 insertions(+), 67 deletions(-)

-- 
1.7.11.2

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


[Intel-gfx] [PATCH] drm/i915: fixup the plane->pipe fixup code

2012-09-06 Thread Daniel Vetter
We need to check whether the _other plane is on our pipe, not whether
our plane is on the other pipe. Otherwise if not both pipes/planes are
active, we won't properly clean up the mess and set up our desired
plane->pipe mapping.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
Tested-by: Vladyslav Shtabovenko 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fd9c275..ce9b247 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
 
 }
 
+static bool
+intel_check_plane_mapping(struct intel_crtc *crtc)
+{
+   struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+   u32 reg, val;
+
+   reg = DSPCNTR(!crtc->plane);
+   val = I915_READ(reg);
+
+   if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
+   (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
+   return false;
+
+   return true;
+}
+
 static void intel_sanitize_crtc(struct intel_crtc *crtc)
 {
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
-   u32 reg, val;
+   u32 reg;
 
/* Clear any frame start delays used for debugging left by the BIOS */
reg = PIPECONF(crtc->pipe);
@@ -8006,17 +8022,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 
/* We need to sanitize the plane -> pipe mapping first because this will
 * disable the crtc (and hence change the state) if it is wrong. */
-   if (!HAS_PCH_SPLIT(dev)) {
+   if (!HAS_PCH_SPLIT(dev) && !intel_check_plane_mapping(crtc)) {
struct intel_connector *connector;
bool plane;
 
-   reg = DSPCNTR(crtc->plane);
-   val = I915_READ(reg);
-
-   if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
-   (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
-   goto ok;
-
DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
  crtc->base.base.id);
 
@@ -8040,7 +8049,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
WARN_ON(crtc->active);
crtc->base.enabled = false;
}
-ok:
 
if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
crtc->pipe == PIPE_A && !crtc->active) {
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 4/4] drm/i915: update dpms property in set_mode

2012-09-06 Thread Daniel Vetter
Hopefully this makes userspace slightly less confused about us
frobbing the dpms state behind its back. Yeah, it would be better
to be more careful with not changing the dpms state, but that is
quite more invasive.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 805324d..bff0936 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6806,7 +6806,13 @@ intel_modeset_update_state(struct drm_device *dev, 
unsigned prepare_pipes)
intel_crtc = to_intel_crtc(connector->encoder->crtc);
 
if (prepare_pipes & (1 << intel_crtc->pipe)) {
+   struct drm_property *dpms_property =
+   dev->mode_config.dpms_property;
+
connector->dpms = DRM_MODE_DPMS_ON;
+   drm_connector_property_set_value(connector,
+dpms_property,
+DRM_MODE_DPMS_ON);
 
intel_encoder = to_intel_encoder(connector->encoder);
intel_encoder->connectors_active = true;
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 3/4] drm/i915: don't call dpms funcs after set_mode

2012-09-06 Thread Daniel Vetter
... because our current set_mode implementation doesn't bother to adjust
for the dpms state, we just forcefully update it. So stop pretending that
we're better than we are and rip out this extranous call.

Note that this totally confuses userspace, because the exposed connector
property isn't actually updated ...

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0973797..805324d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7236,7 +7236,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
struct drm_mode_set save_set;
struct intel_set_config *config;
int ret;
-   int i;
 
BUG_ON(!set);
BUG_ON(!set->crtc);
@@ -7300,15 +7299,6 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
ret = -EINVAL;
goto fail;
}
-
-   if (set->crtc->enabled) {
-   DRM_DEBUG_KMS("Setting connector DPMS state to on\n");
-   for (i = 0; i < set->num_connectors; i++) {
-   DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS 
on\n", set->connectors[i]->base.id,
- 
drm_get_connector_name(set->connectors[i]));
-   
set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
-   }
-   }
} else if (config->fb_changed) {
ret = intel_pipe_set_base(set->crtc,
  set->x, set->y, set->fb);
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 2/4] drm/i915: don't disable fdi links harder in ilk_crtc_enable

2012-09-06 Thread Daniel Vetter
Because they should have been disabled when shutting down the display
pipe previously. To ensure that this is the case, add a few assserts
instead of unconditionally disabling the fdi link.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6c06109..0973797 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3182,10 +3182,12 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
is_pch_port = intel_crtc_driving_pch(crtc);
 
-   if (is_pch_port)
+   if (is_pch_port) {
ironlake_fdi_pll_enable(intel_crtc);
-   else
-   ironlake_fdi_disable(crtc);
+   } else {
+   assert_fdi_tx_disabled(dev_priv, pipe);
+   assert_fdi_rx_disabled(dev_priv, pipe);
+   }
 
/* Enable panel fitting for LVDS */
if (dev_priv->pch_pf_size &&
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 1/4] drm/i915: rip out intel_disable_pch_ports

2012-09-06 Thread Daniel Vetter
Even with the old crtc helper code we should have disabled all
encoders on that pipe by now, and with the new code this would
definitely paper over a bug. We already have the necessary checks
in place in intel_disable_transcoder, so if we accidentally leave
a pch port on, this will be caught.

Hence just rip this all out.

Note that up to the patch in this giant modeset series that removes
the LVDS special case to avoid disabling LVDS in the encoder->prepare
callback ("drm/i915/lvds: ditch ->prepare special case"), this was not
the case for all outputs.

Also note that in

commit 1b3c7a47f993bf9ab6c4c7cc3bbf5588052b58f4
Author: Zhenyu Wang 
Date:   Wed Nov 25 13:09:38 2009 +0800

drm/i915: Fix LVDS stability issue on Ironlake

this was already discovered independently and worked around. How I
bloody hate this entire mess of cludges piled on top of other cludges.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c | 60 
 1 file changed, 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e061acd..6c06109 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1860,59 +1860,6 @@ static void intel_disable_plane(struct drm_i915_private 
*dev_priv,
intel_wait_for_vblank(dev_priv->dev, pipe);
 }
 
-static void disable_pch_dp(struct drm_i915_private *dev_priv,
-  enum pipe pipe, int reg, u32 port_sel)
-{
-   u32 val = I915_READ(reg);
-   if (dp_pipe_enabled(dev_priv, pipe, port_sel, val)) {
-   DRM_DEBUG_KMS("Disabling pch dp %x on pipe %d\n", reg, pipe);
-   I915_WRITE(reg, val & ~DP_PORT_EN);
-   }
-}
-
-static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
-enum pipe pipe, int reg)
-{
-   u32 val = I915_READ(reg);
-   if (hdmi_pipe_enabled(dev_priv, pipe, val)) {
-   DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n",
- reg, pipe);
-   I915_WRITE(reg, val & ~PORT_ENABLE);
-   }
-}
-
-/* Disable any ports connected to this transcoder */
-static void intel_disable_pch_ports(struct drm_i915_private *dev_priv,
-   enum pipe pipe)
-{
-   u32 reg, val;
-
-   val = I915_READ(PCH_PP_CONTROL);
-   I915_WRITE(PCH_PP_CONTROL, val | PANEL_UNLOCK_REGS);
-
-   disable_pch_dp(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
-   disable_pch_dp(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
-   disable_pch_dp(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
-
-   reg = PCH_ADPA;
-   val = I915_READ(reg);
-   if (adpa_pipe_enabled(dev_priv, pipe, val))
-   I915_WRITE(reg, val & ~ADPA_DAC_ENABLE);
-
-   reg = PCH_LVDS;
-   val = I915_READ(reg);
-   if (lvds_pipe_enabled(dev_priv, pipe, val)) {
-   DRM_DEBUG_KMS("disable lvds on pipe %d val 0x%08x\n", pipe, 
val);
-   I915_WRITE(reg, val & ~LVDS_PORT_EN);
-   POSTING_READ(reg);
-   udelay(100);
-   }
-
-   disable_pch_hdmi(dev_priv, pipe, HDMIB);
-   disable_pch_hdmi(dev_priv, pipe, HDMIC);
-   disable_pch_hdmi(dev_priv, pipe, HDMID);
-}
-
 int
 intel_pin_and_fence_fb_obj(struct drm_device *dev,
   struct drm_i915_gem_object *obj,
@@ -3311,13 +3258,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 
ironlake_fdi_disable(crtc);
 
-   /* This is a horrible layering violation; we should be doing this in
-* the connector/encoder ->prepare instead, but we don't always have
-* enough information there about the config to know whether it will
-* actually be necessary or just cause undesired flicker.
-*/
-   intel_disable_pch_ports(dev_priv, pipe);
-
intel_disable_transcoder(dev_priv, pipe);
 
if (HAS_PCH_CPT(dev)) {
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 0/4] some follow-up fixlets from the modeset rework

2012-09-06 Thread Daniel Vetter
Hi all,

Just a few small follow-ups
- 2 patches to make pch/fdi handling more strict and add more asserts to check
  that.
- 2 patches to make dpms handling around modesets more strict.

All four have been included in the modeset-rework branch since a long time and
hence seen tons of testing already. But I've figured they're not part of the
core rework, hence I've split them out.

Flames, comments & review highly welcome.

Cheers, Daniel

Daniel Vetter (4):
  drm/i915: rip out intel_disable_pch_ports
  drm/i915: don't disable fdi links harder in ilk_crtc_enable
  drm/i915: don't call dpms funcs after set_mode
  drm/i915: update dpms property in set_mode

 drivers/gpu/drm/i915/intel_display.c | 84 +---
 1 file changed, 11 insertions(+), 73 deletions(-)

-- 
1.7.11.2

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


Re: [Intel-gfx] [PATCH 00/58] modeset-rework, the basic conversion

2012-09-06 Thread Daniel Vetter
On Sun, Aug 19, 2012 at 09:12:17PM +0200, Daniel Vetter wrote:
> Hi all,
> 
> Changes since last time around:
> - The prep patches are all merged now.
> - I've left out the actual DP fixes/cleanups, I think we should merge those 
> in a
>   separte step.
> - A few bugfixes (thanks to Paulo, Jani and Chris).
> - I've also applied a few bikesheds for naming that Paulo suggested (but I'm 
> not
>   sure whether I've sent those out already in a previous patchbomb).
> 
> Essentially this is just the core rework, which addes the new get_hw_state 
> code,
> refactors all the encoders to use the new functions and finally reworks the
> modeset logic to disable/enable entire pipes, always (and with a deterministic
> order).
> 
> For merging to -next, I plan to pull in everything with a real merge commit. 
> For
> that reason I've put up a modeset-rework-base branch onto my private fdo 
> repo[1].
> That way I can put a short documentation for the new modeset design into the
> merge commit (stichted together from the previous patchbomb cover letters),
> documenting my folly assumptions for eternity.
> 
> I'll also plan to put tags for the entire series in the merge commit, so if 
> you
> have tested this on a few machines, read through and agree with the new 
> designs,
> please reply with your tested-by/acked-by/reviewed-by tags.
> 
> Flames, comments and test reports highly welcome.

Ok, I've just merged the entire pile. Thanks everyone for testing
feedback, comments and review.

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


[Intel-gfx] [PATCH 7/7] drm/i915: expose energy counter on SNB and IVB

2012-09-06 Thread Ben Widawsky
From: Jesse Barnes 

On SNB and IVB, there's an MSR (also exposed through MCHBAR) we can use
to read out the amount of energy used over time.  Expose this in sysfs
to make it easy to do power comparisons with different configurations.

If the platform supports it, the file will show up under the
drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ.
The value in the file is a running total of energy (in microjoules)
consumed by the graphics device.

v2: move to sysfs (Ben, Daniel)
expose a simple value (Chris)
drop unrelated hunk (Ben)

Signed-off-by: Jesse Barnes 

v3: by Ben
Tied it into existing rc6  sysfs entries and named that a more generic
"power attrs." Fixed rebase conflicts.

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_reg.h   |  2 ++
 drivers/gpu/drm/i915/i915_sysfs.c | 25 +++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d0b60f2..0e34f5f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1242,6 +1242,8 @@
 #define CLKCFG_MEM_800 (3 << 4)
 #define CLKCFG_MEM_MASK(7 << 4)
 
+#define SECP_NRG_STTS  (MCHBAR_MIRROR_BASE_SNB + 0x592c)
+
 #define TSC1   0x11001
 #define   TSE  (1<<0)
 #define TR10x11006
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index e3a31ae..6bbc9af 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -76,22 +76,43 @@ show_rc6pp_ms(struct device *kdev, struct device_attribute 
*attr, char *buf)
return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
 }
 
+#define MSR_IA32_PACKAGE_POWER_SKU_UNIT0x0606
+
+static ssize_t
+show_gt_energy_uJ(struct device *dev, struct device_attribute *attr, char *buf)
+{
+   struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+   struct drm_i915_private *dev_priv = dminor->dev->dev_private;
+   u64 ppsu;
+   u32 val, units;
+
+   rdmsrl(MSR_IA32_PACKAGE_POWER_SKU_UNIT, ppsu);
+
+   ppsu = (ppsu & 0x1f00) >> 8;
+   units = 100 / (1 << ppsu); /* convert to uJ */
+   val = I915_READ(SECP_NRG_STTS);
+
+   return snprintf(buf, PAGE_SIZE, "%u", val * units);
+}
+
 static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL);
 static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL);
 static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL);
 static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL);
+static DEVICE_ATTR(gt_energy_uJ, S_IRUGO, show_gt_energy_uJ, NULL);
 
-static struct attribute *rc6_attrs[] = {
+static struct attribute *power_attrs[] = {
&dev_attr_rc6_enable.attr,
&dev_attr_rc6_residency_ms.attr,
&dev_attr_rc6p_residency_ms.attr,
&dev_attr_rc6pp_residency_ms.attr,
+   &dev_attr_gt_energy_uJ.attr,
NULL
 };
 
 static struct attribute_group rc6_attr_group = {
.name = power_group_name,
-   .attrs =  rc6_attrs
+   .attrs =  power_attrs
 };
 #endif
 
-- 
1.7.12

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


[Intel-gfx] [PATCH 6/7] drm/i915: Show render P state thresholds in sysfs

2012-09-06 Thread Ben Widawsky
This is useful for userspace utilities which wish to use the previous
interface, specifically for micromanaging the increase/decrease steps by
setting min == max.

CC: Jacob Pan 
CC: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index 5a5e610..e3a31ae 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -333,10 +333,46 @@ static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, 
gt_cur_freq_mhz_show, NULL);
 static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
gt_max_freq_mhz_store);
 static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
gt_min_freq_mhz_store);
 
+
+static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute 
*attr, char *buf);
+static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
+static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
+static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
+
+/* For now we have a static number of RP states */
+static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute 
*attr, char *buf)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val, rp_state_cap;
+   ssize_t ret;
+
+   ret = mutex_lock_interruptible(&dev->struct_mutex);
+   if (ret)
+   return ret;
+   rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+   mutex_unlock(&dev->struct_mutex);
+
+   if (attr == &dev_attr_gt_RP0_freq_mhz) {
+   val = ((rp_state_cap & 0xff) >> 0) * 
GT_FREQUENCY_MULTIPLIER;
+   } else if (attr == &dev_attr_gt_RP1_freq_mhz) {
+   val = ((rp_state_cap & 0x00ff00) >> 8) * 
GT_FREQUENCY_MULTIPLIER;
+   } else if (attr == &dev_attr_gt_RPn_freq_mhz) {
+   val = ((rp_state_cap & 0xff) >> 16) * 
GT_FREQUENCY_MULTIPLIER;
+   } else {
+   BUG();
+   }
+   return snprintf(buf, PAGE_SIZE, "%d", val);
+}
+
 static const struct attribute *gen6_attrs[] = {
&dev_attr_gt_cur_freq_mhz.attr,
&dev_attr_gt_max_freq_mhz.attr,
&dev_attr_gt_min_freq_mhz.attr,
+   &dev_attr_gt_RP0_freq_mhz.attr,
+   &dev_attr_gt_RP1_freq_mhz.attr,
+   &dev_attr_gt_RPn_freq_mhz.attr,
NULL,
 };
 
-- 
1.7.12

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


[Intel-gfx] [PATCH 5/7] drm/i915: Add setters for min/max frequency

2012-09-06 Thread Ben Widawsky
Provide a standardized sysfs interface for setting min, and max
frequencies.  The code which reads the limits were lifted from the
debugfs files. As a brief explanation, the limits are similar to the CPU
p-states. We have 3 states:

RP0 - ie. max frequency
RP1 - ie. "local min" frequency
RPn - seriously lowest frequency

Initially Daniel asked me to clamp the writes to supported values, but
in conforming to the way the cpufreq drivers seem to work, instead
return -EINVAL (noticed by Jesse in discussion).

The values can be used by userspace wishing to control the limits of the
GPU (see the CC list for people who care).

CC: Arjan van de Ven 
CC: Jacob Pan 
CC: Daniel Vetter 
CC: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 78 ++-
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index 6c7f905..5a5e610 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -238,6 +238,43 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, 
struct device_attribute
return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_max_freq_mhz_store(struct device *kdev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val, rp_state_cap, hw_max;
+   ssize_t ret;
+
+   ret = kstrtou32(buf, 0, &val);
+   if (ret)
+   return ret;
+
+   ret = mutex_lock_interruptible(&dev->struct_mutex);
+   if (ret)
+   return ret;
+
+   rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+   hw_max = (rp_state_cap & 0xff) * GT_FREQUENCY_MULTIPLIER;
+
+   if (val > hw_max)
+   return -EINVAL;
+
+   if (val < dev_priv->rps.min_delay)
+   val = dev_priv->rps.min_delay;
+
+   if (dev_priv->rps.cur_delay > val)
+   gen6_set_rps(dev_priv->dev, val);
+
+   dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+   mutex_unlock(&dev->struct_mutex);
+
+   return count;
+}
+
 static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)
 {
struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
@@ -255,9 +292,46 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, 
struct device_attribute
return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_min_freq_mhz_store(struct device *kdev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val, rp_state_cap, hw_min;
+   ssize_t ret;
+
+   ret = kstrtou32(buf, 0, &val);
+   if (ret)
+   return ret;
+
+   ret = mutex_lock_interruptible(&dev->struct_mutex);
+   if (ret)
+   return ret;
+
+   rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+   hw_min = ((rp_state_cap & 0xff) >> 16) * GT_FREQUENCY_MULTIPLIER;
+
+   if (val < hw_min)
+   return -EINVAL;
+
+   if (val > dev_priv->rps.max_delay)
+   val = dev_priv->rps.max_delay;
+
+   if (dev_priv->rps.cur_delay < val)
+   gen6_set_rps(dev_priv->dev, val);
+
+   dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+   mutex_unlock(&dev->struct_mutex);
+
+   return count;
+}
+
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
NULL);
-static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
gt_max_freq_mhz_store);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
gt_min_freq_mhz_store);
 
 static const struct attribute *gen6_attrs[] = {
&dev_attr_gt_cur_freq_mhz.attr,
-- 
1.7.12

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


[Intel-gfx] [PATCH 4/7 v2] drm/i915: Add current GPU freq to sysfs

2012-09-06 Thread Ben Widawsky
Userspace applications such as PowerTOP are interesting in being able to
read the current GPU frequency. The patch itself sets up a generic array
for gen6 attributes so we can easily add other items in the future (and
it also happens to be just about the cleanest way to do this).

The patch is a nice addition to
commit 1ac02185dff3afac146d745ba220dc6672d1d162
Author: Daniel Vetter 
Date:   Thu Aug 30 13:26:48 2012 +0200

drm/i915: add a tracepoint for gpu frequency changes

Reading the GPU frequncy can be done by reading a file like:
/sys/class/drm/card0/render_frequency_mhz

v2: rename the sysfs file to gt_cur_freq_mhz (Daniel)
Use kdev naming (Chris)
Use DEVICE_ATTR over __ATTR (Ben)
Add min/max freq (Ben/Daniel)
user the new #define for frequency multiplier

CC: Arjan van de Ven 
CC: Jacob Pan 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 71 +++
 1 file changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index c701a17..6c7f905 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -203,6 +203,70 @@ static struct bin_attribute dpf_attrs = {
.mmap = NULL
 };
 
+static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
+   struct device_attribute *attr, char *buf)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
+   mutex_unlock(&dev->struct_mutex);
+
+   return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
+   mutex_unlock(&dev->struct_mutex);
+
+   return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct 
device_attribute *attr, char *buf)
+{
+   struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+   struct drm_device *dev = minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ret = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
+   mutex_unlock(&dev->struct_mutex);
+
+   return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
NULL);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
NULL);
+
+static const struct attribute *gen6_attrs[] = {
+   &dev_attr_gt_cur_freq_mhz.attr,
+   &dev_attr_gt_max_freq_mhz.attr,
+   &dev_attr_gt_min_freq_mhz.attr,
+   NULL,
+};
+
+
 void i915_setup_sysfs(struct drm_device *dev)
 {
int ret;
@@ -220,10 +284,17 @@ void i915_setup_sysfs(struct drm_device *dev)
if (ret)
DRM_ERROR("l3 parity sysfs setup failed\n");
}
+
+   if (INTEL_INFO(dev)->gen >= 6) {
+   ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs);
+   if (ret)
+   DRM_ERROR("gen6 sysfs setup failed\n");
+   }
 }
 
 void i915_teardown_sysfs(struct drm_device *dev)
 {
+   sysfs_remove_files(&dev->primary->kdev.kobj, gen6_attrs);
device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
 }
-- 
1.7.12

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


[Intel-gfx] [PATCH 3/7] drm/i915: #define gpu freq multipler

2012-09-06 Thread Ben Widawsky
Magic numbers are bad mmmkay. In this case in particular the value is
especially weird because the docs say multiple things. We'll need this
value for sysfs, so extracting it is useful for that as well.

CC: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 22 +++---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3d886af..73d1f98 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -930,7 +930,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void 
*unused)
seq_printf(m, "Render p-state limit: %d\n",
   rp_state_limits & 0xff);
seq_printf(m, "CAGF: %dMHz\n", ((rpstat & GEN6_CAGF_MASK) >>
-   GEN6_CAGF_SHIFT) * 50);
+   GEN6_CAGF_SHIFT) * 
GT_FREQUENCY_MULTIPLIER);
seq_printf(m, "RP CUR UP EI: %dus\n", rpupei &
   GEN6_CURICONT_MASK);
seq_printf(m, "RP CUR UP: %dus\n", rpcurup &
@@ -946,15 +946,15 @@ static int i915_cur_delayinfo(struct seq_file *m, void 
*unused)
 
max_freq = (rp_state_cap & 0xff) >> 16;
seq_printf(m, "Lowest (RPN) frequency: %dMHz\n",
-  max_freq * 50);
+  max_freq * GT_FREQUENCY_MULTIPLIER);
 
max_freq = (rp_state_cap & 0xff00) >> 8;
seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
-  max_freq * 50);
+  max_freq * GT_FREQUENCY_MULTIPLIER);
 
max_freq = rp_state_cap & 0xff;
seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
-  max_freq * 50);
+  max_freq * GT_FREQUENCY_MULTIPLIER);
} else {
seq_printf(m, "no P-state info available\n");
}
@@ -1308,7 +1308,7 @@ static int i915_ring_freq_table(struct seq_file *m, void 
*unused)
continue;
}
ia_freq = I915_READ(GEN6_PCODE_DATA);
-   seq_printf(m, "%d\t\t%d\n", gpu_freq * 50, ia_freq * 100);
+   seq_printf(m, "%d\t\t%d\n", gpu_freq * GT_FREQUENCY_MULTIPLIER, 
ia_freq * 100);
}
 
mutex_unlock(&dev->struct_mutex);
@@ -1735,7 +1735,7 @@ i915_max_freq_read(struct file *filp,
return ret;
 
len = snprintf(buf, sizeof(buf),
-  "max freq: %d\n", dev_priv->rps.max_delay * 50);
+  "max freq: %d\n", dev_priv->rps.max_delay * 
GT_FREQUENCY_MULTIPLIER);
mutex_unlock(&dev->struct_mutex);
 
if (len > sizeof(buf))
@@ -1778,9 +1778,9 @@ i915_max_freq_write(struct file *filp,
/*
 * Turbo will still be enabled, but won't go above the set value.
 */
-   dev_priv->rps.max_delay = val / 50;
+   dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
 
-   gen6_set_rps(dev, val / 50);
+   gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
mutex_unlock(&dev->struct_mutex);
 
return cnt;
@@ -1811,7 +1811,7 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, 
size_t max,
return ret;
 
len = snprintf(buf, sizeof(buf),
-  "min freq: %d\n", dev_priv->rps.min_delay * 50);
+  "min freq: %d\n", dev_priv->rps.min_delay * 
GT_FREQUENCY_MULTIPLIER);
mutex_unlock(&dev->struct_mutex);
 
if (len > sizeof(buf))
@@ -1852,9 +1852,9 @@ i915_min_freq_write(struct file *filp, const char __user 
*ubuf, size_t cnt,
/*
 * Turbo will still be enabled, but won't go below the set value.
 */
-   dev_priv->rps.min_delay = val / 50;
+   dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
 
-   gen6_set_rps(dev, val / 50);
+   gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
mutex_unlock(&dev->struct_mutex);
 
return cnt;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f16ab5e..51af543 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1162,6 +1162,8 @@ struct drm_i915_file_private {
 
 #define HAS_L3_GPU_CACHE(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
 
+#define GT_FREQUENCY_MULTIPLIER 50
+
 #include "i915_trace.h"
 
 /**
-- 
1.7.12

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


[Intel-gfx] [PATCH 2/7] drm/i915: variable renames

2012-09-06 Thread Ben Widawsky
From: Ben Widawsky 

Name variables a bit better for copy-pasters. This got turned up as part
of review for upcoming sysfs patches.

CC: Chris Wilson 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index da733a3..c701a17 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -46,32 +46,32 @@ static u32 calc_residency(struct drm_device *dev, const u32 
reg)
 }
 
 static ssize_t
-show_rc6_mask(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-   struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+   struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
return snprintf(buf, PAGE_SIZE, "%x", intel_enable_rc6(dminor->dev));
 }
 
 static ssize_t
-show_rc6_ms(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-   struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+   struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
u32 rc6_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6);
return snprintf(buf, PAGE_SIZE, "%u", rc6_residency);
 }
 
 static ssize_t
-show_rc6p_ms(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-   struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+   struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
u32 rc6p_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6p);
return snprintf(buf, PAGE_SIZE, "%u", rc6p_residency);
 }
 
 static ssize_t
-show_rc6pp_ms(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-   struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+   struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
u32 rc6pp_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6pp);
return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
 }
-- 
1.7.12

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


[Intel-gfx] [PATCH 1/7] drm/i915: Enable some sysfs stuff without CONFIG_PM

2012-09-06 Thread Ben Widawsky
The original patch was actually incorrect in stubbing out the sysfs for
l3 parity.
commit 5ab3633d6907018b0b830a720e877c3884d679c3
Author: Hunt Xu 
Date:   Sun Jul 1 03:45:07 2012 +

drm/i915: make rc6 in sysfs functions conditional

Unfortunately Hunt didn't respond to my review comments, and Daniel
sucked in the patch again ignoring. Worst of all, I'm too lazy to write
the patch for what I originally wanted, which was to keep rc6 sysfs even
without CONFIG_PM. This simpler patch does enough to enable us to add
more sysfs entries though.

NOTE: This is already picked up in dinq, but is here in case anyone is
looking to test this series on it's own.

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_sysfs.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index c5ee7ee..da733a3 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -93,6 +93,7 @@ static struct attribute_group rc6_attr_group = {
.name = power_group_name,
.attrs =  rc6_attrs
 };
+#endif
 
 static int l3_access_valid(struct drm_device *dev, loff_t offset)
 {
@@ -206,13 +207,14 @@ void i915_setup_sysfs(struct drm_device *dev)
 {
int ret;
 
+#ifdef CONFIG_PM
if (INTEL_INFO(dev)->gen >= 6) {
ret = sysfs_merge_group(&dev->primary->kdev.kobj,
&rc6_attr_group);
if (ret)
DRM_ERROR("RC6 residency sysfs setup failed\n");
}
-
+#endif
if (HAS_L3_GPU_CACHE(dev)) {
ret = device_create_bin_file(&dev->primary->kdev, &dpf_attrs);
if (ret)
@@ -225,14 +227,3 @@ void i915_teardown_sysfs(struct drm_device *dev)
device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
 }
-#else
-void i915_setup_sysfs(struct drm_device *dev)
-{
-   return;
-}
-
-void i915_teardown_sysfs(struct drm_device *dev)
-{
-   return;
-}
-#endif /* CONFIG_PM */
-- 
1.7.12

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


[Intel-gfx] [PATCH 0/7] moar sysfs stuff

2012-09-06 Thread Ben Widawsky
Not very thoroughly tested yet. I'll be adding igt tests ASAP.

Ben Widawsky (6):
  drm/i915: Enable some sysfs stuff without CONFIG_PM
  drm/i915: variable renames
  drm/i915: #define gpu freq multipler
  drm/i915: Add current GPU freq to sysfs
  drm/i915: Add setters for min/max frequency
  drm/i915: Show render P state thresholds in sysfs

Jesse Barnes (1):
  drm/i915: expose energy counter on SNB and IVB

 drivers/gpu/drm/i915/i915_debugfs.c |  22 ++--
 drivers/gpu/drm/i915/i915_drv.h |   2 +
 drivers/gpu/drm/i915/i915_reg.h |   2 +
 drivers/gpu/drm/i915/i915_sysfs.c   | 237 
 4 files changed, 230 insertions(+), 33 deletions(-)

-- 
1.7.12

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


Re: [Intel-gfx] [PATCH] drm/i915: no longer call drm_helper_resume_force_mode

2012-09-06 Thread Jesse Barnes
On Wed, 5 Sep 2012 13:04:54 -0700
Jesse Barnes  wrote:

> On Wed, 5 Sep 2012 21:56:08 +0200
> Daniel Vetter  wrote:
> 
> > On Wed, Sep 5, 2012 at 8:31 PM, Jesse Barnes  
> > wrote:
> > > On Wed, 29 Aug 2012 23:13:29 +0200
> > > Daniel Vetter  wrote:
> > >
> > >> Since this only calls crtc helper functions, of which a shocking
> > >> amount are NULL.
> > >>
> > >> Now the curious thing is how the new modeset code worked with this
> > >> function call still present:
> > >>
> > >> Thanks to the hw state readout and the suspend fixes to properly
> > >> quiescent the register state, nothing is actually enabled at resume
> > >> (if the bios doesn't set up anything). Which means resume_force_mode
> > >> doesn't actually do anything and hence nothing blows up at resume
> > >> time.
> > >>
> > >> The other reason things do work is that the fbcon layer has it's own
> > >> resume notifier callback, which restores the mode. And thanks to the
> > >> force vt switch at suspend/resume, that then forces X to restore it's
> > >> own mode.
> > >>
> > >> Hence everything still worked (as long as the bios doesn't enable
> > >> anything). And we can just kill the call to resume_force_mode.
> > >>
> > >> The upside of both this patch and the preceeding patch to quiescent
> > >> the modeset state is that our resume path is much simpler:
> > >> - We now longer restore bogus register values (which most often would
> > >>   enable the backlight a bit and a few ports), causing flickering.
> > >> - We now longer call resume_force_mode to restore a mode that the
> > >>   fbcon layer would overwrite right away anyway.
> > >>
> > >> Signed-off-by: Daniel Vetter 
> > >> ---
> > >>  drivers/gpu/drm/i915/i915_drv.c | 5 -
> > >>  1 file changed, 5 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > >> b/drivers/gpu/drm/i915/i915_drv.c
> > >> index fe7512a..cd6697c 100644
> > >> --- a/drivers/gpu/drm/i915/i915_drv.c
> > >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> > >> @@ -549,11 +549,6 @@ static int i915_drm_thaw(struct drm_device *dev)
> > >>   intel_modeset_setup_hw_state(dev);
> > >>   drm_mode_config_reset(dev);
> > >>   drm_irq_install(dev);
> > >> -
> > >> - /* Resume the modeset for every activated CRTC */
> > >> - mutex_lock(&dev->mode_config.mutex);
> > >> - drm_helper_resume_force_mode(dev);
> > >> - mutex_unlock(&dev->mode_config.mutex);
> > >>   }
> > >>
> > >>   intel_opregion_init(dev);
> > >
> > > Wouldn't the fb layer's modeset end up being a no-op if the suspended
> > > mode was the same as the fb mode (often the case)?  Or at the very
> > > least just a flip rather than a full mode set.
> > 
> > I guess most of the flicker was because the register restoring
> > restored a bunch of crap (since the old modeset state wasn't properly
> > cleared before suspending).
> > 
> > > Though we do need to deal with non-fb, non-X resumes as well.  kmscon
> > > and wayland will expect to be restored at resume time even if CONFIG_VT
> > > and the fb layer aren't compiled into the kernel.
> > 
> > Tbh I was rather surprised that when I've noticed this little issue
> > here the restore still worked - until I've noticed by looking at the
> > logs that both the fbcon and the X server restore their modes.
> > 
> > I'm not sure what exactly we should do here, since even with the
> > current code the concept of a controlling node isn't really defined in
> > the kms interface (fbcon uses a bunch of funky checks to ensure it
> > doesn't clobber the output state of someone else). But for now (with
> > fbcon pretty much being non-optional) things keep on working, and
> > afaict actually work a bit better overall.
> 
> It's probably ok for now, but at some point we'll want some code that
> restores the suspend mode if fbcon isn't enabled...
> 

So acked/reviewed-by: Jesse Barnes 

But note that if someone complains we'll need to add this back...

-- 
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] drm/i915: push commit_output_state past the crtc/encoder preparing

2012-09-06 Thread Jesse Barnes
On Wed, 5 Sep 2012 12:50:26 -0700
Jesse Barnes  wrote:

> On Wed, 5 Sep 2012 21:48:52 +0200
> Daniel Vetter  wrote:
> 
> > On Wed, Sep 5, 2012 at 8:28 PM, Jesse Barnes  
> > wrote:
> > >
> > > The variables have me confused a little... I would have expected
> > > update_state to take modeset_pipes rather than prepare_pipes.  Could
> > > you use either?  Or will that not catch cases where we updated a pipe
> > > that was already on?
> > 
> > The abstract idea for these masks was the following: Any pipe that
> > changes anything goes into prepare_pipes. For any pipe that also
> > changes the mode, it goes in addition into the modeset_pipes mask, so
> > the later is a subset of prepare pipes. The idea here was to avoid the
> > modeset step where not necessary (e.g. when disabling the 2nd output
> > of a cloned crtc we only need to disable/enable, not change anything
> > with the mode or clocks). But after some in-depth discussion with
> > Paulo Zanoni I think we'll move large parts of the mode_set step into
> > the enable function (at least for hsw due to funky ordering
> > requirements), so I think this disdinction doesn't make sense.
> > 
> > The disable mask just contains those pipes that get fully disable (and
> > which then also get removed from the prepares/modset masks).
> > 
> > Hence I pass the prepares mask into update_states, not just the modeset 
> > mask.
> 
> Ok, that makes some sense.  Hopefully we can preserve the full mode set
> vs simple update behavior even after the refactoring for HSW.
> 

Reviewed-by: Jesse Barnes 

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


[Intel-gfx] [PATCH] drm/i915: use the gmbus irq for waits

2012-09-06 Thread Daniel Vetter
We need two special things to properly wire this up:
- Add another argument to gmbus_wait_hw_status to pass in the
  correct interrupt bit in gmbus4.
- Since we can only get an irq for one of the two events we want,
  hand-roll the wait_event_timeout code so that we wake up every
  jiffie and can check for NAKs. This way we also subsume gmbus
  support for platforms without interrupts (or where those are not
  yet enabled).

The important bit really is to only enable one gmbus interrupt source
at the same time - with that piece of lore figured out, this seems to
work flawlessly.

Ben Widawsky rightfully complained the lack of measurements for the
claimed benefits (especially since the first version was actually
broken and fell back to bit-banging). Previously reading the 256 byte
hdmi EDID takes about 72 ms here. With this patch it's down to 33 ms.
Given that transfering the 256 bytes over i2c at wire speed takes
20.5ms alone, the reduction in additional overhead is rather nice.

v2: Chris Wilson wondered whether GMBUS4 might contain some set bits
when booting up an hence result in some spurious interrupts. Since we
clear GMBUS4 after every wait and we do gmbus transfer really early in
the setup sequence to detect displays the window is small, but still
be paranoid and clear it properly.

v3: Clarify the comment that gmbus irq generation can only support one
kind of event, why it bothers us and how we work around that limit.

Cc: Daniel Kurtz 
Signed-off-by: Daniel Vetter 

fixup
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/i915_irq.c  |  4 
 drivers/gpu/drm/i915/intel_i2c.c | 45 ++--
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5c8d5e3..72db70d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -418,6 +418,8 @@ typedef struct drm_i915_private {
 */
uint32_t gpio_mmio_base;
 
+   wait_queue_head_t gmbus_wait_queue;
+
struct pci_dev *bridge_dev;
struct intel_ring_buffer ring[I915_NUM_RINGS];
uint32_t next_seqno;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8415fa6..dadc86b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -598,7 +598,11 @@ out:
 
 static void gmbus_irq_handler(struct drm_device *dev)
 {
+   struct drm_i915_private *dev_priv = (drm_i915_private_t *) 
dev->dev_private;
+
DRM_DEBUG_DRIVER("GMBUS interrupt\n");
+
+   wake_up_all(&dev_priv->gmbus_wait_queue);
 }
 
 static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 57decac..7413595 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -64,6 +64,7 @@ intel_i2c_reset(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
+   I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
 }
 
 static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable)
@@ -205,20 +206,38 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
 
 static int
 gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
-u32 gmbus2_status)
+u32 gmbus2_status,
+u32 gmbus4_irq_en)
 {
-   int ret;
+   int i;
int reg_offset = dev_priv->gpio_mmio_base;
-   u32 gmbus2;
+   u32 gmbus2 = 0;
+   DEFINE_WAIT(wait);
+
+   /* Important: The hw handles only the first bit, so set only one! Since
+* we also need to check for NAKs besides the hw ready/idle signal, we
+* need to wake up periodically and check that ourselves. */
+   I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en);
 
-   ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-  (GMBUS_SATOER | gmbus2_status),
-  50);
+   for (i = 0; i < msecs_to_jiffies(50) + 1; i++) {
+   prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
+   TASK_UNINTERRUPTIBLE);
+
+   gmbus2 = I915_READ(GMBUS2 + reg_offset);
+   if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
+   break;
+
+   schedule_timeout(1);
+   }
+   finish_wait(&dev_priv->gmbus_wait_queue, &wait);
+
+   I915_WRITE(GMBUS4 + reg_offset, 0);
 
if (gmbus2 & GMBUS_SATOER)
return -ENXIO;
-
-   return ret;
+   if (gmbus2 & gmbus2_status)
+   return 0;
+   return -ETIMEDOUT;
 }
 
 static int
@@ -239,7 +258,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct 
i2c_msg *msg,
int ret;
u32 val, loop = 0;
 
-   ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
+  

[Intel-gfx] [PATCH] drm/i915: reorder setup sequence to have irqs for output setup

2012-09-06 Thread Daniel Vetter
Otherwise the new&shiny irq-driven gmbus and dp aux code won't work that
well. Noticed since the dp aux code doesn't have an automatic fallback
with a timeout (since the hw provides for that already).

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_dma.c  |  5 ++---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 15 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2cba7b4..58320e0 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1325,9 +1325,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_gem_stolen;
 
-   intel_modeset_gem_init(dev);
-
-   ret = drm_irq_install(dev);
+   /* Note: This also enables irqs. */
+   ret = intel_modeset_gem_init(dev);
if (ret)
goto cleanup_gem;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9fce782..5c8d5e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1536,7 +1536,7 @@ static inline void intel_unregister_dsm_handler(void) { 
return; }
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
-extern void intel_modeset_gem_init(struct drm_device *dev);
+extern int intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void intel_modeset_setup_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6658367..01289eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7948,7 +7948,6 @@ void intel_modeset_init(struct drm_device *dev)
 
/* Just disable it once at startup */
i915_disable_vga(dev);
-   intel_setup_outputs(dev);
 }
 
 static void
@@ -8206,13 +8205,25 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev)
drm_mode_config_reset(dev);
 }
 
-void intel_modeset_gem_init(struct drm_device *dev)
+int intel_modeset_gem_init(struct drm_device *dev)
 {
+   int ret;
+
intel_modeset_init_hw(dev);
 
intel_setup_overlay(dev);
 
+   ret = drm_irq_install(dev);
+   if (ret)
+   return ret;
+
+   /* Importanat: Output setup needs working interrupts to use
+* interrupt-driven gmbus and dp aux. */
+   intel_setup_outputs(dev);
+
intel_modeset_setup_hw_state(dev);
+
+   return 0;
 }
 
 void intel_modeset_cleanup(struct drm_device *dev)
-- 
1.7.11.2

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


Re: [Intel-gfx] [PATCH 4/4] drm/i915: use gmbus irq to wait for gmbus idle

2012-09-06 Thread Chris Wilson
On Thu,  6 Sep 2012 09:10:02 +0200, Daniel Vetter  
wrote:
> GMBUS_ACTIVE has inverted sense and so doesn't fit into the
> wait_hw_status helper, hence create a new gmbus_wait_idle functions.
> Also, we only care about the idle irq event and nothing else, which
> allows us to use the wait_event_timeout helper directly without
> jumping through hoops to catch NAKs.
> 
> Since gen2/3 don't have gmbus interrupts, handle them separately with
> the old wait_for macro.
> 
> This shaves another few ms off reading EDID from a hdmi screen on my
> testbox here. EDID reading with interrupt driven gmbus is now as fast
> as with busy-looping gmbus at 28 ms here (with negligible cpu
> overhead).

I'll put my neck on the line and say I can't spot any other mistakes:
Reviewed-by: Chris Wilson 

A couple of triffling things, you could use whitespace more uniformly
and the comment for only enabling one interrupt source could do with
the explanation that then means we also have to poll for NAK.
-Chris

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


[Intel-gfx] [PATCH 4/4] drm/i915: use gmbus irq to wait for gmbus idle

2012-09-06 Thread Daniel Vetter
GMBUS_ACTIVE has inverted sense and so doesn't fit into the
wait_hw_status helper, hence create a new gmbus_wait_idle functions.
Also, we only care about the idle irq event and nothing else, which
allows us to use the wait_event_timeout helper directly without
jumping through hoops to catch NAKs.

Since gen2/3 don't have gmbus interrupts, handle them separately with
the old wait_for macro.

This shaves another few ms off reading EDID from a hdmi screen on my
testbox here. EDID reading with interrupt driven gmbus is now as fast
as with busy-looping gmbus at 28 ms here (with negligible cpu
overhead).

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_i2c.c | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 86f2b8c..faf85b3 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -204,6 +204,7 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
algo->data = bus;
 }
 
+#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 4)
 static int
 gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 u32 gmbus2_status,
@@ -239,6 +240,31 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 }
 
 static int
+gmbus_wait_idle(struct drm_i915_private *dev_priv)
+{
+   int ret;
+   int reg_offset = dev_priv->gpio_mmio_base;
+
+#define C ((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
+
+   if (!HAS_GMBUS_IRQ(dev_priv->dev))
+   return wait_for(C, 10);
+
+   /* Important: The hw handles only the first bit, so set only one! */
+   I915_WRITE(GMBUS4 + reg_offset, GMBUS_IDLE_EN);
+
+   ret = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
+
+   I915_WRITE(GMBUS4 + reg_offset, 0);
+
+   if (ret)
+   return 0;
+   else
+   return -ETIMEDOUT;
+#undef C
+}
+
+static int
 gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
u32 gmbus1_index)
 {
@@ -405,8 +431,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
 * We will re-enable it at the start of the next xfer,
 * till then let it sleep.
 */
-   if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
-10)) {
+   if (gmbus_wait_idle(dev_priv)) {
DRM_DEBUG_KMS("GMBUS [%s] timed out waiting for idle\n",
 adapter->name);
ret = -ETIMEDOUT;
@@ -430,8 +455,7 @@ clear_err:
 * it's slow responding and only answers on the 2nd retry.
 */
ret = -ENXIO;
-   if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
-10)) {
+   if (gmbus_wait_idle(dev_priv)) {
DRM_DEBUG_KMS("GMBUS [%s] timed out after NAK\n",
  adapter->name);
ret = -ETIMEDOUT;
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 3/4] drm/i915: use the gmbus irq for waits

2012-09-06 Thread Daniel Vetter
We need two special things to properly wire this up:
- Add another argument to gmbus_wait_hw_status to pass in the
  correct interrupt bit in gmbus4.
- Since we can only get an irq for one of the two events we want,
  hand-roll the wait_event_timeout code so that we wake up every
  jiffie and can check for NAKs. This way we also subsume gmbus
  support for platforms without interrupts (or where those are not
  yet enabled).

The important bit really is to only enable one gmbus interrupt source
at the same time - with that piece of lore figured out, this seems to
work flawlessly.

Ben Widawsky rightfully complained the lack of measurements for the
claimed benefits (especially since the first version was actually
broken and fell back to bit-banging). Previously reading the 256 byte
hdmi EDID takes about 72 ms here. With this patch it's down to 33 ms.
Given that transfering the 256 bytes over i2c at wire speed takes
20.5ms alone, the reduction in additional overhead is rather nice.

v2: Chris Wilson wondered whether GMBUS4 might contain some set bits
when booting up an hence result in some spurious interrupts. Since we
clear GMBUS4 after every wait and we do gmbus transfer really early in
the setup sequence to detect displays the window is small, but still
be paranoid and clear it properly.

Cc: Daniel Kurtz 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/i915_irq.c  |  4 
 drivers/gpu/drm/i915/intel_i2c.c | 43 ++--
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9fce782..d93ce14 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -418,6 +418,8 @@ typedef struct drm_i915_private {
 */
uint32_t gpio_mmio_base;
 
+   wait_queue_head_t gmbus_wait_queue;
+
struct pci_dev *bridge_dev;
struct intel_ring_buffer ring[I915_NUM_RINGS];
uint32_t next_seqno;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8415fa6..dadc86b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -598,7 +598,11 @@ out:
 
 static void gmbus_irq_handler(struct drm_device *dev)
 {
+   struct drm_i915_private *dev_priv = (drm_i915_private_t *) 
dev->dev_private;
+
DRM_DEBUG_DRIVER("GMBUS interrupt\n");
+
+   wake_up_all(&dev_priv->gmbus_wait_queue);
 }
 
 static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 57decac..86f2b8c 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -64,6 +64,7 @@ intel_i2c_reset(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
+   I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
 }
 
 static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable)
@@ -205,20 +206,36 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
 
 static int
 gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
-u32 gmbus2_status)
+u32 gmbus2_status,
+u32 gmbus4_irq_en)
 {
-   int ret;
+   int i;
int reg_offset = dev_priv->gpio_mmio_base;
-   u32 gmbus2;
+   u32 gmbus2 = 0;
+   DEFINE_WAIT(wait);
+
+   /* Important: The hw handles only the first bit, so set only one! */
+   I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en);
 
-   ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-  (GMBUS_SATOER | gmbus2_status),
-  50);
+   for (i = 0; i < msecs_to_jiffies(50) + 1; i++) {
+   prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
+   TASK_UNINTERRUPTIBLE);
+
+   gmbus2 = I915_READ(GMBUS2 + reg_offset);
+   if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
+   break;
+
+   schedule_timeout(1);
+   }
+   finish_wait(&dev_priv->gmbus_wait_queue, &wait);
+
+   I915_WRITE(GMBUS4 + reg_offset, 0);
 
if (gmbus2 & GMBUS_SATOER)
return -ENXIO;
-
-   return ret;
+   if (gmbus2 & gmbus2_status)
+   return 0;
+   return -ETIMEDOUT;
 }
 
 static int
@@ -239,7 +256,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct 
i2c_msg *msg,
int ret;
u32 val, loop = 0;
 
-   ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
+   ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY,
+  GMBUS_HW_RDY_EN);
if (ret)
return ret;
 
@@ -283,7 +301,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct 
i2c_msg *msg)
 
I915_WRITE

[Intel-gfx] [PATCH 2/4] drm/i915: wire up gmbus irq handler

2012-09-06 Thread Daniel Vetter
Only enables the interrupt and puts a irq handler into place, doesn't
do anything yet.

Unfortunately there's no gmbus interrupt support for gen2/3 (safe for
pnv, but there the irq is marked as "Test mode").

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_irq.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a61b41a..8415fa6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -596,6 +596,11 @@ out:
return ret;
 }
 
+static void gmbus_irq_handler(struct drm_device *dev)
+{
+   DRM_DEBUG_DRIVER("GMBUS interrupt\n");
+}
+
 static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
 {
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -607,7 +612,7 @@ static void ibx_irq_handler(struct drm_device *dev, u32 
pch_iir)
 SDE_AUDIO_POWER_SHIFT);
 
if (pch_iir & SDE_GMBUS)
-   DRM_DEBUG_DRIVER("PCH GMBUS interrupt\n");
+   gmbus_irq_handler(dev);
 
if (pch_iir & SDE_AUDIO_HDCP_MASK)
DRM_DEBUG_DRIVER("PCH HDCP audio interrupt\n");
@@ -650,7 +655,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 
pch_iir)
DRM_DEBUG_DRIVER("AUX channel interrupt\n");
 
if (pch_iir & SDE_GMBUS_CPT)
-   DRM_DEBUG_DRIVER("PCH GMBUS interrupt\n");
+   gmbus_irq_handler(dev);
 
if (pch_iir & SDE_AUDIO_CP_REQ_CPT)
DRM_DEBUG_DRIVER("Audio CP request interrupt\n");
@@ -1841,12 +1846,14 @@ static int ironlake_irq_postinstall(struct drm_device 
*dev)
hotplug_mask = (SDE_CRT_HOTPLUG_CPT |
SDE_PORTB_HOTPLUG_CPT |
SDE_PORTC_HOTPLUG_CPT |
-   SDE_PORTD_HOTPLUG_CPT);
+   SDE_PORTD_HOTPLUG_CPT |
+   SDE_GMBUS_CPT);
} else {
hotplug_mask = (SDE_CRT_HOTPLUG |
SDE_PORTB_HOTPLUG |
SDE_PORTC_HOTPLUG |
SDE_PORTD_HOTPLUG |
+   SDE_GMBUS |
SDE_AUX_MASK);
}
 
@@ -1906,7 +1913,8 @@ static int ivybridge_irq_postinstall(struct drm_device 
*dev)
hotplug_mask = (SDE_CRT_HOTPLUG_CPT |
SDE_PORTB_HOTPLUG_CPT |
SDE_PORTC_HOTPLUG_CPT |
-   SDE_PORTD_HOTPLUG_CPT);
+   SDE_PORTD_HOTPLUG_CPT |
+   SDE_GMBUS_CPT);
dev_priv->pch_irq_mask = ~hotplug_mask;
 
I915_WRITE(SDEIIR, I915_READ(SDEIIR));
@@ -1959,6 +1967,7 @@ static int valleyview_irq_postinstall(struct drm_device 
*dev)
POSTING_READ(VLV_IER);
 
i915_enable_pipestat(dev_priv, 0, pipestat_enable);
+   i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_INTERRUPT_STATUS);
i915_enable_pipestat(dev_priv, 1, pipestat_enable);
 
I915_WRITE(VLV_IIR, 0x);
@@ -2454,6 +2463,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
 
dev_priv->pipestat[0] = 0;
dev_priv->pipestat[1] = 0;
+   i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_INTERRUPT_STATUS);
 
/*
 * Enable some error detection, note the instruction error mask
-- 
1.7.11.2

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


[Intel-gfx] [PATCH 1/4] drm/i915: extract gmbus_wait_hw_status

2012-09-06 Thread Daniel Vetter
The gmbus interrupt generation is rather fiddly: We can only ever
enable one interrupt source (but we always want to check for NAK
in addition to the real bit). And the bits in the gmbus status
register don't map at all to the bis in the irq register.

To prepare for this mess, start by extracting the hw status wait
loop into it's own function, consolidate the NAK error handling a
bit. To keep things flexible, pass in the status bit we care about
(in addition to any NAK signalling).

v2: I've failed to notice that the sens of GMBUS_ACTIVE is inverted,
Chris Wilson gladly pointed that out for me. To keep things simple,
ignore that case for  now (we only need to idle the gmbus controller
at the end of an entire i2c transaction, not after every message).

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_i2c.c | 46 ++--
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index b9755f6..57decac 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -204,6 +204,24 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
 }
 
 static int
+gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
+u32 gmbus2_status)
+{
+   int ret;
+   int reg_offset = dev_priv->gpio_mmio_base;
+   u32 gmbus2;
+
+   ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
+  (GMBUS_SATOER | gmbus2_status),
+  50);
+
+   if (gmbus2 & GMBUS_SATOER)
+   return -ENXIO;
+
+   return ret;
+}
+
+static int
 gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
u32 gmbus1_index)
 {
@@ -220,15 +238,10 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct 
i2c_msg *msg,
while (len) {
int ret;
u32 val, loop = 0;
-   u32 gmbus2;
 
-   ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-  (GMBUS_SATOER | GMBUS_HW_RDY),
-  50);
+   ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
if (ret)
-   return -ETIMEDOUT;
-   if (gmbus2 & GMBUS_SATOER)
-   return -ENXIO;
+   return ret;
 
val = I915_READ(GMBUS3 + reg_offset);
do {
@@ -262,7 +275,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct 
i2c_msg *msg)
   GMBUS_SLAVE_WRITE | GMBUS_SW_RDY);
while (len) {
int ret;
-   u32 gmbus2;
 
val = loop = 0;
do {
@@ -271,13 +283,9 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct 
i2c_msg *msg)
 
I915_WRITE(GMBUS3 + reg_offset, val);
 
-   ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-  (GMBUS_SATOER | GMBUS_HW_RDY),
-  50);
+   ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
if (ret)
-   return -ETIMEDOUT;
-   if (gmbus2 & GMBUS_SATOER)
-   return -ENXIO;
+   return ret;
}
return 0;
 }
@@ -346,8 +354,6 @@ gmbus_xfer(struct i2c_adapter *adapter,
I915_WRITE(GMBUS0 + reg_offset, bus->reg0);
 
for (i = 0; i < num; i++) {
-   u32 gmbus2;
-
if (gmbus_is_index_read(msgs, i, num)) {
ret = gmbus_xfer_index_read(dev_priv, &msgs[i]);
i += 1;  /* set i to the index of the read xfer */
@@ -362,13 +368,11 @@ gmbus_xfer(struct i2c_adapter *adapter,
if (ret == -ENXIO)
goto clear_err;
 
-   ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-  (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE),
-  50);
+   ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE);
+   if (ret == -ENXIO)
+   goto clear_err;
if (ret)
goto timeout;
-   if (gmbus2 & GMBUS_SATOER)
-   goto clear_err;
}
 
/* Generate a STOP condition on the bus. Note that gmbus can't generata
-- 
1.7.11.2

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


Re: [Intel-gfx] [PATCH 48/58] drm/i915: use staged outuput config in tv->mode_fixup

2012-09-06 Thread Daniel Vetter
On Wed, Sep 05, 2012 at 11:02:15AM -0700, Jesse Barnes wrote:
> On Sun, 19 Aug 2012 21:13:05 +0200
> Daniel Vetter  wrote:
> 
> > The "is this encoder cloned" check will be reused by the lvds encoder,
> > hence exract it.
> > 
> > v2: Be a bit more careful about that we need to check the new, staged
> > ouput configuration in the check_non_cloned helper ...
> > 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 22 ++
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_tv.c  |  7 ++-
> >  3 files changed, 25 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index c7bd573..c59569e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6563,6 +6563,28 @@ static struct drm_crtc_helper_funcs 
> > intel_helper_funcs = {
> > .disable = intel_crtc_disable,
> >  };
> >  
> > +bool intel_encoder_check_non_cloned(struct intel_encoder *encoder)
> > +{
> > +   struct intel_encoder *other_encoder;
> > +   struct drm_crtc *crtc = &encoder->new_crtc->base;
> > +
> > +   if (WARN_ON(!crtc))
> > +   return true;
> > +
> > +   list_for_each_entry(other_encoder,
> > +   &crtc->dev->mode_config.encoder_list,
> > +   base.head) {
> > +
> > +   if (&other_encoder->new_crtc->base != crtc ||
> > +   encoder == other_encoder)
> > +   continue;
> > +   else
> > +   return false;
> > +   }
> > +
> > +   return true;
> > +}
> 
> encoder_is_cloned() would make the callers more readable and avoid the
> double negative...

Ok, got a bit bored and applied this bikeshed here, too. Won't bother with
resending though.
-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 00/58] modeset-rework, the basic conversion

2012-09-06 Thread Daniel Vetter
On Thu, Sep 06, 2012 at 08:55:40AM +0200, Daniel Vetter wrote:
> On Wed, Sep 05, 2012 at 04:23:55PM -0700, Jesse Barnes wrote:
> > On Sun, 19 Aug 2012 21:12:17 +0200
> > Daniel Vetter  wrote:
> > 
> > > Hi all,
> > > 
> > > Changes since last time around:
> > > - The prep patches are all merged now.
> > > - I've left out the actual DP fixes/cleanups, I think we should merge 
> > > those in a
> > >   separte step.
> > > - A few bugfixes (thanks to Paulo, Jani and Chris).
> > > - I've also applied a few bikesheds for naming that Paulo suggested (but 
> > > I'm not
> > >   sure whether I've sent those out already in a previous patchbomb).
> > > 
> > > Essentially this is just the core rework, which addes the new 
> > > get_hw_state code,
> > > refactors all the encoders to use the new functions and finally reworks 
> > > the
> > > modeset logic to disable/enable entire pipes, always (and with a 
> > > deterministic
> > > order).
> > > 
> > > For merging to -next, I plan to pull in everything with a real merge 
> > > commit. For
> > > that reason I've put up a modeset-rework-base branch onto my private fdo 
> > > repo[1].
> > > That way I can put a short documentation for the new modeset design into 
> > > the
> > > merge commit (stichted together from the previous patchbomb cover 
> > > letters),
> > > documenting my folly assumptions for eternity.
> > > 
> > > I'll also plan to put tags for the entire series in the merge commit, so 
> > > if you
> > > have tested this on a few machines, read through and agree with the new 
> > > designs,
> > > please reply with your tested-by/acked-by/reviewed-by tags.
> > > 
> > > Flames, comments and test reports highly welcome.
> > 
> > Ok I've tested on Ironlake, Cantiga, Crestline, and Pineview so far and
> > things look good.  I ran testdisplay both with and without VGA
> > attached (the ILK has a eDP panel), and tried S3 and S4 both with and
> > without VGA both in the console and in X.
> > 
> > There was one issue on Pineview where the VGA seemed to get
> > "forgotten", but I haven't isolated it yet.  I canceled testdisplay
> > part way through and that seemed to confuse fbcon about what was there.
> 
> QA reported a similar issue where fbcon refuses to light up the display
> after testdisplay completed a while back:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=42194
> 
> > I also see an issue with 1280x800 modes across all platforms, but that
> > may just be the monitor, I need to test more.
> > 
> > I'm testing Montara (that's 855 for you youngsters) now, but it's slow
> > so not all my builds have completed there yet.
> > 
> > Overall though:
> > Tested-by: Jesse Barnes 
> 
> Cool, thanks a lot for review&testing. I'll do the merge now (need to slap
> all your r-b's onto patches first).

Actually I'm still lacking an r-b on two patches ... I'll annoy you again
later today.
-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