Re: [Intel-gfx] [PATCH 9/9] drm/i915/bdw: Enable RC6

2014-02-06 Thread S, Deepak

On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

It is tested and looking fairly stable now, so turn it on. It wasn't
intentionally turned off originally :P

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index a5c9142..377bd7f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4428,7 +4428,7 @@ void intel_enable_gt_powersave(struct
drm_device *dev)
 ironlake_enable_drps(dev);
 ironlake_enable_rc6(dev);
 intel_init_emon(dev);
-   } else if (IS_GEN6(dev) || IS_GEN7(dev)) {
+   } else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_BROADWELL(dev)) {
 /*
  * PCU communication is slow and this doesn't need
to be
  * done at any specific time, so do this out of our
fast path
--
1.8.5.3

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



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


Re: [Intel-gfx] [PATCH 8/9] drm/i915/bdw: Implement a basic PM interrupt handler

2014-02-06 Thread S, Deepak

On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.

The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.

This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/i915_irq.c  | 80
++--
  drivers/gpu/drm/i915/i915_reg.h  |  1 +
  drivers/gpu/drm/i915/intel_drv.h |  2 +
  drivers/gpu/drm/i915/intel_pm.c  | 39 +++-
  4 files changed, 117 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 72ade87..ab0c7ac 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -214,6 +214,53 @@ static bool ivb_can_enable_err_int(struct
drm_device *dev)
 return true;
  }

+/**
+  * bdw_update_pm_irq - update GT interrupt 2
+  * @dev_priv: driver private
+  * @interrupt_mask: mask of interrupt bits to update
+  * @enabled_irq_mask: mask of interrupt bits to enable
+  *
+  * Copied from the snb function, updated with relevant register
offsets
+  */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   if (dev_priv->pc8.irqs_disabled) {
+   WARN(1, "IRQs disabled\n");
+   dev_priv->pc8.regsave.gen6_pmimr &= ~interrupt_mask;
+   dev_priv->pc8.regsave.gen6_pmimr |= (~enabled_irq_mask &
+interrupt_mask);
+   return;
+   }
+
+   new_val = dev_priv->pm_irq_mask;
+   new_val &= ~interrupt_mask;
+   new_val |= (~enabled_irq_mask & interrupt_mask);
+
+   if (new_val != dev_priv->pm_irq_mask) {
+   dev_priv->pm_irq_mask = new_val;
+   I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+  dev_priv->pm_irq_mask);
+   POSTING_READ(GEN8_GT_IMR(2));
+   }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t
mask)
+{
+   bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t
mask)
+{
+   bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
  static bool cpt_can_enable_serr_int(struct drm_device *dev)
  {
 struct drm_i915_private *dev_priv = dev->dev_private;
@@ -997,11 +1044,15 @@ static void gen6_pm_rps_work(struct
work_struct *work)
 pm_iir = dev_priv->rps.pm_iir;
 dev_priv->rps.pm_iir = 0;
 /* Make sure not to corrupt PMIMR state used by ringbuffer
code */
-   snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+   if (IS_BROADWELL(dev_priv->dev))
+   bdw_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+   else {
+   /* Make sure we didn't queue anything we're not
going to process. */
+   snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+   WARN_ON(pm_iir & ~GEN6_PM_RPS_EVENTS);
+   }
 spin_unlock_irq(&dev_priv->irq_lock);

-   /* Make sure we didn't queue anything we're not going to
process. */
-   WARN_ON(pm_iir & ~GEN6_PM_RPS_EVENTS);

 if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
 return;
@@ -1192,6 +1243,19 @@ static void snb_gt_irq_handler(struct
drm_device *dev,
 ivybridge_parity_error_irq_handler(dev, gt_iir);
  }

+static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv,
u32 pm_iir)
+{
+   if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
+   return;
+
+   spin_lock(&dev_priv->irq_lock);
+   dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS;
+   bdw_disable_pm_irq(dev_priv, pm_iir & GEN6_PM_RPS_EVENTS);
+   spin_unlock(&dev_priv->irq_lock);
+
+  

Re: [Intel-gfx] Request for feedback - Sprite flip notification support

2014-02-06 Thread Vijay Purushothaman

On 2/6/2014 12:28 PM, Vijay Purushothaman wrote:

On 2/5/2014 10:18 PM, Ville Syrjälä wrote:

On Wed, Feb 05, 2014 at 09:25:36PM +0530, Vijay Purushothaman wrote:

On 2/5/2014 8:43 PM, Ville Syrjälä wrote:

On Wed, Feb 05, 2014 at 08:35:11PM +0530, Vijay Purushothaman wrote:

Hello,

In our current driver implementation we support flip notifications
only
for primary plane. So, in a full screen video playback scenario where
only one sprite plane is active, the user space is forced to rely on
primary plane flip notification even though there is no real need for
this plane to be active. Ideally we should be able to support flip
notifications for any given plane. Switching off the primary plane
(when
not used) will help in better memory self refresh & decent power
savings..

We do have a hack in android product trees which supports flip
notifications for one sprite plane. unfortunately this hack in its
current form cannot be considered for up streaming...

My current thinking is to have an array of unpin_work items to
match the
number of planes. Is anyone working on this or thought about this
scenario in detail? Any pointers / restrictions that needs to
considered
for a generic implementation of this feature?


The plan is to implement the nuclear page flip which will take care of
all planes in the same way.


Thanks Ville. If the nuclear page flip is part of your bigger atomic
mode set framework, is there a way you can split this into smaller sets
for merge? Multiple product trees will benefit from the nuclear page
flip.


I've split things up already somewhat. Some has landed some has not.

Currently I have my minimal "atomic update of sprite+primary during
setplane" series I need to get in. It shouldn't need major work anymore,
just some minor tweaks. But I realized I need to limit this to just
pch platforms for now. Making it work reliably on gmch platforms
require some extra interrupt related work. The main thing here is that
it adds the mechanism to do the update atomically for the entire pipe.

After that I need to post the last bits of my watermark update saga.
This too will initially be limited to pch platforms only. Obviously
watermarks need to updated correctly to avoid underruns when planes
get shuffled around.


Is there anything that i can help with? Like testing your patches with
android user space?


There's nothing to test at this point unless you want to test my old
branch from year ago or something.

What needs to be done:
- review the latest atomic framework patches from Rob Clark
- expose primary/cursor planes as drm_planes
   * this could in theory be skipped, but it'll lead to cruft in the API
 we need to maintain until the end of time. Also I think restructing
 stuff internally to this direction will be a good idea anyway to
make
 the nuclear flip code neater. This more or less involves collecting
 the plane state to some plane_config type of structure, and being
 able to pre-compute that
- try to collect the necessary missing bits from my last
   atomic branch to implement the nuclear flip
   * the flip helper thing to update an arbitrary collection of planes
 atomically, maybe could be simplified a bit
   * mechanism to queue nuclear flips and make them wait for the
 GPU to finish writing to all the relevant buffers before issuing
 the actual flips/updates
- finally hook up the atomic ioctl to do the nuclear flip
   * pre-pin all buffers, pre-compute plane configs, pre-compute
 watermarks, check everything, wait for the GPU, and finally
 do the update

For the atomic modeset side some of that's the same really. There too we
also need to pre-compute the plane configs and pre-pin buffers. Most of
the rest we already pre-compute via the pipe config. One major thing
left out of the pipe config pre-compute currently is PLLs. We compute
that stuff way too late still. We also need to massage the modeset code
some more to make it capable of modesetting multiple pipes at once.



Thanks for the detailed answer. This should solve most of the display
issues in the product trees.. considering the amount of work involved
this looks more like a long term solution. Would it be okay if we submit
a short term solution for sprite flip notifications? This would help us
to force a standard approach across multiple android product kernels. We
can revert this fix once the atomic mode set / nuclear page flip is ready.


Ville / Daniel,

Ping.. Could you please suggest whether this is okay? If you think this 
is not worth or if nuclear page flip is on the horizon i will focus on 
display self refresh patches..


Thanks,
Vijay

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


Re: [Intel-gfx] [PATCH 7/9] drm/i915/bdw: RPS frequency bits are the same as HSW

2014-02-06 Thread S, Deepak

On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 34cc898..deaaaf2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3016,7 +3016,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)

 gen6_set_rps_thresholds(dev_priv, val);

-   if (IS_HASWELL(dev))
+   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 I915_WRITE(GEN6_RPNSWREQ,
HSW_FREQUENCY(val));
 else
--
1.8.5.3

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


Looks fine

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


Re: [Intel-gfx] [PATCH 5/9] drm/i915/bdw: Set rp_state_caps

2014-02-06 Thread S, Deepak


On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 20 +---
  1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 6acb429..ae59bd9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3184,6 +3184,17 @@ static void gen6_enable_rps_interrupts(struct
drm_device *dev)
 I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
  }

+static void parse_rp_state_cap(struct drm_i915_private *dev_priv,
u32 rp_state_cap)
+{
+   /* In units of 50MHz */
+   dev_priv->rps.hw_max = dev_priv->rps.max_delay =
rp_state_cap & 0xff;
+   dev_priv->rps.min_delay = (rp_state_cap >> 16) & 0xff;
+   dev_priv->rps.rp1_delay = (rp_state_cap >>  8) & 0xff;
+   dev_priv->rps.rp0_delay = (rp_state_cap >>  0) & 0xff;
+   dev_priv->rps.rpe_delay = dev_priv->rps.rp1_delay;
+   dev_priv->rps.cur_delay = 0;
+}
+
  static void gen8_enable_rps(struct drm_device *dev)
  {
 struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3202,6 +3213,7 @@ static void gen8_enable_rps(struct drm_device
*dev)
 I915_WRITE(GEN6_RC_CONTROL, 0);

 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+   parse_rp_state_cap(dev_priv, rp_state_cap);

 /* 2b: Program RC6 thresholds.*/
 I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
@@ -3288,13 +3300,7 @@ static void gen6_enable_rps(struct drm_device
*dev)
 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);

-   /* In units of 50MHz */
-   dev_priv->rps.hw_max = dev_priv->rps.max_delay =
rp_state_cap & 0xff;
-   dev_priv->rps.min_delay = (rp_state_cap >> 16) & 0xff;
-   dev_priv->rps.rp1_delay = (rp_state_cap >>  8) & 0xff;
-   dev_priv->rps.rp0_delay = (rp_state_cap >>  0) & 0xff;
-   dev_priv->rps.rpe_delay = dev_priv->rps.rp1_delay;
-   dev_priv->rps.cur_delay = 0;
+   parse_rp_state_cap(dev_priv, rp_state_cap);

 /* disable the counters and set deterministic thresholds */
 I915_WRITE(GEN6_RC_CONTROL, 0);
--
1.8.5.3

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



Looks fine
Reviewed-by: Deepak S 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/9] drm/i915/bdw: Use centralized rc6 info print

2014-02-06 Thread S, Deepak


On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 944b99c..6acb429 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct
drm_device *dev)
 /* 3: Enable RC6 */
 if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
-   DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ?
"on" : "off");
+   intel_print_rc6_info(dev, rc6_mask);
 I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-   GEN6_RC_CTL_EI_MODE(1) |
-   rc6_mask);
+   GEN6_RC_CTL_EI_MODE(1) |
+   rc6_mask);

 /* 4 Program defaults and thresholds for RPS*/
 I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request
500 MHz */
--
1.8.5.3

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


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


Re: [Intel-gfx] [PATCH 3/9] drm/i915: Just print rc6 facts

2014-02-06 Thread S, Deepak



On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

Everything can be overridden by module parameters, so don't confuse the
users that are using them.

We have RC6 turned on for all platforms which support it, but Ironlake,
so the need to explain the situation is no longer pressing.

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 258241b..944b99c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3135,16 +3135,10 @@ static void valleyview_disable_rps(struct
drm_device *dev)

  static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
  {
-   if (IS_GEN6(dev))
-   DRM_DEBUG_DRIVER("Sandybridge: deep RC6 disabled\n");
-
-   if (IS_HASWELL(dev))
-   DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
-
 DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
-   (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
-   (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
-   (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" :
"off");
+(mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
+(mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
+(mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
  }

  int intel_enable_rc6(const struct drm_device *dev)
--
1.8.5.3

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



Reviewed-by: Deepak S 

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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

2014-02-06 Thread Goel, Akash
> Don't cause us to rewrite the PTE for the batch buffer between each 
> execbuffer (ro for the batch, rw next time it gets used as a texture).
> In fact, do not change ro without user intervention.
> gt_old_ro is unused
Sorry we didn't consider this scenario, that a batch buffer could be 
subsequently used as a different type of buffer also, like as a texture buffer. 
But is the remap going to have a significant overhead.

>> Use byt_pte_encode() instead of hacking the result of ppgtt->pte_encode().
>> Consider expanding i915_cache_level so that it included the concept of 
>> PROT_READ | PROT_WRITE.
Thanks, actually we initially thought of doing it in a same way. Can we 
overload the i915_cache_level enum with the RO flag, while calling 
'insert_entries' for VALLEYVIEW platform. 

>> Also, what's the exact use-case for this here? And if we need to expose this 
>> to userspace, then it needs a testcase.
>> -Daniel
Since we have this RO bit available for our disposal on BYT platform, we 
thought we can avail it in order to have an extra protection on the buffers.
We initially used it only for Ring & Batch buffers as we know, without User's 
input, that they are supposed to be read-only from GPU side.
For extending this for other buffers, we need a Libdrm level change. 
Or can we use the Read/Write domains information in relocation entries to 
decide this internally on the Driver side, but that will require some change in 
the exec-buffer path.

Best Regards
Akash

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Thursday, February 06, 2014 11:06 PM
To: Chris Wilson; Goel, Akash; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit 
support

On Thu, Feb 06, 2014 at 11:56:37AM +, Chris Wilson wrote:
> On Thu, Feb 06, 2014 at 10:22:28AM +, Goel, Akash wrote:
> > Please kindly review this patch.
> > 
> > Best regards
> > Akash
> > -Original Message-
> > From: Goel, Akash
> > Sent: Thursday, January 09, 2014 5:55 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Goel, Akash
> > Subject: [PATCH] drm/i915/vlv: Added write-enable pte bit support
> > 
> > From: Akash Goel 
> > 
> > This adds support for using the write-enable bit in the GTT entry for VLV.
> > This is handled via a read-only flag in the GEM buffer object which is then 
> > used to check if the write-enable bit has to be set or not when writing the 
> > GTT entries.
> > Currently by default only the Batch buffer & Ring buffers are being marked 
> > as read only.
> 
> Don't cause us to rewrite the PTE for the batch buffer between each 
> execbuffer (ro for the batch, rw next time it gets used as a texture).
> In fact, do not change ro without user intervention.
> 
> gt_old_ro is unused
> 
> Use byt_pte_encode() instead of hacking the result of
> ppgtt->pte_encode().
> 
> Consider expanding i915_cache_level so that it included the concept of 
> PROT_READ | PROT_WRITE.

Also, what's the exact use-case for this here? And if we need to expose this to 
userspace, then it needs a testcase.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/9] drm/i915: Stop pretending VLV has rc6+

2014-02-06 Thread S, Deepak



On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

It wasn't ever used by the caller anyway with the exception of what we
show in sysfs.

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index bcbdac2..258241b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3161,7 +3161,7 @@ int intel_enable_rc6(const struct drm_device *dev)
 if (INTEL_INFO(dev)->gen == 5)
 return 0;

-   if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev))
+   if (IS_IVYBRIDGE(dev))
 return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
 else
 return INTEL_RC6_ENABLE;
--
1.8.5.3

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


I think, we can avoid else condition and return INTEL_RC6_ENABLE if the 
other platform checks fails?


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


Re: [Intel-gfx] [PATCH 1/9] drm/i915: Clarify RC6 enabling

2014-02-06 Thread S, Deepak


On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky

mailto:benjamin.widaw...@intel.com>> wrote:

At one time, we though all future platforms would have the deeper RC6
states. As it turned out, they killed it after Ivybridge, and began
using other means to achieve the power savings (the stuff we need to get
to PC7+).

The enable function was left in a weird state of odd corner cases as a
result. Since the future is now, and we also have some insight into
what's currently the future, we have an opportunity to simplify, and
future proof the function.

NOTE: VLV will be addressed in a subsequent patch. This patch was trying
not to change functionality.

NOTE2: All callers sanitize the return value anyway, so this patch is
simply to have the code make a bit more sense.

Signed-off-by: Ben Widawsky mailto:b...@bwidawsk.net>>
---
  drivers/gpu/drm/i915/intel_pm.c | 10 +++---
  1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 53d64bb..bcbdac2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3161,14 +3161,10 @@ int intel_enable_rc6(const struct drm_device
*dev)
 if (INTEL_INFO(dev)->gen == 5)
 return 0;

-   if (IS_HASWELL(dev))
-   return INTEL_RC6_ENABLE;
-
-   /* snb/ivb have more than one rc6 state. */
-   if (INTEL_INFO(dev)->gen == 6)
+   if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev))
+   return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
+   else
 return INTEL_RC6_ENABLE;
-
-   return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
  }

  static void gen6_enable_rps_interrupts(struct drm_device *dev)
--
1.8.5.3

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





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


Re: [Intel-gfx] 3.13.1 - WARNING at drivers/gpu/drm/i915/i915_irq.c:1240

2014-02-06 Thread Thomas Meyer
Am Dienstag, den 04.02.2014, 21:17 +0100 schrieb Daniel Vetter:
> On Tue, Feb 04, 2014 at 08:37:02PM +0100, Thomas Meyer wrote:
> > 
> > Hi,
> > 
> > I see a *lot* of these warning in 3.13.1.
> > 3.12.x never showed this problem.
> > Any ideas?!
> 
> Can you please try latest the drm-intel-nightly git branch from
> git://anongit.freedesktop.org/drm-intel ?
> 
> If that one's still affected then please boot with drm.debug=0xe added to
> your kernel cmdline, reproduce the WARN and attach the complete dmesg.
> 

Hi,

with 3.13.0-drm-00890-g4055ebc (drm-intel-nightly) I get these warnings:

[0.550068] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[0.550070] [drm] Driver supports precise vblank timestamp query.
[0.550146] vgaarb: device changed decodes: 
PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[0.621056] [drm:i965_irq_handler] *ERROR* pipe A underrun
[0.860335] fbcon: inteldrmfb (fb0) is primary device
[0.940094] [ cut here ]
[0.940103] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/intel_panel.c:714 
i965_enable_backlight+0x12a/0x150()
[0.940103] backlight already enabled
[0.940105] Modules linked in:
[0.940109] CPU: 0 PID: 1 Comm: swapper Not tainted 
3.13.0-drm-00890-g4055ebc #122
[0.940110] Hardware name: Acer Aspire 1810T/JM11-MS, BIOS v1.3310 03/25/2010
[0.940113]  0009 8802368a1448 81546104 
8802368a1480
[0.940115]  810391fc 880234ff4000  
880234fb8400
[0.940118]  c000 880236a11c00 8802368a14e0 
81039267
[0.940119] Call Trace:
[0.940125]  [] dump_stack+0x19/0x1b
[0.940129]  [] warn_slowpath_common+0x6c/0x90
[0.940132]  [] warn_slowpath_fmt+0x47/0x50
[0.940135]  [] ? schedule_timeout+0xf9/0x190
[0.940138]  [] i965_enable_backlight+0x12a/0x150
[0.940141]  [] ? gen4_write64+0x30/0x30
[0.940144]  [] intel_panel_enable_backlight+0x96/0xd0
[0.940146]  [] intel_enable_lvds+0x158/0x170
[0.940148]  [] ? gen4_write64+0x30/0x30
[0.940152]  [] i9xx_crtc_enable+0x2f1/0x420
[0.940155]  [] __intel_set_mode+0x8af/0x1660
[0.940159]  [] intel_set_mode+0x11/0x30
[0.940162]  [] intel_crtc_set_config+0x984/0xc70
[0.940165]  [] ? 
set_inverse_trans_unicode.isra.5+0xb8/0xd0
[0.940169]  [] drm_mode_set_config_internal+0x5d/0xe0
[0.940173]  [] drm_fb_helper_set_par+0x69/0xf0
[0.940177]  [] fbcon_init+0x504/0x580
[0.940180]  [] visual_init+0xb3/0x110
[0.940183]  [] do_bind_con_driver+0x153/0x320
[0.940186]  [] do_take_over_console+0x114/0x1c0
[0.940188]  [] do_fbcon_takeover+0x5b/0xc0
[0.940191]  [] fbcon_event_notify+0x60d/0x720
[0.940194]  [] notifier_call_chain+0x4c/0x70
[0.940197]  [] __blocking_notifier_call_chain+0x48/0x70
[0.940200]  [] blocking_notifier_call_chain+0x11/0x20
[0.940204]  [] fb_notifier_call_chain+0x16/0x20
[0.940207]  [] register_framebuffer+0x1e6/0x320
[0.940211]  [] drm_fb_helper_initial_config+0x327/0x500
[0.940214]  [] ? intel_modeset_setup_hw_state+0x8e8/0xbe0
[0.940217]  [] ? __kmalloc+0xc4/0xf0
[0.940220]  [] ? kmem_cache_alloc+0xac/0xb0
[0.940224]  [] intel_fbdev_initial_config+0x1c/0x20
[0.940227]  [] i915_driver_load+0xda1/0xde0
[0.940231]  [] drm_dev_register+0x76/0x160
[0.940234]  [] drm_get_pci_dev+0x9b/0x210
[0.940237]  [] i915_pci_probe+0x36/0x50
[0.940240]  [] pci_device_probe+0x7c/0xe0
[0.940244]  [] driver_probe_device+0x6d/0x230
[0.940247]  [] __driver_attach+0x8b/0x90
[0.940250]  [] ? __device_attach+0x40/0x40
[0.940253]  [] bus_for_each_dev+0x63/0xa0
[0.940256]  [] driver_attach+0x19/0x20
[0.940258]  [] bus_add_driver+0x170/0x220
[0.940262]  [] ? drm_core_init+0x109/0x109
[0.940265]  [] driver_register+0x5f/0xf0
[0.940268]  [] ? drm_core_init+0x109/0x109
[0.940271]  [] __pci_register_driver+0x3a/0x40
[0.940273]  [] drm_pci_init+0x115/0x130
[0.940276]  [] ? drm_core_init+0x109/0x109
[0.940279]  [] i915_init+0x6a/0x6c
[0.940281]  [] do_one_initcall+0x10a/0x160
[0.940284]  [] ? parse_args+0x1e8/0x320
[0.940287]  [] kernel_init_freeable+0x142/0x1d1
[0.940290]  [] ? do_early_param+0x88/0x88
[0.940292]  [] ? rest_init+0x80/0x80
[0.940295]  [] kernel_init+0x9/0x120
[0.940297]  [] ret_from_fork+0x7a/0xb0
[0.940300]  [] ? rest_init+0x80/0x80
[0.940305] ---[ end trace 529feadd43242174 ]---
[1.002905] Console: switching to colour frame buffer device 170x48

And this one, but I'm not sure if this one is related:

[1.886833] ata2: SATA link down (SStatus 0 SControl 300)
[1.920205] irq 16: nobody cared (try booting with the "irqpoll" option)
[1.920269] CPU: 0 PID: 56 Comm: irq/16-uhci_hcd Tainted: GW
3.13.0-drm-00890-g4055ebc #122
[1.920271] Hardware name: Acer Aspire 1810T/JM11-MS, BIOS v1.3310 03/25/2010
[1.920274]  880236a

Re: [Intel-gfx] [PATCH 6/6] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v9

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 09:47:47PM +, Jesse Barnes wrote:
> On Wed, 5 Feb 2014 18:14:15 +
> Chris Wilson  wrote:
> 
> > On Wed, Feb 05, 2014 at 05:30:48PM +, Jesse Barnes wrote:
> > > +static bool intel_fbdev_init_bios(struct drm_device *dev,
> > > +  struct intel_fbdev *ifbdev)
> > > +{
> > > + struct intel_framebuffer *fb = NULL;
> > > + struct drm_crtc *crtc;
> > > + struct intel_crtc *intel_crtc;
> > > + struct intel_plane_config *plane_config = NULL;
> > > + unsigned int last_size = 0, max_size = 0, tmp;
> > > +
> > > + /* Find the largest framebuffer to use, then free the others */
> > > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > > + intel_crtc = to_intel_crtc(crtc);
> > > +
> > > + if (!intel_crtc->active || !intel_crtc->plane_config.fb) {
> > > + DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
> > > +   pipe_name(intel_crtc->pipe));
> > > + continue;
> > > + }
> > > +
> > > + tmp = intel_crtc->config.adjusted_mode.crtc_hdisplay *
> > > + intel_crtc->config.adjusted_mode.crtc_vdisplay *
> > > + intel_crtc->plane_config.fb->base.pitches[0];
> > 
> > This reads as width * height * stride. I presume you meant bpp/8 here,
> > but since we keep the fb and its pitch intact, you need height * stride.
> > Actually, it preserves a completely different fb, so this estimate of
> > size is incomplete.
> 
> Right, I meant to just use pitches here.  And fb could be NULL, so I
> need to handle that case.  What do you mean by "it preserves a
> completely different fb"?

tmp here is meant to be the size required for this pipe in the target
framebuffer, but we are calculating the size required in its current
framebuffer. They may be different.
 
> > 
> > > + max_size = max(max_size, tmp);
> > > +
> > > + /*
> > > +  * Make sure the fb will fit the biggest active pipe.  If
> > > +  * not, clear out any previous usage.
> > > +  */
> > > + if (intel_crtc->plane_config.size > last_size &&
> > > + intel_crtc->plane_config.size >= max_size) {
> > > + plane_config = &intel_crtc->plane_config;
> > > + last_size = plane_config->size;
> > > + fb = plane_config->fb;
> > > + } else {
> > > + plane_config = NULL;
> > > + fb = NULL;
> > > + }
> > 
> > Two CRTCs sharing a plane_config will now end up with plane_config/fb =
> > NULL, and so bail out. This is confusing me. If we here just found the
> > largest fb and then did a second pass confirming that all CRTC and planes
> > fitted into it, I would find it easier to understand.
> 
> Ah yeah, I was trying to be tricky, doing this in one pass.  But doing
> it in two is better and clearer (I had this earlier but then thought
> I'd be bikeshedded into a single pass, oh well).

I thought you had tried to reduce the two-passes into one, but I think
it has come unstuck.
 
> > > + }
> > > +
> > > + /* Free unused fbs */
> > > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > > + struct intel_framebuffer *cur_fb;
> > > +
> > > + intel_crtc = to_intel_crtc(crtc);
> > > + cur_fb = intel_crtc->plane_config.fb;
> > > +
> > > + if (cur_fb && cur_fb != fb)
> > > + intel_framebuffer_fini(cur_fb);
> > > + }
> > > +
> > > + if (!fb) {
> > > + DRM_DEBUG_KMS("no active pipes found, not using BIOS config\n");
> > > + goto out;
> > > + }
> > > +
> > > + ifbdev->preferred_bpp = plane_config->fb->base.bits_per_pixel;
> > > + ifbdev->helper.funcs = &intel_fb_helper_funcs;
> > > + ifbdev->helper.funcs->initial_config = intel_fb_initial_config;
> > > + ifbdev->fb = fb;
> > > +
> > > + /* Assuming a single fb across all pipes here */
> > > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > > + intel_crtc = to_intel_crtc(crtc);
> > > +
> > > + if (!intel_crtc->active)
> > > + continue;
> > > +
> > > + /*
> > > +  * This should only fail on the first one so we don't need
> > > +  * to cleanup any secondary crtc->fbs
> > > +  */
> > > + if (intel_pin_and_fence_fb_obj(dev, fb->obj, NULL))
> > > + goto out_unref_obj;
> > > +
> > > + crtc->fb = &fb->base;
> > > + drm_gem_object_reference(&fb->obj->base);
> > > + drm_framebuffer_reference(&fb->base);
> > > + }
> > > +
> > > + DRM_DEBUG_KMS("using BIOS fb for initial console\n");
> > > + return true;
> > 
> > Somewhere around here we must disable all CRTCs that are active and have
> > no fb.
> 
> You mean on error?  Otherwise that shouldn't be possible...  we would
> have failed earlier on when we failed to find an fb suitable for all
> the active pipes (or so goes the theory).  Unless I'm missing some
> other i

Re: [Intel-gfx] [PATCH 6/6] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v9

2014-02-06 Thread Jesse Barnes
On Wed, 5 Feb 2014 18:14:15 +
Chris Wilson  wrote:

> On Wed, Feb 05, 2014 at 05:30:48PM +, Jesse Barnes wrote:
> > +static bool intel_fbdev_init_bios(struct drm_device *dev,
> > +struct intel_fbdev *ifbdev)
> > +{
> > +   struct intel_framebuffer *fb = NULL;
> > +   struct drm_crtc *crtc;
> > +   struct intel_crtc *intel_crtc;
> > +   struct intel_plane_config *plane_config = NULL;
> > +   unsigned int last_size = 0, max_size = 0, tmp;
> > +
> > +   /* Find the largest framebuffer to use, then free the others */
> > +   list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > +   intel_crtc = to_intel_crtc(crtc);
> > +
> > +   if (!intel_crtc->active || !intel_crtc->plane_config.fb) {
> > +   DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
> > + pipe_name(intel_crtc->pipe));
> > +   continue;
> > +   }
> > +
> > +   tmp = intel_crtc->config.adjusted_mode.crtc_hdisplay *
> > +   intel_crtc->config.adjusted_mode.crtc_vdisplay *
> > +   intel_crtc->plane_config.fb->base.pitches[0];
> 
> This reads as width * height * stride. I presume you meant bpp/8 here,
> but since we keep the fb and its pitch intact, you need height * stride.
> Actually, it preserves a completely different fb, so this estimate of
> size is incomplete.

Right, I meant to just use pitches here.  And fb could be NULL, so I
need to handle that case.  What do you mean by "it preserves a
completely different fb"?

> 
> > +   max_size = max(max_size, tmp);
> > +
> > +   /*
> > +* Make sure the fb will fit the biggest active pipe.  If
> > +* not, clear out any previous usage.
> > +*/
> > +   if (intel_crtc->plane_config.size > last_size &&
> > +   intel_crtc->plane_config.size >= max_size) {
> > +   plane_config = &intel_crtc->plane_config;
> > +   last_size = plane_config->size;
> > +   fb = plane_config->fb;
> > +   } else {
> > +   plane_config = NULL;
> > +   fb = NULL;
> > +   }
> 
> Two CRTCs sharing a plane_config will now end up with plane_config/fb =
> NULL, and so bail out. This is confusing me. If we here just found the
> largest fb and then did a second pass confirming that all CRTC and planes
> fitted into it, I would find it easier to understand.

Ah yeah, I was trying to be tricky, doing this in one pass.  But doing
it in two is better and clearer (I had this earlier but then thought
I'd be bikeshedded into a single pass, oh well).

> 
> > +   }
> > +
> > +   /* Free unused fbs */
> > +   list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > +   struct intel_framebuffer *cur_fb;
> > +
> > +   intel_crtc = to_intel_crtc(crtc);
> > +   cur_fb = intel_crtc->plane_config.fb;
> > +
> > +   if (cur_fb && cur_fb != fb)
> > +   intel_framebuffer_fini(cur_fb);
> > +   }
> > +
> > +   if (!fb) {
> > +   DRM_DEBUG_KMS("no active pipes found, not using BIOS config\n");
> > +   goto out;
> > +   }
> > +
> > +   ifbdev->preferred_bpp = plane_config->fb->base.bits_per_pixel;
> > +   ifbdev->helper.funcs = &intel_fb_helper_funcs;
> > +   ifbdev->helper.funcs->initial_config = intel_fb_initial_config;
> > +   ifbdev->fb = fb;
> > +
> > +   /* Assuming a single fb across all pipes here */
> > +   list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > +   intel_crtc = to_intel_crtc(crtc);
> > +
> > +   if (!intel_crtc->active)
> > +   continue;
> > +
> > +   /*
> > +* This should only fail on the first one so we don't need
> > +* to cleanup any secondary crtc->fbs
> > +*/
> > +   if (intel_pin_and_fence_fb_obj(dev, fb->obj, NULL))
> > +   goto out_unref_obj;
> > +
> > +   crtc->fb = &fb->base;
> > +   drm_gem_object_reference(&fb->obj->base);
> > +   drm_framebuffer_reference(&fb->base);
> > +   }
> > +
> > +   DRM_DEBUG_KMS("using BIOS fb for initial console\n");
> > +   return true;
> 
> Somewhere around here we must disable all CRTCs that are active and have
> no fb.

You mean on error?  Otherwise that shouldn't be possible...  we would
have failed earlier on when we failed to find an fb suitable for all
the active pipes (or so goes the theory).  Unless I'm missing some
other implication like with the num_pipes vs fbdev ptr thing...

> > +
> > +out_unref_obj:
> > +   intel_framebuffer_fini(fb);
> > +out:
> > +
> > +   return false;
> > +}
> > +
> >  int intel_fbdev_init(struct drm_device *dev)
> >  {
> > struct intel_fbdev *ifbdev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > int ret;
> 
> A useful assertion here would be:
> 
> if (WARN_ON(INTEL_INFO(dev)->num_pipe

[Intel-gfx] [PATCH] tests/gem_gtt_hog: Fix for BDW

2014-02-06 Thread Rodrigo Vivi
Update XY_COLOR_BLT command for Broadwell.

v2: stash devid and remove ugly double allocation. (by Chris).
v3: fix inverted blt command size and stash fd, devid and intel_gen.

Cc: Chris Wilson ch...@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi 
---
 tests/gem_gtt_hog.c | 59 +++--
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 53cd7eb..3149ff4 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -44,30 +44,43 @@
 
 static const uint32_t canary = 0xdeadbeef;
 
+typedef struct data {
+   int fd;
+   int devid;
+   int intel_gen;
+} data_t;
+
 static double elapsed(const struct timeval *start,
  const struct timeval *end)
 {
return 1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - 
start->tv_usec);
 }
 
-static void busy(int fd, uint32_t handle, int size, int loops)
+static void busy(data_t *data, uint32_t handle, int size, int loops)
 {
struct drm_i915_gem_relocation_entry reloc[20];
struct drm_i915_gem_exec_object2 gem_exec[2];
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_pwrite gem_pwrite;
struct drm_i915_gem_create create;
-   uint32_t buf[122], *b;
-   int i;
+   uint32_t buf[170], *b;
+   int i, b_size;
 
memset(reloc, 0, sizeof(reloc));
memset(gem_exec, 0, sizeof(gem_exec));
memset(&execbuf, 0, sizeof(execbuf));
 
b = buf;
+   b_size = sizeof(uint32_t) * (data->intel_gen >= 8 ? 170 : 122);
for (i = 0; i < 20; i++) {
-   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
-   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   if (data->intel_gen >= 8) {
+   *b++ = MI_NOOP;
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 5 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   } else {
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   }
*b++ = 0xf0 << 16 | 1 << 25 | 1 << 24 | 4096;
*b++ = 0;
*b++ = size >> 12 << 16 | 1024;
@@ -76,6 +89,8 @@ static void busy(int fd, uint32_t handle, int size, int loops)
reloc[i].read_domains = I915_GEM_DOMAIN_RENDER;
reloc[i].write_domain = I915_GEM_DOMAIN_RENDER;
*b++ = 0;
+   if (data->intel_gen >= 8)
+   *b++ = 0;
*b++ = canary;
}
*b++ = MI_BATCH_BUFFER_END;
@@ -86,56 +101,55 @@ static void busy(int fd, uint32_t handle, int size, int 
loops)
 
create.handle = 0;
create.size = 4096;
-   drmIoctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
+   drmIoctl(data->fd, DRM_IOCTL_I915_GEM_CREATE, &create);
gem_exec[1].handle = create.handle;
gem_exec[1].relocation_count = 20;
gem_exec[1].relocs_ptr = (uintptr_t)reloc;
 
execbuf.buffers_ptr = (uintptr_t)gem_exec;
execbuf.buffer_count = 2;
-   execbuf.batch_len = sizeof(buf);
+   execbuf.batch_len = b_size;
execbuf.flags = 1 << 11;
-   if (HAS_BLT_RING(intel_get_drm_devid(fd)))
+   if (HAS_BLT_RING(data->devid))
execbuf.flags |= I915_EXEC_BLT;
 
gem_pwrite.handle = gem_exec[1].handle;
gem_pwrite.offset = 0;
-   gem_pwrite.size = sizeof(buf);
+   gem_pwrite.size = b_size;
gem_pwrite.data_ptr = (uintptr_t)buf;
-   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &gem_pwrite) == 0) {
+   if (drmIoctl(data->fd, DRM_IOCTL_I915_GEM_PWRITE, &gem_pwrite) == 0) {
while (loops--)
-   drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
+   drmIoctl(data->fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, 
&execbuf);
}
 
-   drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &create.handle);
+   drmIoctl(data->fd, DRM_IOCTL_GEM_CLOSE, &create.handle);
 }
 
-static void run(int child)
+static void run(data_t *data, int child)
 {
const int size = 4096 * (256 + child * child);
const int tiling = child % 2;
const int write = child % 2;
-   int fd = drm_open_any();
-   uint32_t handle = gem_create(fd, size);
+   uint32_t handle = gem_create(data->fd, size);
uint32_t *ptr;
uint32_t x;
 
igt_assert(handle);
 
if (tiling != I915_TILING_NONE)
-   gem_set_tiling(fd, handle, tiling, 4096);
+   gem_set_tiling(data->fd, handle, tiling, 4096);
 
/* load up the unfaulted bo */
-   busy(fd, handle, size, 100);
+   busy(data, handle, size, 100);
 
/* Note that we ignore the API and rely on the implict
 * set-to-gtt-domain within the fault handler.
 */
if (write) {
-   ptr = gem_mmap(fd, handle, s

Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

2014-02-06 Thread Eric Anholt
akash.g...@intel.com writes:

> From: Akash Goel 
>
> This adds support for using the write-enable bit in the GTT entry for VLV.
> This is handled via a read-only flag in the GEM buffer object which
> is then used to check if the write-enable bit has to be set or not
> when writing the GTT entries.
> Currently by default only the Batch buffer & Ring buffers are being marked
> as read only.

What happens when we GTT-mapped write a batchbuffer that had previously
been silently made RO by the kernel?  Does the CPU take a fault?


pgpDQ1M9TtmWZ.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/5] drm/i915: fix VDD override off wait

2014-02-06 Thread Patrik Jakobsson
On Thu, Feb 6, 2014 at 8:30 PM, Paulo Zanoni  wrote:
> 2014-02-06 15:22 GMT-02:00 Chris Wilson :
>> On Thu, Feb 06, 2014 at 06:16:02PM +0100, Patrik Jakobsson wrote:
>>> On Fri, Dec 6, 2013 at 8:47 PM, Jesse Barnes  
>>> wrote:
>>> > On Fri,  6 Dec 2013 17:32:42 -0200
>>> > Paulo Zanoni  wrote:
>>> >
>>> >> From: Paulo Zanoni 
>>> >>
>>> >> If we're disabling the VDD override bit and the panel is enabled, we
>>> >> don't need to wait for anything. If the panel is disabled, then we
>>> >> need to actually wait for panel_power_cycle_delay, not
>>> >> panel_power_down_delay, because the power down delay was already
>>> >> respected when we disabled the panel.
>>> >>
>>> >> Signed-off-by: Paulo Zanoni 
>>> >> ---
>>> >>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>>> >>  1 file changed, 3 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>>> >> b/drivers/gpu/drm/i915/intel_dp.c
>>> >> index fe327ce..a2aace2 100644
>>> >> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> >> @@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct 
>>> >> intel_dp *intel_dp)
>>> >>   /* Make sure sequencer is idle before allowing subsequent 
>>> >> activity */
>>> >>   DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
>>> >>   I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>>> >> - msleep(intel_dp->panel_power_down_delay);
>>> >> +
>>> >> + if ((pp & POWER_TARGET_ON) == 0)
>>> >> + msleep(intel_dp->panel_power_cycle_delay);
>>> >>   }
>>> >>  }
>>> >>
>>> >
>>> > Lemme check the eDP docs on this one...  it's supposed to be T12, which
>>> > is the time between power cycles.  Yeah that matches what we're using
>>> > elsewhere, so:
>>> >
>>> > Reviewed-by: Jesse Barnes 
>>>
>>> Starting with this patch I get a blank screen when booting my MacBook Air 
>>> 6,2.
>>> Reverting only this patch on top of 3.14-rc1 doesn't help so it's most 
>>> likely
>>> the entire series that needs to be looked at.
>>>
>>> Doing a suspend/resume fixes the problem.
>
> Oh, well, at least we didn't break suspend/resume :)
>
> I just requested some information at the bugzilla entry created by
> Chris. Can you please provide the requested files there, and move the
> discussion to bugzilla?
>
>>
>> I've filed this as https://bugs.freedesktop.org/show_bug.cgi?id=74628
>
> Thanks for doing this! It's too easy to forget about mailing-list-only
> bug reports.

I was just a bit lazy. The dmesg is up on bugzilla.

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


Re: [Intel-gfx] [PATCH 3/5] drm/i915: fix VDD override off wait

2014-02-06 Thread Paulo Zanoni
2014-02-06 15:22 GMT-02:00 Chris Wilson :
> On Thu, Feb 06, 2014 at 06:16:02PM +0100, Patrik Jakobsson wrote:
>> On Fri, Dec 6, 2013 at 8:47 PM, Jesse Barnes  
>> wrote:
>> > On Fri,  6 Dec 2013 17:32:42 -0200
>> > Paulo Zanoni  wrote:
>> >
>> >> From: Paulo Zanoni 
>> >>
>> >> If we're disabling the VDD override bit and the panel is enabled, we
>> >> don't need to wait for anything. If the panel is disabled, then we
>> >> need to actually wait for panel_power_cycle_delay, not
>> >> panel_power_down_delay, because the power down delay was already
>> >> respected when we disabled the panel.
>> >>
>> >> Signed-off-by: Paulo Zanoni 
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>> >>  1 file changed, 3 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> >> b/drivers/gpu/drm/i915/intel_dp.c
>> >> index fe327ce..a2aace2 100644
>> >> --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> @@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct 
>> >> intel_dp *intel_dp)
>> >>   /* Make sure sequencer is idle before allowing subsequent 
>> >> activity */
>> >>   DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
>> >>   I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>> >> - msleep(intel_dp->panel_power_down_delay);
>> >> +
>> >> + if ((pp & POWER_TARGET_ON) == 0)
>> >> + msleep(intel_dp->panel_power_cycle_delay);
>> >>   }
>> >>  }
>> >>
>> >
>> > Lemme check the eDP docs on this one...  it's supposed to be T12, which
>> > is the time between power cycles.  Yeah that matches what we're using
>> > elsewhere, so:
>> >
>> > Reviewed-by: Jesse Barnes 
>>
>> Starting with this patch I get a blank screen when booting my MacBook Air 
>> 6,2.
>> Reverting only this patch on top of 3.14-rc1 doesn't help so it's most likely
>> the entire series that needs to be looked at.
>>
>> Doing a suspend/resume fixes the problem.

Oh, well, at least we didn't break suspend/resume :)

I just requested some information at the bugzilla entry created by
Chris. Can you please provide the requested files there, and move the
discussion to bugzilla?

>
> I've filed this as https://bugs.freedesktop.org/show_bug.cgi?id=74628

Thanks for doing this! It's too easy to forget about mailing-list-only
bug reports.

> Thanks,
> -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



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


Re: [Intel-gfx] [PATCH] tests/gem_gtt_hog: Fix for BDW

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 04:19:08PM -0200, Rodrigo Vivi wrote:
> Update XY_COLOR_BLT command for Broadwell.
> 
> v2: stash devid and remove ugly double allocation. (by Chris).
> 
> Cc: Chris Wilson ch...@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi 
> ---
>  tests/gem_gtt_hog.c | 24 +---
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
> index 53cd7eb..4198a94 100644
> --- a/tests/gem_gtt_hog.c
> +++ b/tests/gem_gtt_hog.c
> @@ -57,17 +57,25 @@ static void busy(int fd, uint32_t handle, int size, int 
> loops)
>   struct drm_i915_gem_execbuffer2 execbuf;
>   struct drm_i915_gem_pwrite gem_pwrite;
>   struct drm_i915_gem_create create;
> - uint32_t buf[122], *b;
> - int i;
> + uint32_t buf[170], *b;
> + int i, b_size;
> + int devid = intel_get_drm_devid(fd);

This function is called many times. But the value is static.
-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 3/3] drm/i915: BDW PSR: Remove DDIA limitation for Broadwell.

2014-02-06 Thread Rodrigo Vivi
Broadwell has a PSR per transcoder, where DDIA supports
link disable and link standby modes while other
transcoders only support link standby.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_dp.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 13019b4..86360a5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1650,16 +1650,21 @@ static void intel_edp_psr_setup(struct intel_dp 
*intel_dp)
 
 static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 {
-   struct drm_device *dev = intel_dp_to_dev(intel_dp);
+   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t aux_clock_divider;
int precharge = 0x3;
int msg_size = 5;   /* Header(4) + Message(1) */
+   bool only_standby = false;
 
aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
 
+   if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
+   only_standby = true;
+
/* Enable PSR in sink */
-   if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
+   if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
DP_PSR_ENABLE &
~DP_PSR_MAIN_LINK_ACTIVE);
@@ -1680,14 +1685,19 @@ static void intel_edp_psr_enable_sink(struct intel_dp 
*intel_dp)
 
 static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
 {
-   struct drm_device *dev = intel_dp_to_dev(intel_dp);
+   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t max_sleep_time = 0x1f;
uint32_t idle_frames = 1;
uint32_t val = 0x0;
const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
+   bool only_standby = false;
+
+   if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
+   only_standby = true;
 
-   if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
+   if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby) {
val |= EDP_PSR_LINK_STANDBY;
val |= EDP_PSR_TP2_TP3_TIME_0us;
val |= EDP_PSR_TP1_TIME_0us;
@@ -1720,8 +1730,8 @@ static bool intel_edp_psr_match_conditions(struct 
intel_dp *intel_dp)
return false;
}
 
-   if ((intel_encoder->type != INTEL_OUTPUT_EDP) ||
-   (dig_port->port != PORT_A)) {
+   if (IS_HASWELL(dev) && (intel_encoder->type != INTEL_OUTPUT_EDP ||
+   dig_port->port != PORT_A)) {
DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
return false;
}
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 1/3] drm/i915: BDW PSR: Add single frame update support.

2014-02-06 Thread Rodrigo Vivi
When link is in stand by and PSR exit is triggered by a primary or sprite
plane flip this mode allows only one single updated frame to be send to
display than get back to PSR immediately.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_dp.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc3ea04..3d6a88b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1987,6 +1987,7 @@
 #define EDP_PSR_BASE(dev)   (IS_HASWELL(dev) ? 0x64800 : 
0x6f800)
 #define EDP_PSR_CTL(dev)   (EDP_PSR_BASE(dev) + 0)
 #define   EDP_PSR_ENABLE   (1<<31)
+#define   BDW_PSR_SINGLE_FRAME (1<<30)
 #define   EDP_PSR_LINK_DISABLE (0<<27)
 #define   EDP_PSR_LINK_STANDBY (1<<27)
 #define   EDP_PSR_MIN_LINK_ENTRY_TIME_MASK (3<<25)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1ef3d4..39fffb0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1692,6 +1692,7 @@ static void intel_edp_psr_enable_source(struct intel_dp 
*intel_dp)
val |= EDP_PSR_TP2_TP3_TIME_0us;
val |= EDP_PSR_TP1_TIME_0us;
val |= EDP_PSR_SKIP_AUX_EXIT;
+   val |= IS_BROADWELL(dev) ? BDW_PSR_SINGLE_FRAME : 0;
} else
val |= EDP_PSR_LINK_DISABLE;
 
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 2/3] drm/i915: BDW PSR: Remove limitations that aren't valid for BDW.

2014-02-06 Thread Rodrigo Vivi
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_dp.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 39fffb0..13019b4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1750,6 +1750,10 @@ static bool intel_edp_psr_match_conditions(struct 
intel_dp *intel_dp)
return false;
}
 
+   /* Below limitations aren't valid for Broadwell */
+   if (IS_BROADWELL(dev))
+   goto out;
+
if (I915_READ(SPRCTL(intel_crtc->pipe)) & SPRITE_ENABLE) {
DRM_DEBUG_KMS("PSR condition failed: Sprite is Enabled\n");
return false;
@@ -1766,6 +1770,7 @@ static bool intel_edp_psr_match_conditions(struct 
intel_dp *intel_dp)
return false;
}
 
+ out:
dev_priv->psr.source_ok = true;
return true;
 }
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 0/3] BDW PSR New features

2014-02-06 Thread Rodrigo Vivi
These 3 patches remove bdw psr limitations and add single frame update
support.

It is important to highlight that it was never tested since I don't have a
BDW here with me. According to PM docs this should work as it is.

Rodrigo Vivi (3):
  drm/i915: BDW PSR: Add single frame update support.
  drm/i915: BDW PSR: Remove limitations that aren't valid for BDW.
  drm/i915: BDW PSR: Remove DDIA limitation for Broadwell.

 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_dp.c | 28 ++--
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
1.8.3.1

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


[Intel-gfx] [PATCH] tests/gem_gtt_hog: Fix for BDW

2014-02-06 Thread Rodrigo Vivi
Update XY_COLOR_BLT command for Broadwell.

v2: stash devid and remove ugly double allocation. (by Chris).

Cc: Chris Wilson ch...@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi 
---
 tests/gem_gtt_hog.c | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 53cd7eb..4198a94 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -57,17 +57,25 @@ static void busy(int fd, uint32_t handle, int size, int 
loops)
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_pwrite gem_pwrite;
struct drm_i915_gem_create create;
-   uint32_t buf[122], *b;
-   int i;
+   uint32_t buf[170], *b;
+   int i, b_size;
+   int devid = intel_get_drm_devid(fd);
 
memset(reloc, 0, sizeof(reloc));
memset(gem_exec, 0, sizeof(gem_exec));
memset(&execbuf, 0, sizeof(execbuf));
 
+   b_size = devid >= 8 ? 170 : 122;
b = buf;
for (i = 0; i < 20; i++) {
-   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
-   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   if (devid >= 8) {
+   *b++ = MI_NOOP;
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   } else {
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 5 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   }
*b++ = 0xf0 << 16 | 1 << 25 | 1 << 24 | 4096;
*b++ = 0;
*b++ = size >> 12 << 16 | 1024;
@@ -76,6 +84,8 @@ static void busy(int fd, uint32_t handle, int size, int loops)
reloc[i].read_domains = I915_GEM_DOMAIN_RENDER;
reloc[i].write_domain = I915_GEM_DOMAIN_RENDER;
*b++ = 0;
+   if (devid >= 8)
+   *b++ = 0;
*b++ = canary;
}
*b++ = MI_BATCH_BUFFER_END;
@@ -93,14 +103,14 @@ static void busy(int fd, uint32_t handle, int size, int 
loops)
 
execbuf.buffers_ptr = (uintptr_t)gem_exec;
execbuf.buffer_count = 2;
-   execbuf.batch_len = sizeof(buf);
+   execbuf.batch_len = b_size;
execbuf.flags = 1 << 11;
-   if (HAS_BLT_RING(intel_get_drm_devid(fd)))
+   if (HAS_BLT_RING(devid))
execbuf.flags |= I915_EXEC_BLT;
 
gem_pwrite.handle = gem_exec[1].handle;
gem_pwrite.offset = 0;
-   gem_pwrite.size = sizeof(buf);
+   gem_pwrite.size = b_size;
gem_pwrite.data_ptr = (uintptr_t)buf;
if (drmIoctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &gem_pwrite) == 0) {
while (loops--)
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 11:56:37AM +, Chris Wilson wrote:
> On Thu, Feb 06, 2014 at 10:22:28AM +, Goel, Akash wrote:
> > Please kindly review this patch.
> > 
> > Best regards
> > Akash
> > -Original Message-
> > From: Goel, Akash 
> > Sent: Thursday, January 09, 2014 5:55 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Goel, Akash
> > Subject: [PATCH] drm/i915/vlv: Added write-enable pte bit support
> > 
> > From: Akash Goel 
> > 
> > This adds support for using the write-enable bit in the GTT entry for VLV.
> > This is handled via a read-only flag in the GEM buffer object which is then 
> > used to check if the write-enable bit has to be set or not when writing the 
> > GTT entries.
> > Currently by default only the Batch buffer & Ring buffers are being marked 
> > as read only.
> 
> Don't cause us to rewrite the PTE for the batch buffer between each
> execbuffer (ro for the batch, rw next time it gets used as a texture).
> In fact, do not change ro without user intervention.
> 
> gt_old_ro is unused
> 
> Use byt_pte_encode() instead of hacking the result of
> ppgtt->pte_encode().
> 
> Consider expanding i915_cache_level so that it included the concept of
> PROT_READ | PROT_WRITE.

Also, what's the exact use-case for this here? And if we need to expose
this to userspace, then it needs a testcase.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t] lib: fix signed/unsigned comparison issues

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 04:31:54PM +, Thomas Wood wrote:
> Store the result of set_vt_mode as a signed value so that errors can be
> caught correctly.
> 
> Signed-off-by: Thomas Wood 

Both merged, thanks.
-Daniel

> ---
>  lib/igt_kms.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 3960d24..5f341ff 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -620,15 +620,16 @@ static void restore_vt_mode_at_exit(int sig)
>  
>  void igt_set_vt_graphics_mode(void)
>  {
> + long ret;
> +
>   igt_install_exit_handler(restore_vt_mode_at_exit);
>  
>   igt_disable_exit_handler();
> - orig_vt_mode = set_vt_mode(KD_GRAPHICS);
> - if (orig_vt_mode < 0)
> - orig_vt_mode = -1UL;
> + ret = set_vt_mode(KD_GRAPHICS);
>   igt_enable_exit_handler();
>  
> - igt_assert(orig_vt_mode >= 0);
> + igt_assert(ret >= 0);
> + orig_vt_mode = ret;
>  }
>  
>  int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector 
> *connector,
> -- 
> 1.8.5.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 4/4] tests/gem_gtt_hog: Fix for BDW

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 02:54:27PM -0200, Rodrigo Vivi wrote:
> Update XY_COLOR_BLT command for Broadwell.
> 
> Signed-off-by: Rodrigo Vivi 
> ---
>  tests/gem_gtt_hog.c | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
> index 53cd7eb..d97658e 100644
> --- a/tests/gem_gtt_hog.c
> +++ b/tests/gem_gtt_hog.c
> @@ -57,17 +57,26 @@ static void busy(int fd, uint32_t handle, int size, int 
> loops)
>   struct drm_i915_gem_execbuffer2 execbuf;
>   struct drm_i915_gem_pwrite gem_pwrite;
>   struct drm_i915_gem_create create;
> - uint32_t buf[122], *b;
> + uint32_t buf[122], buf_bdw[170], *b;

Just allocate the one buffer large enough to take both and tweak the
length use to pwrite/exec.
-Chris

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


Re: [Intel-gfx] [PATCH 3/5] drm/i915: fix VDD override off wait

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 06:16:02PM +0100, Patrik Jakobsson wrote:
> On Fri, Dec 6, 2013 at 8:47 PM, Jesse Barnes  wrote:
> > On Fri,  6 Dec 2013 17:32:42 -0200
> > Paulo Zanoni  wrote:
> >
> >> From: Paulo Zanoni 
> >>
> >> If we're disabling the VDD override bit and the panel is enabled, we
> >> don't need to wait for anything. If the panel is disabled, then we
> >> need to actually wait for panel_power_cycle_delay, not
> >> panel_power_down_delay, because the power down delay was already
> >> respected when we disabled the panel.
> >>
> >> Signed-off-by: Paulo Zanoni 
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index fe327ce..a2aace2 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct 
> >> intel_dp *intel_dp)
> >>   /* Make sure sequencer is idle before allowing subsequent 
> >> activity */
> >>   DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
> >>   I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
> >> - msleep(intel_dp->panel_power_down_delay);
> >> +
> >> + if ((pp & POWER_TARGET_ON) == 0)
> >> + msleep(intel_dp->panel_power_cycle_delay);
> >>   }
> >>  }
> >>
> >
> > Lemme check the eDP docs on this one...  it's supposed to be T12, which
> > is the time between power cycles.  Yeah that matches what we're using
> > elsewhere, so:
> >
> > Reviewed-by: Jesse Barnes 
> 
> Starting with this patch I get a blank screen when booting my MacBook Air 6,2.
> Reverting only this patch on top of 3.14-rc1 doesn't help so it's most likely
> the entire series that needs to be looked at.
> 
> Doing a suspend/resume fixes the problem.

I've filed this as https://bugs.freedesktop.org/show_bug.cgi?id=74628
Thanks,
-Chris

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


Re: [Intel-gfx] [PATCH 4/4] tests/gem_gtt_hog: Fix for BDW

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 02:54:27PM -0200, Rodrigo Vivi wrote:
> Update XY_COLOR_BLT command for Broadwell.

Please stash the result of intel_get_drm_device(fd) >= 8; it makes a
big mess if you are trying to debug using drm.debug=1
-Chris

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


Re: [Intel-gfx] [PATCH v3 00/12] Enabling 180 degree rotation for sprite and crtc planes

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 06:29:15PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 06, 2014 at 08:34:41PM +0530, sagar.a.kam...@intel.com wrote:
> > From: Sagar Kamble 
> > 
> > These patches will enable 180 degree rotation for CRTC and Sprite planes. 
> > This
> > version has following changes:
> > 1. Addressed review comments for CRTC rotation from FBC, page flip, CRTC 
> > active/
> > inactive perspective.
> > 2. Removed drm_rect_rotate amd drm_rect_rotate_inv functions as they dont 
> > take
> > care of clipped planes and 180 rotation in that case produces invalid 
> > result.
> > 
> > Sagar Kamble (3):
> >   drm/i915: Add 180 degree primary plane rotation support
> >   drm: Set property to return invalid for unsupported arguments for
> > bitmask property
> >   drm/i915: Removing rotate and inverse rotate calls from update_plane
> > 
> > Ville Syrjälä (9):
> >   drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
> >   drm: Add support_bits parameter to drm_property_create_bitmask()
> >   drm: Add drm_mode_create_rotation_property()
> >   drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
> >   drm: Add drm_rect rotation functions
> >   drm: Add drm_rotation_simplify()
> >   drm/i915: Add 180 degree sprite rotation support
> >   drm/i915: Make intel_plane_restore() return an error
> >   drm/i915: Add rotation property for sprites
> 
> Oh I forgot to mention it last time around, since this series touches
> more than i915, it should be cc:ed to dri-devel as well.

Ah, mail cross-over. Another process-thing: When resubmitting patches
please put a little changelog into both your cover letter and individual
patches. That way reviewers can quickly gauge what changed and concentrate
on looking at just those parts. That helps with streamline the review
process.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/5] drm/i915: fix VDD override off wait

2014-02-06 Thread Patrik Jakobsson
On Fri, Dec 6, 2013 at 8:47 PM, Jesse Barnes  wrote:
> On Fri,  6 Dec 2013 17:32:42 -0200
> Paulo Zanoni  wrote:
>
>> From: Paulo Zanoni 
>>
>> If we're disabling the VDD override bit and the panel is enabled, we
>> don't need to wait for anything. If the panel is disabled, then we
>> need to actually wait for panel_power_cycle_delay, not
>> panel_power_down_delay, because the power down delay was already
>> respected when we disabled the panel.
>>
>> Signed-off-by: Paulo Zanoni 
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index fe327ce..a2aace2 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct 
>> intel_dp *intel_dp)
>>   /* Make sure sequencer is idle before allowing subsequent 
>> activity */
>>   DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
>>   I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>> - msleep(intel_dp->panel_power_down_delay);
>> +
>> + if ((pp & POWER_TARGET_ON) == 0)
>> + msleep(intel_dp->panel_power_cycle_delay);
>>   }
>>  }
>>
>
> Lemme check the eDP docs on this one...  it's supposed to be T12, which
> is the time between power cycles.  Yeah that matches what we're using
> elsewhere, so:
>
> Reviewed-by: Jesse Barnes 

Starting with this patch I get a blank screen when booting my MacBook Air 6,2.
Reverting only this patch on top of 3.14-rc1 doesn't help so it's most likely
the entire series that needs to be looked at.

Doing a suspend/resume fixes the problem.

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


[Intel-gfx] [PATCH 1/4] tests/gem_wait_render_timeout: Use XY_COLOR_BLT instead of COLOR_BLT.

2014-02-06 Thread Rodrigo Vivi
Signed-off-by: Rodrigo Vivi 
---
 tests/gem_wait_render_timeout.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/gem_wait_render_timeout.c b/tests/gem_wait_render_timeout.c
index 2971752..4ecb93f 100644
--- a/tests/gem_wait_render_timeout.c
+++ b/tests/gem_wait_render_timeout.c
@@ -86,13 +86,13 @@ static void blt_color_fill(struct intel_batchbuffer *batch,
 {
const unsigned short height = pages/4;
const unsigned short width =  4096;
-   BEGIN_BATCH(5);
-   OUT_BATCH(COLOR_BLT_CMD |
- COLOR_BLT_WRITE_ALPHA |
- COLOR_BLT_WRITE_RGB);
+   BEGIN_BATCH(6);
+   OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | 4 |
+ COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB);
OUT_BATCH((3 << 24) | /* 32 Bit Color */
- 0xF0  | /* Raster OP copy background register */
+ (0xF0 << 16)  | /* Raster OP copy background register */
  0); /* Dest pitch is 0 */
+   OUT_BATCH(0);
OUT_BATCH(width << 16   |
  height);
OUT_RELOC(buf, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH 1/1] tests/kms_rotate_crc: Test to verify rotation of planes through CRC

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 08:34:31PM +0530, sagar.a.kam...@intel.com wrote:
> From: Sagar Kamble 
> 
> This test will verify 180 degree rotation of CRTC and sprite planes
> by grabbing CRC for already rotated image and comparing with CRC calculated
> after triggering rotation through DRM property.
> 
> Signed-off-by: Sagar Kamble 
> Tested-by: Sagar Kamble 

Minor style nit: The tested-by tag is to credit testing done by
non-authors. This is especially important if testing requires a special
machine/setup (e.g. the one from a bug reporter) - adding the mail helps
in contacting the right people again if a commit breaks something.

Adding your own mail is redundant since it's assumed that developers test
their own stuff ;-)

Two more general comments below.


[snip]

> +igt_main
> +{
> + data_t data = {};

igt_fixture/subtest use longjmps, so putting stack variables into the same
function as those is ill-defined. bkm is to add them as global variables
right above igt_main.

> + fp = fopen("rotate.log", "w+"); 

Please log data to stdout using printf and friends. That way test runners
can easily redirect test output or capture it themselve for storage into a
db. Maybe we need an official igt_log, Jeff McGee somewhat proposed this
already. Also anything not related to enumerating the subtests must be put
into igt_fictures.

> +
> + igt_skip_on_simulation();
> +
> + igt_fixture {
> + data.drm_fd = drm_open_any();
> + igt_set_vt_graphics_mode();
> +
> + igt_debugfs_init(&data.debugfs);
> + igt_pipe_crc_check(&data.debugfs);
> +
> + display_init(&data);
> + }
> +
> + run_test(&data);
> +
> + igt_fixture {
> + display_fini(&data);
> + }
> +
> + fclose(fp);
> +}
> -- 
> 1.8.5
> 

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


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


[Intel-gfx] [PATCH 1/4] tests/gem_wait_render_timeout: Use XY_COLOR_BLT instead of COLOR_BLT.

2014-02-06 Thread Rodrigo Vivi
Signed-off-by: Rodrigo Vivi 
---
 tests/gem_wait_render_timeout.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/gem_wait_render_timeout.c b/tests/gem_wait_render_timeout.c
index 2971752..4ecb93f 100644
--- a/tests/gem_wait_render_timeout.c
+++ b/tests/gem_wait_render_timeout.c
@@ -86,13 +86,13 @@ static void blt_color_fill(struct intel_batchbuffer *batch,
 {
const unsigned short height = pages/4;
const unsigned short width =  4096;
-   BEGIN_BATCH(5);
-   OUT_BATCH(COLOR_BLT_CMD |
- COLOR_BLT_WRITE_ALPHA |
- COLOR_BLT_WRITE_RGB);
+   BEGIN_BATCH(6);
+   OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | 4 |
+ COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB);
OUT_BATCH((3 << 24) | /* 32 Bit Color */
- 0xF0  | /* Raster OP copy background register */
+ (0xF0 << 16)  | /* Raster OP copy background register */
  0); /* Dest pitch is 0 */
+   OUT_BATCH(0);
OUT_BATCH(width << 16   |
  height);
OUT_RELOC(buf, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 2/4] tests/gem_wait_render_timeout: Fix for BDW

2014-02-06 Thread Rodrigo Vivi
Update XY_COLOR_BLT command for Broadwell.

Signed-off-by: Rodrigo Vivi 
---
 tests/gem_wait_render_timeout.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tests/gem_wait_render_timeout.c b/tests/gem_wait_render_timeout.c
index 4ecb93f..9fe6910 100644
--- a/tests/gem_wait_render_timeout.c
+++ b/tests/gem_wait_render_timeout.c
@@ -86,9 +86,17 @@ static void blt_color_fill(struct intel_batchbuffer *batch,
 {
const unsigned short height = pages/4;
const unsigned short width =  4096;
-   BEGIN_BATCH(6);
-   OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | 4 |
- COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB);
+
+   if (intel_gen(batch->devid) >= 8) {
+   BEGIN_BATCH(8);
+   OUT_BATCH(MI_NOOP);
+   OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | 5 |
+ COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB);
+   } else {
+   BEGIN_BATCH(6);
+   OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | 4 |
+ COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB);
+   }
OUT_BATCH((3 << 24) | /* 32 Bit Color */
  (0xF0 << 16)  | /* Raster OP copy background register */
  0); /* Dest pitch is 0 */
@@ -96,6 +104,8 @@ static void blt_color_fill(struct intel_batchbuffer *batch,
OUT_BATCH(width << 16   |
  height);
OUT_RELOC(buf, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
+   if (intel_gen(batch->devid) >= 8)
+   OUT_BATCH(0);
OUT_BATCH(rand()); /* random pattern */
ADVANCE_BATCH();
 }
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 3/4] tests/gem_gtt_hog: Use XY_COLOR_BLT instead of COLOR_BLT.

2014-02-06 Thread Rodrigo Vivi
Signed-off-by: Rodrigo Vivi 
---
 tests/gem_gtt_hog.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 53d7dd7..53cd7eb 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -42,9 +42,6 @@
 #include "i915_drm.h"
 #include "drmtest.h"
 
-#define BLT_WRITE_ALPHA(1<<21)
-#define BLT_WRITE_RGB  (1<<20)
-
 static const uint32_t canary = 0xdeadbeef;
 
 static double elapsed(const struct timeval *start,
@@ -60,7 +57,7 @@ static void busy(int fd, uint32_t handle, int size, int loops)
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_pwrite gem_pwrite;
struct drm_i915_gem_create create;
-   uint32_t buf[102], *b;
+   uint32_t buf[122], *b;
int i;
 
memset(reloc, 0, sizeof(reloc));
@@ -69,9 +66,11 @@ static void busy(int fd, uint32_t handle, int size, int 
loops)
 
b = buf;
for (i = 0; i < 20; i++) {
-   *b++ = COLOR_BLT_CMD | BLT_WRITE_ALPHA | BLT_WRITE_RGB;
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
*b++ = 0xf0 << 16 | 1 << 25 | 1 << 24 | 4096;
-   *b++ = size >> 12 << 16 | 4096;
+   *b++ = 0;
+   *b++ = size >> 12 << 16 | 1024;
reloc[i].offset = (b - buf) * sizeof(uint32_t);
reloc[i].target_handle = handle;
reloc[i].read_domains = I915_GEM_DOMAIN_RENDER;
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH v3 01/12] drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 08:34:42PM +0530, sagar.a.kam...@intel.com wrote:
> From: Ville Syrjälä 
> 
> The rotation property stuff should be standardized among all drivers.
> Move the bits to drm_crtc.h from omap_drv.h.
> 
> Signed-off-by: Ville Syrjälä 
> Tested-by: Sagar Kamble 

Patches which touch code outside of i915 must be submitted to the
dri-devel mailing list. Also, since you're touching a driver please also
cc the driver maintainers/authors (Tomi/Rob in this case) so that they'll
notice the patch.

scripts/get_maintainers.pl can help you with adding a reasonable Cc: list
to the sob section of your patches.
-Daniel

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h | 7 ---
>  include/drm/drm_crtc.h | 8 
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h 
> b/drivers/gpu/drm/omapdrm/omap_drv.h
> index 428b2981..aac8e10 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -119,13 +119,6 @@ struct omap_drm_private {
>   struct omap_drm_irq error_handler;
>  };
>  
> -/* this should probably be in drm-core to standardize amongst drivers */
> -#define DRM_ROTATE_0 0
> -#define DRM_ROTATE_901
> -#define DRM_ROTATE_180   2
> -#define DRM_ROTATE_270   3
> -#define DRM_REFLECT_X4
> -#define DRM_REFLECT_Y5
>  
>  #ifdef CONFIG_DEBUG_FS
>  int omap_debugfs_init(struct drm_minor *minor);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 71727b6..d5c46c1 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -65,6 +65,14 @@ struct drm_object_properties {
>   uint64_t values[DRM_OBJECT_MAX_PROPERTY];
>  };
>  
> +/* rotation property bits */
> +#define DRM_ROTATE_0 0
> +#define DRM_ROTATE_901
> +#define DRM_ROTATE_180   2
> +#define DRM_ROTATE_270   3
> +#define DRM_REFLECT_X4
> +#define DRM_REFLECT_Y5
> +
>  /*
>   * Note on terminology:  here, for brevity and convenience, we refer to 
> connector
>   * control chips as 'CRTCs'.  They can control any type of connector, VGA, 
> LVDS,
> -- 
> 1.8.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


[Intel-gfx] [PATCH 4/4] tests/gem_gtt_hog: Fix for BDW

2014-02-06 Thread Rodrigo Vivi
Update XY_COLOR_BLT command for Broadwell.

Signed-off-by: Rodrigo Vivi 
---
 tests/gem_gtt_hog.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 53cd7eb..d97658e 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -57,17 +57,26 @@ static void busy(int fd, uint32_t handle, int size, int 
loops)
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_pwrite gem_pwrite;
struct drm_i915_gem_create create;
-   uint32_t buf[122], *b;
+   uint32_t buf[122], buf_bdw[170], *b;
int i;
 
memset(reloc, 0, sizeof(reloc));
memset(gem_exec, 0, sizeof(gem_exec));
memset(&execbuf, 0, sizeof(execbuf));
 
-   b = buf;
+   if (intel_get_drm_devid(fd) >= 8)
+   b = buf_bdw;
+   else
+   b = buf;
for (i = 0; i < 20; i++) {
-   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
-   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   if (intel_get_drm_devid(fd) >= 8) {
+   *b++ = MI_NOOP;
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   } else {
+   *b++ = XY_COLOR_BLT_CMD_NOLEN | 5 |
+   COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
+   }
*b++ = 0xf0 << 16 | 1 << 25 | 1 << 24 | 4096;
*b++ = 0;
*b++ = size >> 12 << 16 | 1024;
@@ -76,6 +85,8 @@ static void busy(int fd, uint32_t handle, int size, int loops)
reloc[i].read_domains = I915_GEM_DOMAIN_RENDER;
reloc[i].write_domain = I915_GEM_DOMAIN_RENDER;
*b++ = 0;
+   if (intel_get_drm_devid(fd) >= 8)
+   *b++ = 0;
*b++ = canary;
}
*b++ = MI_BATCH_BUFFER_END;
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH] drm/i915: Prevent recursion by retiring requests when the ring is full

2014-02-06 Thread Daniel Vetter
On Tue, Jan 28, 2014 at 11:25:41AM +, Chris Wilson wrote:
> On Tue, Jan 28, 2014 at 01:15:35PM +0200, Ville Syrjälä wrote:
> > On Mon, Jan 27, 2014 at 10:43:07PM +, Chris Wilson wrote:
> > > As the VM do not track activity of objects and instead use a large
> > > hammer to forcibly idle and evict all of their associated objects when
> > > one is released, it is possible for that to cause a recursion when we
> > > need to wait for free space on a ring and call retire requests.
> > > (intel_ring_begin -> intel_ring_wait_request ->
> > > i915_gem_retire_requests_ring -> i915_gem_context_free ->
> > > i915_gem_evict_vm -> i915_gpu_idle -> intel_ring_begin etc)
> > > 
> > > In order to remove the requirement for calling retire-requests from
> > > intel_ring_wait_request, we have to inline a couple of steps from
> > > retiring requests, notably we have to record the position of the request
> > > we wait for and use that to update the available ring space.
> > > 
> > > Signed-off-by: Chris Wilson 
> > 
> > Looks good to me.
> > Reviewed-by: Ville Syrjälä 
> > 
> > I do have a couple of questions about request->tail though.
> > 
> > We set it to -1 in intel_ring_wait_request(). Isn't that going to cause
> > problems for i915_request_guilty()?
> > 
> > When not -1, request->tail points to just before the commands that
> > .add_request() adds to the ring. So that means intel_ring_wait_request()
> > might have to wait for one extra request, and I guess more importantly
> > if the GPU hangs inside the .add_request() commands, we won't attribute
> > the hang to the request in question. Was it designe to be that way, or
> > is there a bug here?
> 
> Ah good questions. I completely forgot about the behaviour here when we
> adjusted this for hangstats...
> 
> Setting it to -1 should not confuse the guilty search since that should
> be done (or at least changed so that is) based on a completion search.
> After making that change, we should be able to set request->tail back to
> being just after the request. Also I think we are quite safe to drop the
> manipulation of request->tail inside intel_ring_wait_request().

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


Re: [Intel-gfx] [PATCH v3 00/12] Enabling 180 degree rotation for sprite and crtc planes

2014-02-06 Thread Ville Syrjälä
On Thu, Feb 06, 2014 at 08:34:41PM +0530, sagar.a.kam...@intel.com wrote:
> From: Sagar Kamble 
> 
> These patches will enable 180 degree rotation for CRTC and Sprite planes. This
> version has following changes:
> 1. Addressed review comments for CRTC rotation from FBC, page flip, CRTC 
> active/
> inactive perspective.
> 2. Removed drm_rect_rotate amd drm_rect_rotate_inv functions as they dont take
> care of clipped planes and 180 rotation in that case produces invalid result.
> 
> Sagar Kamble (3):
>   drm/i915: Add 180 degree primary plane rotation support
>   drm: Set property to return invalid for unsupported arguments for
> bitmask property
>   drm/i915: Removing rotate and inverse rotate calls from update_plane
> 
> Ville Syrjälä (9):
>   drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
>   drm: Add support_bits parameter to drm_property_create_bitmask()
>   drm: Add drm_mode_create_rotation_property()
>   drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
>   drm: Add drm_rect rotation functions
>   drm: Add drm_rotation_simplify()
>   drm/i915: Add 180 degree sprite rotation support
>   drm/i915: Make intel_plane_restore() return an error
>   drm/i915: Add rotation property for sprites

Oh I forgot to mention it last time around, since this series touches
more than i915, it should be cc:ed to dri-devel as well.

> 
>  drivers/gpu/drm/drm_crtc.c   |  72 +-
>  drivers/gpu/drm/drm_rect.c   | 140 
> +++
>  drivers/gpu/drm/i915/i915_drv.h  |   1 +
>  drivers/gpu/drm/i915/i915_reg.h  |   4 +
>  drivers/gpu/drm/i915/intel_display.c |  75 ++-
>  drivers/gpu/drm/i915/intel_drv.h |   5 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |  87 --
>  drivers/gpu/drm/omapdrm/omap_drv.h   |   7 --
>  drivers/gpu/drm/omapdrm/omap_plane.c |  17 ++---
>  include/drm/drm_crtc.h   |  15 +++-
>  include/drm/drm_rect.h   |   6 ++
>  11 files changed, 397 insertions(+), 32 deletions(-)
> 
> -- 
> 1.8.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/5] Add power feature debugfs disabling

2014-02-06 Thread Daniel Vetter
On Thu, Feb 6, 2014 at 4:44 PM, Jeff McGee  wrote:
> Our Android system validation tests are expecting these interfaces. That's
> not igt, I know, but is supporting downstream test suites a priority? I can
> get our val guys on the list to +1 the need for these patches. Likewise I
> can request a developer from my team to review these patches. Or are you
> looking specifically for someone outside our downstream product to 2nd the
> need-for and quality of the patches?

I don't mind if something is only used in one product - if it's useful
sooner or later other product teams will pick up on in. So upstreaming
is the right thing. But if you add a debugfs for tests I also want to
have the tests upstreamed for the following reasons:

- If a product team deems it useful to test something, our upstream QA
should probably do the same.

- Without testcases actually using these on upstream there's a good
chance that we'll break them. Which means forward-rolling to new
forklifts will be more of a pain than strictly needed. We already have
our issues with upstream collaboration, no need to make it more ugly.

- For interfaces used in tests/scripts I also want to do a bit of api
review, which is best done by looking at the actual users. Ofc debugfs
doesn't have strict abi guarantees imposed by the linux community like
ioctls/sysfs which we're essentially never allowed to break. But
change the interfaces still has its costs.

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 i-g-t] lib: fix signed/unsigned comparison issues

2014-02-06 Thread Thomas Wood
Store the result of set_vt_mode as a signed value so that errors can be
caught correctly.

Signed-off-by: Thomas Wood 
---
 lib/igt_kms.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 3960d24..5f341ff 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -620,15 +620,16 @@ static void restore_vt_mode_at_exit(int sig)
 
 void igt_set_vt_graphics_mode(void)
 {
+   long ret;
+
igt_install_exit_handler(restore_vt_mode_at_exit);
 
igt_disable_exit_handler();
-   orig_vt_mode = set_vt_mode(KD_GRAPHICS);
-   if (orig_vt_mode < 0)
-   orig_vt_mode = -1UL;
+   ret = set_vt_mode(KD_GRAPHICS);
igt_enable_exit_handler();
 
-   igt_assert(orig_vt_mode >= 0);
+   igt_assert(ret >= 0);
+   orig_vt_mode = ret;
 }
 
 int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
-- 
1.8.5.3

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


[Intel-gfx] [PATCH i-g-t] assembler: define YY_NO_INPUT to prevent unused symbol warnings

2014-02-06 Thread Thomas Wood
Signed-off-by: Thomas Wood 
---
 assembler/lex.l | 1 +
 1 file changed, 1 insertion(+)

diff --git a/assembler/lex.l b/assembler/lex.l
index 4f1f961..81a52ba 100644
--- a/assembler/lex.l
+++ b/assembler/lex.l
@@ -12,6 +12,7 @@ extern char *input_filename;
 /* Locations */
 int yycolumn = 1;
 
+#define YY_NO_INPUT
 #define YY_USER_ACTION \
yylloc.first_line = yylloc.last_line = yylineno;\
yylloc.first_column = yycolumn; \
-- 
1.8.5.3

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


Re: [Intel-gfx] [PATCH v3 11/12] drm: Set property to return invalid for unsupported arguments for bitmask property

2014-02-06 Thread Ville Syrjälä
On Thu, Feb 06, 2014 at 08:34:52PM +0530, sagar.a.kam...@intel.com wrote:
> From: Sagar Kamble 
> 
> DRM will not propagate the set_property call for bitmask drm
> properties if they are not supported by underlying driver.
> 
> Signed-off-by: Sagar Kamble 
> Tested-by: Sagar Kamble 
> ---
>  drivers/gpu/drm/drm_crtc.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 4f5e408..4c92741 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3420,6 +3420,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
> *dev, void *data,
>   struct drm_mode_object *arg_obj;
>   struct drm_mode_object *prop_obj;
>   struct drm_property *property;
> + struct drm_property_enum *prop_enum;
> + bool supported_val = false;
>   int ret = -EINVAL;
>   int i;
>  
> @@ -3451,6 +3453,22 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
> *dev, void *data,
>   }
>   property = obj_to_property(prop_obj);
>  
> + if (property->flags | DRM_MODE_PROP_BITMASK) {
> + if (!list_empty(&property->enum_blob_list)) {
> + list_for_each_entry(prop_enum,
> + &property->enum_blob_list, head) {
> + if (BIT(prop_enum->value) == arg->value) {
> + supported_val = true;
> + break;
> + }
> + }
> + }
> + if (!supported_val) {
> + ret = -EINVAL;
> + goto out;
> + }
> + }
> +
>   if (!drm_property_change_is_valid(property, arg->value))

drm_property_change_is_valid() should take care of this, except I think
my original patch to pass the supported_bits information is a bit buggy.
It will allocate the values[] array based on all possible rotation bits,
but then it will populate it sparsely based on the supported_bits.

Apparently that needs to be changed to allocate only the required amount
of space in values[]. I think that should make
drm_property_change_is_valid() correctly detect the valid bits.

Actually I think even my buggy patch should mostly work, it'll just
always accept DRM_ROTATE_0 even if that wasnt't one of the
supported_bits. That's because since values[] is zeroed initially.

Oh, I see your patch does actually a bit more than that. It would reject
any bitmask property change with multiple bits set. That's not what we
want, as the driver is the only one that can decide which combination(s)
of bits are in fact valid.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 10/12] drm/i915: Add 180 degree primary plane rotation support

2014-02-06 Thread Ville Syrjälä
On Thu, Feb 06, 2014 at 08:34:51PM +0530, sagar.a.kam...@intel.com wrote:
> From: Sagar Kamble 
> 
> Primary planes support 180 degree rotation. Expose the feature
> through rotation drm property.
> 
> v2: Calculating linear/tiled offsets based on pipe source width and
> height. Added 180 degree rotation support in ironlake_update_plane.
> 
> v3: Checking if CRTC is active before issueing update_plane. Added
> wait for vblank to make sure we dont overtake page flips. Disabling
> FBC since it does not work with rotated planes.
> 
> Signed-off-by: Sagar Kamble 
> Tested-by: Sagar Kamble 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 75 
> ++--
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  3 files changed, 74 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57906c5..d3000c4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3553,6 +3553,7 @@
>  #define   DISPPLANE_NO_LINE_DOUBLE   0
>  #define   DISPPLANE_STEREO_POLARITY_FIRST0
>  #define   DISPPLANE_STEREO_POLARITY_SECOND   (1<<18)
> +#define   DISPPLANE_ROTATE_180   (1<<15)
>  #define   DISPPLANE_TRICKLE_FEED_DISABLE (1<<14) /* Ironlake */
>  #define   DISPPLANE_TILED(1<<10)
>  #define _DSPAADDR(dev_priv->info->display_mmio_offset + 0x70184)
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 4d4a0d9..961308d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
> struct drm_framebuffer *fb,
>   unsigned long linear_offset;
>   u32 dspcntr;
>   u32 reg;
> + int pixel_size;
>  
>   switch (plane) {
>   case 0:
> @@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
> struct drm_framebuffer *fb,
>   return -EINVAL;
>   }
>  
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>   intel_fb = to_intel_framebuffer(fb);
>   obj = intel_fb->obj;
>  
> @@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
> struct drm_framebuffer *fb,
>   dspcntr = I915_READ(reg);
>   /* Mask out pixel format bits in case we change it */
>   dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
> + dspcntr &= ~DISPPLANE_ROTATE_180;
> +
>   switch (fb->pixel_format) {
>   case DRM_FORMAT_C8:
>   dspcntr |= DISPPLANE_8BPP;
> @@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
> struct drm_framebuffer *fb,
>   if (IS_G4X(dev))
>   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> - I915_WRITE(reg, dspcntr);
> -
>   linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>  
>   if (INTEL_INFO(dev)->gen >= 4) {
> @@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
> struct drm_framebuffer *fb,
>   intel_crtc->dspaddr_offset = linear_offset;
>   }
>  
> + if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
> + dspcntr |= DISPPLANE_ROTATE_180;
> +
> + x += (intel_crtc->config.pipe_src_w - 1);
> + y += (intel_crtc->config.pipe_src_h - 1);
> + linear_offset += (intel_crtc->config.pipe_src_h - 1) * 
> fb->pitches[0] +
> +   intel_crtc->config.pipe_src_w * 
> pixel_size;
> + }
> +
> + I915_WRITE(reg, dspcntr);
> +
>   DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
> i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
> fb->pitches[0]);
> @@ -2137,6 +2150,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>   unsigned long linear_offset;
>   u32 dspcntr;
>   u32 reg;
> + int pixel_size;
>  
>   switch (plane) {
>   case 0:
> @@ -2148,6 +2162,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>   return -EINVAL;
>   }
>  
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>   intel_fb = to_intel_framebuffer(fb);
>   obj = intel_fb->obj;
>  
> @@ -2155,6 +2170,8 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>   dspcntr = I915_READ(reg);
>   /* Mask out pixel format bits in case we change it */
>   dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
> + dspcntr &= ~DISPPLANE_ROTATE_180;
> +
>   switch (fb->pixel_format) {
>   case DRM_FORMAT_C8:
>   dspcntr |= DISPPLANE_8BPP;
> @@ -2192,8 +2209,6 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>   else
>   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> - I915_WRITE(reg, dspcntr);
> -
>   linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>   intel_crtc->dspaddr_offset =
>   int

Re: [Intel-gfx] [PATCH v3 12/12] drm/i915: Removing rotate and inverse rotate calls from update_plane

2014-02-06 Thread Ville Syrjälä
On Thu, Feb 06, 2014 at 08:34:53PM +0530, sagar.a.kam...@intel.com wrote:
> From: Sagar Kamble 
> 
> With clipped sprites these transformations are not working. these
> functions transform complete sprite irrespective of clipping present.
> This leads to invisible portion of sprite show up when rotate 180 if
> it was out of visible area before.

We need to orient the src and dst coordinates the same way to make
clipping come out right, and then go back to the original orientation to
make our src buffer offset calculation work out. I'm pretty sure this was
working exactly as inteded for me.

If the code is actually buggy, I'm going to want a test case that
demonstrates that bug.

> 
> Signed-off-by: Sagar Kamble 
> Tested-by: Sagar Kamble 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 62b9f84..e7e4c55 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>   max_scale = intel_plane->max_downscale << 16;
>   min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>  
> - drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> - intel_plane->rotation);
> -
>   hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
>   BUG_ON(hscale < 0);
>  
> @@ -810,9 +807,6 @@ intel_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>drm_rect_width(&dst) * hscale - 
> drm_rect_width(&src),
>drm_rect_height(&dst) * vscale - 
> drm_rect_height(&src));
>  
> - drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> - intel_plane->rotation);
> -
>   /* sanity check to make sure the src viewport wasn't enlarged */
>   WARN_ON(src.x1 < (int) src_x ||
>   src.y1 < (int) src_y ||
> -- 
> 1.8.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: don't call swsci_setup on resume

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 04:35:10PM +0100, Daniel Vetter wrote:
> On Thu, Feb 6, 2014 at 4:32 PM, Chris Wilson  wrote:
> > On Thu, Feb 06, 2014 at 01:19:48PM -0200, Paulo Zanoni wrote:
> >> From: Paulo Zanoni 
> >>
> >> This function is used to discover which swsci callbacks the machine
> >> supports. It calls swsci() 3 times, and usually takes a considerable
> >> number of milliseconds to finish.
> >>
> >> I don't see a reason for a change on the supported callbacks between
> >> boot and resume, so use the values retrieved at boot time forever,
> >> making resume a little bit faster.
> >
> > This looks unsafe following a hibernate resume where the system could
> > change around us. Across a suspend resume, you can argue that we do not
> > need to shootdown OpRegion (and not just swsci) at all.
> 
> See my other mail, but I think _init/_fini take care of all this. The
> only exception would be if the bios moves the opregion around, but
> that's only really possible if you update it or change your machine
> otherwise. Then you get both pieces anyway.

Exactly, that's why I think trusting the layout post hibernate is
danagerous.
-Chris

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


Re: [Intel-gfx] [PATCH 0/5] Add power feature debugfs disabling

2014-02-06 Thread Jeff McGee
On Tue, Feb 04, 2014 at 12:30:00PM +0100, Daniel Vetter wrote:
> On Fri, Jan 31, 2014 at 03:42:47PM -0600, jeff.mc...@intel.com wrote:
> > From: Jeff McGee 
> > 
> > This series has recently been accepted into the Haswell Android kernel and
> > helps with debugging and profiling these power features. I would like it
> > to be considered for upstream incorporation. The patches here have been
> > rebased (minimal changes required) and compile-tested only.
> > 
> > Broad device support is provided, accept for RPS and RC6 with Broadwell
> > and Valleyview. Both of these were somewhat of a moving target and I
> > didn't have devices to work with. Support can of course be added with
> > help from appropriate folks.
> > 
> > The hooks introduce some amount of overhead as an additional check is
> > often needed to determine whether the feature is on or off - similar to
> > the module parameters that already exist. I felt that the overhead was
> > minimal enough and didn't want to ugly up the code with CONFIG_DEBUG_FS
> > compile conditionals. But I'm open to the list's thoughts on this.
> > 
> > IGT tests of these new interfaces can certainly be added. I wanted to
> > make sure there was sufficient interest in having these interfaces before
> > starting on the tests. So please provide feedback.
> 
> debugfs doesn't have any abi guarantees and hence requirements are much
> lower. Generally I only want a testcase for new debugfs if it is complex
> infrastructure and other testcases rely on its functionality, like the CRC
> stuff. Simple on/off knobs imo don't need testcases really.
> 
> Still even debugfs code has a bit of cost, so I'll hold off until someone
> says that "yep, this is useful for developing stuff, I'll review it" or
> some i-g-ts show up.

Our Android system validation tests are expecting these interfaces. That's
not igt, I know, but is supporting downstream test suites a priority? I can
get our val guys on the list to +1 the need for these patches. Likewise I
can request a developer from my team to review these patches. Or are you
looking specifically for someone outside our downstream product to 2nd the
need-for and quality of the patches?
-Jeff
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: don't call swsci_setup on resume

2014-02-06 Thread Daniel Vetter
On Thu, Feb 6, 2014 at 4:32 PM, Chris Wilson  wrote:
> On Thu, Feb 06, 2014 at 01:19:48PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni 
>>
>> This function is used to discover which swsci callbacks the machine
>> supports. It calls swsci() 3 times, and usually takes a considerable
>> number of milliseconds to finish.
>>
>> I don't see a reason for a change on the supported callbacks between
>> boot and resume, so use the values retrieved at boot time forever,
>> making resume a little bit faster.
>
> This looks unsafe following a hibernate resume where the system could
> change around us. Across a suspend resume, you can argue that we do not
> need to shootdown OpRegion (and not just swsci) at all.

See my other mail, but I think _init/_fini take care of all this. The
only exception would be if the bios moves the opregion around, but
that's only really possible if you update it or change your machine
otherwise. Then you get both pieces anyway.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: don't call swsci_setup on resume

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 01:19:48PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni 
> 
> This function is used to discover which swsci callbacks the machine
> supports. It calls swsci() 3 times, and usually takes a considerable
> number of milliseconds to finish.
> 
> I don't see a reason for a change on the supported callbacks between
> boot and resume, so use the values retrieved at boot time forever,
> making resume a little bit faster.

This looks unsafe following a hibernate resume where the system could
change around us. Across a suspend resume, you can argue that we do not
need to shootdown OpRegion (and not just swsci) at all.
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915: don't call swsci_setup on resume

2014-02-06 Thread Daniel Vetter
On Thu, Feb 6, 2014 at 4:19 PM, Paulo Zanoni  wrote:
> From: Paulo Zanoni 
>
> This function is used to discover which swsci callbacks the machine
> supports. It calls swsci() 3 times, and usually takes a considerable
> number of milliseconds to finish.
>
> I don't see a reason for a change on the supported callbacks between
> boot and resume, so use the values retrieved at boot time forever,
> making resume a little bit faster.
>
> Signed-off-by: Paulo Zanoni 

I think we could simply not call opregion_setup in our thaw function.
The actual runtime setup is already split out into _init/fini
functions, so if anything in opregion_setup needs to be done at resume
we could simply move that. But afaict the split looks sane already.

That way the init sequence is much less tricky than the conditional
swsci_setup in your patch. The only write we have in _setup is
ASLE_ARDY_NOT_READY, but that's the default state - the driver only
needs to signal readiness once everything is up (in _init) and tell
the bios again that it's gone in _fini.
-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] [PULL] drm-intel-fixes

2014-02-06 Thread Daniel Vetter
Hi Dave,

Just minor stuff really, on vlv dp fix and two patches to tune down some
opregion sanity check. Plus MAINTAINERS update for the new git repo, which
is the only reason I've really bothered with this pull request.

Cheers, Daniel


The following changes since commit ec14ba47791965d2c08e0a681ff44eacbf3c4553:

  drm/i915: Fix the offset issue for the stolen GEM objects (2014-01-28 
09:04:42 +0100)

are available in the git repository at:

  ssh://git.freedesktop.org/git/drm-intel tags/drm-intel-fixes-2014-02-06

for you to fetch changes up to bdde5c6a258a702bdfa7d1f4ae804a7bc405e788:

  drm/i915: demote opregion excessive timeout WARN_ONCE to DRM_INFO_ONCE 
(2014-02-04 21:10:45 +0100)


Daniel Vetter (1):
  MAINTAINERS: Update drm/i915 git repo

Imre Deak (1):
  drm/i915: vlv: fix DP PHY lockup due to invalid PP sequencer setup

Jani Nikula (2):
  drm: add DRM_INFO_ONCE() to print a one-time DRM_INFO() message
  drm/i915: demote opregion excessive timeout WARN_ONCE to DRM_INFO_ONCE

 MAINTAINERS   |  2 +-
 drivers/gpu/drm/i915/intel_dp.c   | 10 ++
 drivers/gpu/drm/i915/intel_opregion.c |  9 ++---
 include/drm/drmP.h|  3 +++
 4 files changed, 16 insertions(+), 8 deletions(-)

-- 
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] drm/i915: don't call swsci_setup on resume

2014-02-06 Thread Paulo Zanoni
From: Paulo Zanoni 

This function is used to discover which swsci callbacks the machine
supports. It calls swsci() 3 times, and usually takes a considerable
number of milliseconds to finish.

I don't see a reason for a change on the supported callbacks between
boot and resume, so use the values retrieved at boot time forever,
making resume a little bit faster.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/i915_dma.c   | 1 +
 drivers/gpu/drm/i915/i915_drv.h   | 2 ++
 drivers/gpu/drm/i915/intel_opregion.c | 8 
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 258b1be..f57e220 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1616,6 +1616,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
intel_setup_mchbar(dev);
intel_setup_gmbus(dev);
intel_opregion_setup(dev);
+   intel_swsci_setup(dev);
 
intel_setup_bios(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..4f65e12 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2507,6 +2507,7 @@ extern int intel_opregion_notify_encoder(struct 
intel_encoder *intel_encoder,
 bool enable);
 extern int intel_opregion_notify_adapter(struct drm_device *dev,
 pci_power_t state);
+extern void intel_swsci_setup(struct drm_device *dev);
 #else
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
 static inline void intel_opregion_fini(struct drm_device *dev) { return; }
@@ -2521,6 +2522,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 {
return 0;
 }
+static inline void intel_swsci_setup(struct drm_device *dev) { return; }
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index 6845960..579bf24 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -780,13 +780,16 @@ void intel_opregion_fini(struct drm_device *dev)
opregion->lid_state = NULL;
 }
 
-static void swsci_setup(struct drm_device *dev)
+void intel_swsci_setup(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_opregion *opregion = &dev_priv->opregion;
bool requested_callbacks = false;
u32 tmp;
 
+   if (!opregion->swsci)
+   return;
+
/* Sub-function code 0 is okay, let's allow them. */
opregion->swsci_gbda_sub_functions = 1;
opregion->swsci_sbcb_sub_functions = 1;
@@ -836,8 +839,6 @@ static void swsci_setup(struct drm_device *dev)
 opregion->swsci_gbda_sub_functions,
 opregion->swsci_sbcb_sub_functions);
 }
-#else /* CONFIG_ACPI */
-static inline void swsci_setup(struct drm_device *dev) {}
 #endif  /* CONFIG_ACPI */
 
 int intel_opregion_setup(struct drm_device *dev)
@@ -885,7 +886,6 @@ int intel_opregion_setup(struct drm_device *dev)
if (mboxes & MBOX_SWSCI) {
DRM_DEBUG_DRIVER("SWSCI supported\n");
opregion->swsci = base + OPREGION_SWSCI_OFFSET;
-   swsci_setup(dev);
}
if (mboxes & MBOX_ASLE) {
DRM_DEBUG_DRIVER("ASLE supported\n");
-- 
1.8.5.3

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


[Intel-gfx] [PATCH v3 07/12] drm/i915: Add 180 degree sprite rotation support

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

The sprite planes (in fact all display planes starting from gen4)
support 180 degree rotation. Add the relevant low level bits to the
sprite code to make use of that feature.

The upper layers are not yet plugged in.

v2: HSW handles the rotated buffer offset automagically

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 drivers/gpu/drm/i915/intel_drv.h|  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 37 +
 3 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index abd18cd..57906c5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3637,6 +3637,7 @@
 #define   DVS_YUV_ORDER_UYVY   (1<<16)
 #define   DVS_YUV_ORDER_YVYU   (2<<16)
 #define   DVS_YUV_ORDER_VYUY   (3<<16)
+#define   DVS_ROTATE_180   (1<<15)
 #define   DVS_DEST_KEY (1<<2)
 #define   DVS_TRICKLE_FEED_DISABLE (1<<14)
 #define   DVS_TILED(1<<10)
@@ -3707,6 +3708,7 @@
 #define   SPRITE_YUV_ORDER_UYVY(1<<16)
 #define   SPRITE_YUV_ORDER_YVYU(2<<16)
 #define   SPRITE_YUV_ORDER_VYUY(3<<16)
+#define   SPRITE_ROTATE_180(1<<15)
 #define   SPRITE_TRICKLE_FEED_DISABLE  (1<<14)
 #define   SPRITE_INT_GAMMA_ENABLE  (1<<13)
 #define   SPRITE_TILED (1<<10)
@@ -3780,6 +3782,7 @@
 #define   SP_YUV_ORDER_UYVY(1<<16)
 #define   SP_YUV_ORDER_YVYU(2<<16)
 #define   SP_YUV_ORDER_VYUY(3<<16)
+#define   SP_ROTATE_180(1<<15)
 #define   SP_TILED (1<<10)
 #define _SPALINOFF (VLV_DISPLAY_BASE + 0x72184)
 #define _SPASTRIDE (VLV_DISPLAY_BASE + 0x72188)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44067bc..85864fc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -397,6 +397,7 @@ struct intel_plane {
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y;
uint32_t src_w, src_h;
+   unsigned int rotation;
 
/* Since we need to change the watermarks before/after
 * enabling/disabling the planes, we need to store the parameters here
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..477d4d7 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -60,6 +60,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
sprctl &= ~SP_PIXFORMAT_MASK;
sprctl &= ~SP_YUV_BYTE_ORDER_MASK;
sprctl &= ~SP_TILED;
+   sprctl &= ~SP_ROTATE_180;
 
switch (fb->pixel_format) {
case DRM_FORMAT_YUYV:
@@ -131,6 +132,14 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
fb->pitches[0]);
linear_offset -= sprsurf_offset;
 
+   if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+   sprctl |= SP_ROTATE_180;
+
+   x += src_w;
+   y += src_h;
+   linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
+   }
+
I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
 
@@ -238,6 +247,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
sprctl &= ~SPRITE_RGB_ORDER_RGBX;
sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
sprctl &= ~SPRITE_TILED;
+   sprctl &= ~SPRITE_ROTATE_180;
 
switch (fb->pixel_format) {
case DRM_FORMAT_XBGR:
@@ -299,6 +309,17 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
   pixel_size, fb->pitches[0]);
linear_offset -= sprsurf_offset;
 
+   if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+   sprctl |= SPRITE_ROTATE_180;
+
+   /* HSW does this automagically in hardware */
+   if (!IS_HASWELL(dev)) {
+   x += src_w;
+   y += src_h;
+   linear_offset += src_h * fb->pitches[0] + src_w * 
pixel_size;
+   }
+   }
+
I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
 
@@ -422,6 +443,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
dvscntr &= ~DVS_RGB_ORDER_XBGR;
dvscntr &= ~DVS_YUV_BYTE_ORDER_MASK;
dvscntr &= ~DVS_TILED;
+   dvscntr &= ~DVS_ROTATE_180;
 
switch (fb->pixel_format) {
case DRM_FORMAT_XBGR:
@@ -478,6 +500,14 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
   pixel_size, fb->pitches[0]);
linear_offset -= dvssurf_offset;
 
+   if (intel_plane->rotation == BI

[Intel-gfx] [PATCH v3 06/12] drm: Add drm_rotation_simplify()

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

drm_rotation_simplify() can be used to eliminate unsupported rotation
flags. It will check if any unsupported flags are present, and if so
it will modify the rotation to an alternate form by adding 180 degrees
to rotation angle, and flipping the reflect x and y bits. The hope is
that this identity transform will eliminate the unsupported flags.

Of course that might not result in any more supported rotation, so
the caller is still responsible for checking the result afterwards.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_crtc.c | 30 ++
 include/drm/drm_crtc.h |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4983c6e..4f5e408 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4006,6 +4006,36 @@ int drm_format_vert_chroma_subsampling(uint32_t format)
 EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
 
 /**
+ * drm_rotation_simplify() - Try to simplify the rotation
+ * @rotation: Rotation to be simplified
+ * @supported_rotations: Supported rotations
+ *
+ * Attempt to simplify the rotation to a form that is supported.
+ * Eg. if the hardware supports everything except DRM_REFLECT_X
+ * one could call this function like this:
+ *
+ * drm_rotation_simplify(rotation, BIT(DRM_ROTATE_0) |
+ *   BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_180) |
+ *   BIT(DRM_ROTATE_270) | BIT(DRM_REFLECT_Y));
+ *
+ * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
+ * transforms the hardware supports, this function may not
+ * be able to produce a supported transform, so the caller should
+ * check the result afterwards.
+ */
+unsigned int drm_rotation_simplify(unsigned int rotation,
+  unsigned int supported_rotations)
+{
+   if (rotation & ~supported_rotations) {
+   rotation ^= BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y);
+   rotation = (rotation & ~0xf) | BIT((ffs(rotation & 0xf) + 1) % 
4);
+   }
+
+   return rotation;
+}
+EXPORT_SYMBOL(drm_rotation_simplify);
+
+/**
  * drm_mode_config_init - initialize DRM mode_configuration structure
  * @dev: DRM device
  *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 4b3ac70..18f2eed 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1185,6 +1185,8 @@ extern int drm_format_vert_chroma_subsampling(uint32_t 
format);
 extern const char *drm_get_format_name(uint32_t format);
 extern struct drm_property *drm_mode_create_rotation_property(struct 
drm_device *dev,
  unsigned int 
supported_rotations);
+extern unsigned int drm_rotation_simplify(unsigned int rotation,
+ unsigned int supported_rotations);
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 11/12] drm: Set property to return invalid for unsupported arguments for bitmask property

2014-02-06 Thread sagar . a . kamble
From: Sagar Kamble 

DRM will not propagate the set_property call for bitmask drm
properties if they are not supported by underlying driver.

Signed-off-by: Sagar Kamble 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/drm_crtc.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4f5e408..4c92741 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3420,6 +3420,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
*dev, void *data,
struct drm_mode_object *arg_obj;
struct drm_mode_object *prop_obj;
struct drm_property *property;
+   struct drm_property_enum *prop_enum;
+   bool supported_val = false;
int ret = -EINVAL;
int i;
 
@@ -3451,6 +3453,22 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
*dev, void *data,
}
property = obj_to_property(prop_obj);
 
+   if (property->flags | DRM_MODE_PROP_BITMASK) {
+   if (!list_empty(&property->enum_blob_list)) {
+   list_for_each_entry(prop_enum,
+   &property->enum_blob_list, head) {
+   if (BIT(prop_enum->value) == arg->value) {
+   supported_val = true;
+   break;
+   }
+   }
+   }
+   if (!supported_val) {
+   ret = -EINVAL;
+   goto out;
+   }
+   }
+
if (!drm_property_change_is_valid(property, arg->value))
goto out;
 
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 05/12] drm: Add drm_rect rotation functions

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

Add some helper functions to move drm_rects between different rotated
coordinate spaces. One function does the forward transform and
another does the inverse.

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/drm_rect.c | 140 +
 include/drm/drm_rect.h |   6 ++
 2 files changed, 146 insertions(+)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index 7047ca0..631f5af 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -293,3 +293,143 @@ void drm_rect_debug_print(const struct drm_rect *r, bool 
fixed_point)
DRM_DEBUG_KMS("%dx%d%+d%+d\n", w, h, r->x1, r->y1);
 }
 EXPORT_SYMBOL(drm_rect_debug_print);
+
+/**
+ * drm_rect_rotate - Rotate the rectangle
+ * @r: rectangle to be rotated
+ * @width: Width of the coordinate space
+ * @height: Height of the coordinate space
+ * @rotation: Transformation to be applied
+ *
+ * Apply @rotation to the coordinates of rectangle @r.
+ *
+ * @width and @height combined with @rotation define
+ * the location of the new origin.
+ *
+ * @width correcsponds to the horizontal and @height
+ * to the vertical axis of the untransformed coordinate
+ * space.
+ */
+void drm_rect_rotate(struct drm_rect *r,
+int width, int height,
+unsigned int rotation)
+{
+   struct drm_rect tmp;
+
+   if (rotation & (BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y))) {
+   tmp = *r;
+
+   if (rotation & BIT(DRM_REFLECT_X)) {
+   r->x1 = width - tmp.x2;
+   r->x2 = width - tmp.x1;
+   }
+
+   if (rotation & BIT(DRM_REFLECT_Y)) {
+   r->y1 = height - tmp.y2;
+   r->y2 = height - tmp.y1;
+   }
+   }
+
+   switch (rotation & 0xf) {
+   case BIT(DRM_ROTATE_0):
+   break;
+   case BIT(DRM_ROTATE_90):
+   tmp = *r;
+   r->x1 = tmp.y1;
+   r->x2 = tmp.y2;
+   r->y1 = width - tmp.x2;
+   r->y2 = width - tmp.x1;
+   break;
+   case BIT(DRM_ROTATE_180):
+   tmp = *r;
+   r->x1 = width - tmp.x2;
+   r->x2 = width - tmp.x1;
+   r->y1 = height - tmp.y2;
+   r->y2 = height - tmp.y1;
+   break;
+   case BIT(DRM_ROTATE_270):
+   tmp = *r;
+   r->x1 = height - tmp.y2;
+   r->x2 = height - tmp.y1;
+   r->y1 = tmp.x1;
+   r->y2 = tmp.x2;
+   break;
+   default:
+   break;
+   }
+}
+EXPORT_SYMBOL(drm_rect_rotate);
+
+/**
+ * drm_rect_rotate_inv - Inverse rotate the rectangle
+ * @r: rectangle to be rotated
+ * @width: Width of the coordinate space
+ * @height: Height of the coordinate space
+ * @rotation: Transformation whose inverse is to be applied
+ *
+ * Apply the inverse of @rotation to the coordinates
+ * of rectangle @r.
+ *
+ * @width and @height combined with @rotation define
+ * the location of the new origin.
+ *
+ * @width correcsponds to the horizontal and @height
+ * to the vertical axis of the original untransformed
+ * coordinate space, so that you never have to flip
+ * them when doing a rotatation and its inverse.
+ * That is, if you do:
+ *
+ * drm_rotate(&r, width, height, rotation);
+ * drm_rotate_inv(&r, width, height, rotation);
+ *
+ * you will always get back the original rectangle.
+ */
+void drm_rect_rotate_inv(struct drm_rect *r,
+int width, int height,
+unsigned int rotation)
+{
+   struct drm_rect tmp;
+
+   switch (rotation & 0xf) {
+   case BIT(DRM_ROTATE_0):
+   break;
+   case BIT(DRM_ROTATE_90):
+   tmp = *r;
+   r->x1 = width - tmp.y2;
+   r->x2 = width - tmp.y1;
+   r->y1 = tmp.x1;
+   r->y2 = tmp.x2;
+   break;
+   case BIT(DRM_ROTATE_180):
+   tmp = *r;
+   r->x1 = width - tmp.x2;
+   r->x2 = width - tmp.x1;
+   r->y1 = height - tmp.y2;
+   r->y2 = height - tmp.y1;
+   break;
+   case BIT(DRM_ROTATE_270):
+   tmp = *r;
+   r->x1 = tmp.y1;
+   r->x2 = tmp.y2;
+   r->y1 = height - tmp.x2;
+   r->y2 = height - tmp.x1;
+   break;
+   default:
+   break;
+   }
+
+   if (rotation & (BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y))) {
+   tmp = *r;
+
+   if (rotation & BIT(DRM_REFLECT_X)) {
+   r->x1 = width - tmp.x2;
+   r->x2 = width - tmp.x1;
+   }
+
+   if (rotation & BIT(DRM_REFLECT_Y)) {
+   r->y1 = height - tmp.y2;
+   r->y2 = height - tmp.y1;
+   

[Intel-gfx] [PATCH v3 01/12] drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

The rotation property stuff should be standardized among all drivers.
Move the bits to drm_crtc.h from omap_drv.h.

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/omapdrm/omap_drv.h | 7 ---
 include/drm/drm_crtc.h | 8 
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h 
b/drivers/gpu/drm/omapdrm/omap_drv.h
index 428b2981..aac8e10 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -119,13 +119,6 @@ struct omap_drm_private {
struct omap_drm_irq error_handler;
 };
 
-/* this should probably be in drm-core to standardize amongst drivers */
-#define DRM_ROTATE_0   0
-#define DRM_ROTATE_90  1
-#define DRM_ROTATE_180 2
-#define DRM_ROTATE_270 3
-#define DRM_REFLECT_X  4
-#define DRM_REFLECT_Y  5
 
 #ifdef CONFIG_DEBUG_FS
 int omap_debugfs_init(struct drm_minor *minor);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 71727b6..d5c46c1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -65,6 +65,14 @@ struct drm_object_properties {
uint64_t values[DRM_OBJECT_MAX_PROPERTY];
 };
 
+/* rotation property bits */
+#define DRM_ROTATE_0   0
+#define DRM_ROTATE_90  1
+#define DRM_ROTATE_180 2
+#define DRM_ROTATE_270 3
+#define DRM_REFLECT_X  4
+#define DRM_REFLECT_Y  5
+
 /*
  * Note on terminology:  here, for brevity and convenience, we refer to 
connector
  * control chips as 'CRTCs'.  They can control any type of connector, VGA, 
LVDS,
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 12/12] drm/i915: Removing rotate and inverse rotate calls from update_plane

2014-02-06 Thread sagar . a . kamble
From: Sagar Kamble 

With clipped sprites these transformations are not working. these
functions transform complete sprite irrespective of clipping present.
This leads to invisible portion of sprite show up when rotate 180 if
it was out of visible area before.

Signed-off-by: Sagar Kamble 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/i915/intel_sprite.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 62b9f84..e7e4c55 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
max_scale = intel_plane->max_downscale << 16;
min_scale = intel_plane->can_scale ? 1 : (1 << 16);
 
-   drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
-   intel_plane->rotation);
-
hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
BUG_ON(hscale < 0);
 
@@ -810,9 +807,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
 drm_rect_width(&dst) * hscale - 
drm_rect_width(&src),
 drm_rect_height(&dst) * vscale - 
drm_rect_height(&src));
 
-   drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
-   intel_plane->rotation);
-
/* sanity check to make sure the src viewport wasn't enlarged */
WARN_ON(src.x1 < (int) src_x ||
src.y1 < (int) src_y ||
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 02/12] drm: Add support_bits parameter to drm_property_create_bitmask()

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

Make drm_property_create_bitmask() a bit more generic by allowing the
caller to specify which bits are in fact supported. This allows multiple
callers to use the same enum list, but still create different versions
of the same property with different list of supported bits.

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/drm_crtc.c | 6 +-
 include/drm/drm_crtc.h | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3b7d32d..628d3d3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2906,7 +2906,8 @@ EXPORT_SYMBOL(drm_property_create_enum);
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 int flags, const char *name,
 const struct drm_prop_enum_list *props,
-int num_values)
+int num_values,
+unsigned int supported_bits)
 {
struct drm_property *property;
int i, ret;
@@ -2918,6 +2919,9 @@ struct drm_property *drm_property_create_bitmask(struct 
drm_device *dev,
return NULL;
 
for (i = 0; i < num_values; i++) {
+   if (!(supported_bits & (1 << i)))
+   continue;
+
ret = drm_property_add_enum(property, i,
  props[i].type,
  props[i].name);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d5c46c1..41b86d2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1070,7 +1070,8 @@ extern struct drm_property 
*drm_property_create_enum(struct drm_device *dev, int
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 int flags, const char *name,
 const struct drm_prop_enum_list *props,
-int num_values);
+int num_values,
+unsigned int supported_bits);
 struct drm_property *drm_property_create_range(struct drm_device *dev, int 
flags,
 const char *name,
 uint64_t min, uint64_t max);
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 04/12] drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

Use the new drm_mode_create_rotation_property() in omapdrm.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
b/drivers/gpu/drm/omapdrm/omap_plane.c
index 046d5e6..fee8f35 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -300,16 +300,13 @@ void omap_plane_install_properties(struct drm_plane 
*plane,
if (priv->has_dmm) {
prop = priv->rotation_prop;
if (!prop) {
-   const struct drm_prop_enum_list props[] = {
-   { DRM_ROTATE_0,   "rotate-0" },
-   { DRM_ROTATE_90,  "rotate-90" },
-   { DRM_ROTATE_180, "rotate-180" },
-   { DRM_ROTATE_270, "rotate-270" },
-   { DRM_REFLECT_X,  "reflect-x" },
-   { DRM_REFLECT_Y,  "reflect-y" },
-   };
-   prop = drm_property_create_bitmask(dev, 0, "rotation",
-   props, ARRAY_SIZE(props));
+   prop = drm_mode_create_rotation_property(dev,
+
BIT(DRM_ROTATE_0) |
+
BIT(DRM_ROTATE_90) |
+
BIT(DRM_ROTATE_180) |
+
BIT(DRM_ROTATE_270) |
+
BIT(DRM_REFLECT_X) |
+
BIT(DRM_REFLECT_Y));
if (prop == NULL)
return;
priv->rotation_prop = prop;
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 03/12] drm: Add drm_mode_create_rotation_property()

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

Add a function to create a standards compliant rotation property.

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/drm_crtc.c | 18 ++
 include/drm/drm_crtc.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 628d3d3..4983c6e 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4118,3 +4118,21 @@ void drm_mode_config_cleanup(struct drm_device *dev)
idr_destroy(&dev->mode_config.crtc_idr);
 }
 EXPORT_SYMBOL(drm_mode_config_cleanup);
+
+struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+  unsigned int 
supported_rotations)
+{
+   static const struct drm_prop_enum_list props[] = {
+   { DRM_ROTATE_0,   "rotate-0" },
+   { DRM_ROTATE_90,  "rotate-90" },
+   { DRM_ROTATE_180, "rotate-180" },
+   { DRM_ROTATE_270, "rotate-270" },
+   { DRM_REFLECT_X,  "reflect-x" },
+   { DRM_REFLECT_Y,  "reflect-y" },
+   };
+
+   return drm_property_create_bitmask(dev, 0, "rotation",
+  props, ARRAY_SIZE(props),
+  supported_rotations);
+}
+EXPORT_SYMBOL(drm_mode_create_rotation_property);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 41b86d2..4b3ac70 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1183,6 +1183,8 @@ extern int drm_format_plane_cpp(uint32_t format, int 
plane);
 extern int drm_format_horz_chroma_subsampling(uint32_t format);
 extern int drm_format_vert_chroma_subsampling(uint32_t format);
 extern const char *drm_get_format_name(uint32_t format);
+extern struct drm_property *drm_mode_create_rotation_property(struct 
drm_device *dev,
+ unsigned int 
supported_rotations);
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 09/12] drm/i915: Add rotation property for sprites

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

Sprite planes support 180 degree rotation. The lower layers are now in
place, so hook in the standard rotation property to expose the feature
to the users.

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 42 -
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fa37dfd..ea2efc3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1548,6 +1548,7 @@ typedef struct drm_i915_private {
 
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+   struct drm_property *rotation_property;
 
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 511934c..62b9f84 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1047,6 +1047,30 @@ out_unlock:
return ret;
 }
 
+static int intel_plane_set_property(struct drm_plane *plane,
+   struct drm_property *prop,
+   uint64_t val)
+{
+   struct drm_i915_private *dev_priv = plane->dev->dev_private;
+   struct intel_plane *intel_plane = to_intel_plane(plane);
+   uint64_t old_val;
+   int ret = -ENOENT;
+
+   if (prop == dev_priv->rotation_property) {
+   /* exactly one rotation angle please */
+   if (hweight32(val & 0xf) != 1)
+   return -EINVAL;
+
+   old_val = intel_plane->rotation;
+   intel_plane->rotation = val;
+   ret = intel_plane_restore(plane);
+   if (ret)
+   intel_plane->rotation = old_val;
+   }
+
+   return ret;
+}
+
 int intel_plane_restore(struct drm_plane *plane)
 {
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -1073,6 +1097,7 @@ static const struct drm_plane_funcs intel_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
.destroy = intel_destroy_plane,
+   .set_property = intel_plane_set_property,
 };
 
 static uint32_t ilk_plane_formats[] = {
@@ -1109,6 +1134,7 @@ static uint32_t vlv_plane_formats[] = {
 int
 intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 {
+   struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_plane *intel_plane;
unsigned long possible_crtcs;
const uint32_t *plane_formats;
@@ -1183,8 +1209,22 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, 
int plane)
 &intel_plane_funcs,
 plane_formats, num_plane_formats,
 false);
-   if (ret)
+   if (ret) {
kfree(intel_plane);
+   goto out;
+   }
+
+   if (!dev_priv->rotation_property)
+   dev_priv->rotation_property =
+   drm_mode_create_rotation_property(dev,
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
+
+   if (dev_priv->rotation_property)
+   drm_object_attach_property(&intel_plane->base.base,
+  dev_priv->rotation_property,
+  intel_plane->rotation);
 
+ out:
return ret;
 }
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 08/12] drm/i915: Make intel_plane_restore() return an error

2014-02-06 Thread sagar . a . kamble
From: Ville Syrjälä 

Propagate the error from intel_update_plane() up through
intel_plane_restore() to the caller. This will be used for
rollback purposes when setting properties fails.

Signed-off-by: Ville Syrjälä 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/i915/intel_drv.h|  2 +-
 drivers/gpu/drm/i915/intel_sprite.c | 14 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85864fc..7a79b8e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -897,7 +897,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t 
sdvo_reg, bool is_sdvob);
 int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
 void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
   enum plane plane);
-void intel_plane_restore(struct drm_plane *plane);
+int intel_plane_restore(struct drm_plane *plane);
 void intel_plane_disable(struct drm_plane *plane);
 int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
  struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 477d4d7..511934c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1047,18 +1047,18 @@ out_unlock:
return ret;
 }
 
-void intel_plane_restore(struct drm_plane *plane)
+int intel_plane_restore(struct drm_plane *plane)
 {
struct intel_plane *intel_plane = to_intel_plane(plane);
 
if (!plane->crtc || !plane->fb)
-   return;
+   return 0;
 
-   intel_update_plane(plane, plane->crtc, plane->fb,
-  intel_plane->crtc_x, intel_plane->crtc_y,
-  intel_plane->crtc_w, intel_plane->crtc_h,
-  intel_plane->src_x, intel_plane->src_y,
-  intel_plane->src_w, intel_plane->src_h);
+   return intel_update_plane(plane, plane->crtc, plane->fb,
+ intel_plane->crtc_x, intel_plane->crtc_y,
+ intel_plane->crtc_w, intel_plane->crtc_h,
+ intel_plane->src_x, intel_plane->src_y,
+ intel_plane->src_w, intel_plane->src_h);
 }
 
 void intel_plane_disable(struct drm_plane *plane)
-- 
1.8.5

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


[Intel-gfx] [PATCH v3 10/12] drm/i915: Add 180 degree primary plane rotation support

2014-02-06 Thread sagar . a . kamble
From: Sagar Kamble 

Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.

v2: Calculating linear/tiled offsets based on pipe source width and
height. Added 180 degree rotation support in ironlake_update_plane.

v3: Checking if CRTC is active before issueing update_plane. Added
wait for vblank to make sure we dont overtake page flips. Disabling
FBC since it does not work with rotated planes.

Signed-off-by: Sagar Kamble 
Tested-by: Sagar Kamble 
---
 drivers/gpu/drm/i915/i915_reg.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 75 ++--
 drivers/gpu/drm/i915/intel_drv.h |  2 +
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 57906c5..d3000c4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3553,6 +3553,7 @@
 #define   DISPPLANE_NO_LINE_DOUBLE 0
 #define   DISPPLANE_STEREO_POLARITY_FIRST  0
 #define   DISPPLANE_STEREO_POLARITY_SECOND (1<<18)
+#define   DISPPLANE_ROTATE_180 (1<<15)
 #define   DISPPLANE_TRICKLE_FEED_DISABLE   (1<<14) /* Ironlake */
 #define   DISPPLANE_TILED  (1<<10)
 #define _DSPAADDR  (dev_priv->info->display_mmio_offset + 0x70184)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..961308d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
struct drm_framebuffer *fb,
unsigned long linear_offset;
u32 dspcntr;
u32 reg;
+   int pixel_size;
 
switch (plane) {
case 0:
@@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
struct drm_framebuffer *fb,
return -EINVAL;
}
 
+   pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb->obj;
 
@@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
struct drm_framebuffer *fb,
dspcntr = I915_READ(reg);
/* Mask out pixel format bits in case we change it */
dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+   dspcntr &= ~DISPPLANE_ROTATE_180;
+
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
struct drm_framebuffer *fb,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
-   I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
 
if (INTEL_INFO(dev)->gen >= 4) {
@@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
struct drm_framebuffer *fb,
intel_crtc->dspaddr_offset = linear_offset;
}
 
+   if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
+   dspcntr |= DISPPLANE_ROTATE_180;
+
+   x += (intel_crtc->config.pipe_src_w - 1);
+   y += (intel_crtc->config.pipe_src_h - 1);
+   linear_offset += (intel_crtc->config.pipe_src_h - 1) * 
fb->pitches[0] +
+   intel_crtc->config.pipe_src_w * pixel_size;
+   }
+
+   I915_WRITE(reg, dspcntr);
+
DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
  i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
  fb->pitches[0]);
@@ -2137,6 +2150,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
unsigned long linear_offset;
u32 dspcntr;
u32 reg;
+   int pixel_size;
 
switch (plane) {
case 0:
@@ -2148,6 +2162,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
return -EINVAL;
}
 
+   pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb->obj;
 
@@ -2155,6 +2170,8 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
dspcntr = I915_READ(reg);
/* Mask out pixel format bits in case we change it */
dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+   dspcntr &= ~DISPPLANE_ROTATE_180;
+
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2192,8 +2209,6 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
else
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
-   I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
intel_crtc->dspaddr_offset =
intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
@@ -2201,6 +2216,19 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
   fb->pitches[0]);
linear_of

[Intel-gfx] [PATCH 1/1] tests/kms_rotate_crc: Test to verify rotation of planes through CRC

2014-02-06 Thread sagar . a . kamble
From: Sagar Kamble 

This test will verify 180 degree rotation of CRTC and sprite planes
by grabbing CRC for already rotated image and comparing with CRC calculated
after triggering rotation through DRM property.

Signed-off-by: Sagar Kamble 
Tested-by: Sagar Kamble 
---
 tests/Makefile.sources |   1 +
 tests/kms_rotate_crc.c | 543 +
 2 files changed, 544 insertions(+)
 create mode 100644 tests/kms_rotate_crc.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a8c0c96..c19fc88 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -59,6 +59,7 @@ TESTS_progs_M = \
kms_flip \
kms_pipe_crc_basic \
kms_render \
+   kms_rotate_crc \
kms_setmode \
pm_lpsp \
pm_pc8 \
diff --git a/tests/kms_rotate_crc.c b/tests/kms_rotate_crc.c
new file mode 100644
index 000..5d218c9
--- /dev/null
+++ b/tests/kms_rotate_crc.c
@@ -0,0 +1,543 @@
+/*
+ * Copyright ?? 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Author:
+ *Sagar Kamble 
+ */
+
+
+#include 
+#include 
+#include 
+#include 
+
+#include "drm_fourcc.h"
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+
+#define DRM_ROTATE_0   0
+#define DRM_ROTATE_90  1
+#define DRM_ROTATE_180 2
+#define DRM_ROTATE_270 3
+#define DRM_REFLECT_X  4
+#define DRM_REFLECT_Y  5
+#define DRM_ROTATE_NUM 6
+
+#define BIT(x) (1 << x)
+
+FILE *fp = NULL;
+
+typedef struct {
+   struct kmstest_connector_config config;
+   drmModeModeInfo mode;
+   struct kmstest_fb fb[DRM_ROTATE_NUM];
+   struct kmstest_fb sprite_fb[DRM_ROTATE_NUM]
+} connector_t;
+
+typedef struct {
+   int drm_fd;
+   igt_debugfs_t debugfs;
+   drmModeRes *resources;
+   igt_crc_t ref_crtc_crc[DRM_ROTATE_NUM];
+   igt_crc_t ref_sprite_crc[DRM_ROTATE_NUM];
+   igt_pipe_crc_t **pipe_crc;
+   uint32_t crtc_id;
+   uint32_t sprite_id;
+   uint32_t crtc_idx;
+   uint32_t crtc_fb_id[DRM_ROTATE_NUM];
+   uint32_t sprite_fb_id[DRM_ROTATE_NUM];
+} data_t;
+
+static void set_rotation(int drm_fd, bool is_sprite, uint32_t plane_id, 
uint64_t rotation)
+{
+   int i = 0, j = 0, ret = 0;
+   drmModeObjectPropertiesPtr props = NULL;
+
+   props = drmModeObjectGetProperties(drm_fd, plane_id,
+   is_sprite ? DRM_MODE_OBJECT_PLANE : 
DRM_MODE_OBJECT_CRTC);
+   fprintf(fp, "\nPlane id: 0x%x, Count_props=%d ", plane_id, 
props->count_props);
+
+   for (i = 0; i < props->count_props; i++) 
+   {
+   drmModePropertyPtr prop = drmModeGetProperty(drm_fd, 
props->props[i]);
+   fprintf(fp, "\nProp->name=%s ", prop->name);
+
+   if (strcmp(prop->name, "rotation") == 0) 
+   {
+   igt_assert(prop->flags & DRM_MODE_PROP_BITMASK);
+   fprintf(fp, "\nRotation property enum count %d", 
prop->count_enums);
+   fprintf(fp, "\nRotation type\tValue");
+   for (j = 0; j < prop->count_enums; j++)
+   fprintf(fp, "\n%s: 0x%x", prop->enums[j].name, 
prop->enums[j].value); 
+   
+   ret = drmModeObjectSetProperty(drm_fd, plane_id, 
is_sprite ? DRM_MODE_OBJECT_PLANE : DRM_MODE_OBJECT_CRTC,
+   
(uint32_t)prop->prop_id, rotation);
+   if (ret)
+   {
+   fprintf(fp, "\nRotation setting\(0x%x\) failed 
!!!", rotation);
+   return;
+   }
+   else
+   fprintf(fp, "\nPlane with id 0x%x is rotated 
with setting 0x%x", plane_id, rotation);   
+   }
+   drmModeFreeProp

[Intel-gfx] [PATCH v3 00/12] Enabling 180 degree rotation for sprite and crtc planes

2014-02-06 Thread sagar . a . kamble
From: Sagar Kamble 

These patches will enable 180 degree rotation for CRTC and Sprite planes. This
version has following changes:
1. Addressed review comments for CRTC rotation from FBC, page flip, CRTC active/
inactive perspective.
2. Removed drm_rect_rotate amd drm_rect_rotate_inv functions as they dont take
care of clipped planes and 180 rotation in that case produces invalid result.

Sagar Kamble (3):
  drm/i915: Add 180 degree primary plane rotation support
  drm: Set property to return invalid for unsupported arguments for
bitmask property
  drm/i915: Removing rotate and inverse rotate calls from update_plane

Ville Syrjälä (9):
  drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
  drm: Add support_bits parameter to drm_property_create_bitmask()
  drm: Add drm_mode_create_rotation_property()
  drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
  drm: Add drm_rect rotation functions
  drm: Add drm_rotation_simplify()
  drm/i915: Add 180 degree sprite rotation support
  drm/i915: Make intel_plane_restore() return an error
  drm/i915: Add rotation property for sprites

 drivers/gpu/drm/drm_crtc.c   |  72 +-
 drivers/gpu/drm/drm_rect.c   | 140 +++
 drivers/gpu/drm/i915/i915_drv.h  |   1 +
 drivers/gpu/drm/i915/i915_reg.h  |   4 +
 drivers/gpu/drm/i915/intel_display.c |  75 ++-
 drivers/gpu/drm/i915/intel_drv.h |   5 +-
 drivers/gpu/drm/i915/intel_sprite.c  |  87 --
 drivers/gpu/drm/omapdrm/omap_drv.h   |   7 --
 drivers/gpu/drm/omapdrm/omap_plane.c |  17 ++---
 include/drm/drm_crtc.h   |  15 +++-
 include/drm/drm_rect.h   |   6 ++
 11 files changed, 397 insertions(+), 32 deletions(-)

-- 
1.8.5

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


Re: [Intel-gfx] Add reset subtest to pm_rps

2014-02-06 Thread Jeff McGee
On Thu, Feb 06, 2014 at 07:48:18AM +0100, Daniel Vetter wrote:
> On Wed, Feb 05, 2014 at 01:21:34PM -0600, Jeff McGee wrote:
> > Gentle ping on this patch set for igt.
> 
> Both series pulled in, sorry for slacking off a bit.
> -Daniel

Thanks, Daniel. The two driver patches to fix the failing subtests have
been updated and sent to the list. I suppose the process is for QA to
confirm the failures and then the patches are considered. I'll be out
for the next 4 weeks, but I'll let my teammates know to support any
future requests on this topic.
-Jeff
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 8/9] drm/i915/bdw: Implement a basic PM interrupt handler

2014-02-06 Thread Rodrigo Vivi
On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> Almost all of it is reusable from the existing code. The primary
> difference is we need to do even less in the interrupt handler, since
> interrupts are not shared in the same way.
>
> The patch is mostly a copy-paste of the existing snb+ code, with updates
> to the relevant parts requiring changes to the interrupt handling. As
> such it /should/ be relatively trivial. It's highly likely that I missed
> some places where I need a gen8 version of the PM interrupts, but it has
> become invisible to me by now.
>
> This patch could probably be split into adding the new functions,
> followed by actually handling the interrupts. Since the code is
> currently disabled (and broken) I think the patch stands better by
> itself.
>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/i915_irq.c  | 80 
> ++--
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  drivers/gpu/drm/i915/intel_pm.c  | 39 +++-
>  4 files changed, 117 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 72ade87..ab0c7ac 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -214,6 +214,53 @@ static bool ivb_can_enable_err_int(struct drm_device 
> *dev)
> return true;
>  }
>
> +/**
> +  * bdw_update_pm_irq - update GT interrupt 2
> +  * @dev_priv: driver private
> +  * @interrupt_mask: mask of interrupt bits to update
> +  * @enabled_irq_mask: mask of interrupt bits to enable
> +  *
> +  * Copied from the snb function, updated with relevant register offsets
> +  */
> +static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
> + uint32_t interrupt_mask,
> + uint32_t enabled_irq_mask)
> +{
> +   uint32_t new_val;
> +
> +   assert_spin_locked(&dev_priv->irq_lock);
> +
> +   if (dev_priv->pc8.irqs_disabled) {
> +   WARN(1, "IRQs disabled\n");
> +   dev_priv->pc8.regsave.gen6_pmimr &= ~interrupt_mask;
> +   dev_priv->pc8.regsave.gen6_pmimr |= (~enabled_irq_mask &
> +interrupt_mask);
> +   return;
> +   }
> +
> +   new_val = dev_priv->pm_irq_mask;
> +   new_val &= ~interrupt_mask;
> +   new_val |= (~enabled_irq_mask & interrupt_mask);
> +
> +   if (new_val != dev_priv->pm_irq_mask) {
> +   dev_priv->pm_irq_mask = new_val;
> +   I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
> +  dev_priv->pm_irq_mask);
> +   POSTING_READ(GEN8_GT_IMR(2));
> +   }
> +}
> +
> +void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> +{
> +   bdw_update_pm_irq(dev_priv, mask, mask);
> +}
> +
> +void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> +{
> +   bdw_update_pm_irq(dev_priv, mask, 0);
> +}
> +
> +
>  static bool cpt_can_enable_serr_int(struct drm_device *dev)
>  {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -997,11 +1044,15 @@ static void gen6_pm_rps_work(struct work_struct *work)
> pm_iir = dev_priv->rps.pm_iir;
> dev_priv->rps.pm_iir = 0;
> /* Make sure not to corrupt PMIMR state used by ringbuffer code */
> -   snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
> +   if (IS_BROADWELL(dev_priv->dev))
> +   bdw_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
> +   else {
> +   /* Make sure we didn't queue anything we're not going to 
> process. */
> +   snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);

inverted lines since comment sounds to be related to above line.

> +   WARN_ON(pm_iir & ~GEN6_PM_RPS_EVENTS);
> +   }
> spin_unlock_irq(&dev_priv->irq_lock);
>
> -   /* Make sure we didn't queue anything we're not going to process. */
> -   WARN_ON(pm_iir & ~GEN6_PM_RPS_EVENTS);
>
> if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
> return;
> @@ -1192,6 +1243,19 @@ static void snb_gt_irq_handler(struct drm_device *dev,
> ivybridge_parity_error_irq_handler(dev, gt_iir);
>  }
>
> +static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv, u32 
> pm_iir)
> +{
> +   if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
> +   return;
> +
> +   spin_lock(&dev_priv->irq_lock);
> +   dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS;
> +   bdw_disable_pm_irq(dev_priv, pm_iir & GEN6_PM_RPS_EVENTS);
> +   spin_unlock(&dev_priv->irq_lock);
> +
> +   queue_work(dev_priv->wq, &dev_priv->rps.work);

sorry if this is a stupid question, but don't we need the vebox part here?

> +}
> +
>  static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
>struct drm_i915_p

Re: [Intel-gfx] [PATCH 9/9] drm/i915/bdw: Enable RC6

2014-02-06 Thread Rodrigo Vivi
On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> It is tested and looking fairly stable now, so turn it on. It wasn't
> intentionally turned off originally :P
>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a5c9142..377bd7f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4428,7 +4428,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
> ironlake_enable_drps(dev);
> ironlake_enable_rc6(dev);
> intel_init_emon(dev);
> -   } else if (IS_GEN6(dev) || IS_GEN7(dev)) {
> +   } else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_BROADWELL(dev)) {

why not IS_GEN8?
or why not use gen >= gen6?

anyway: Reviewed-by: Rodrigo Vivi 

> /*
>  * PCU communication is slow and this doesn't need to be
>  * done at any specific time, so do this out of our fast path
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


Re: [Intel-gfx] [PATCH 7/9] drm/i915/bdw: RPS frequency bits are the same as HSW

2014-02-06 Thread Rodrigo Vivi
Reviewed-by: Rodrigo Vivi 

On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 34cc898..deaaaf2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3016,7 +3016,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>
> gen6_set_rps_thresholds(dev_priv, val);
>
> -   if (IS_HASWELL(dev))
> +   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> I915_WRITE(GEN6_RPNSWREQ,
>HSW_FREQUENCY(val));
> else
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


Re: [Intel-gfx] [PATCH 5/9] drm/i915/bdw: Set rp_state_caps

2014-02-06 Thread Rodrigo Vivi
On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6acb429..ae59bd9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3184,6 +3184,17 @@ static void gen6_enable_rps_interrupts(struct 
> drm_device *dev)
> I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
>  }
>
> +static void parse_rp_state_cap(struct drm_i915_private *dev_priv, u32 
> rp_state_cap)

line over 80, but meh
Reviewed-by: Rodrigo Vivi 

> +{
> +   /* In units of 50MHz */
> +   dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff;
> +   dev_priv->rps.min_delay = (rp_state_cap >> 16) & 0xff;
> +   dev_priv->rps.rp1_delay = (rp_state_cap >>  8) & 0xff;
> +   dev_priv->rps.rp0_delay = (rp_state_cap >>  0) & 0xff;
> +   dev_priv->rps.rpe_delay = dev_priv->rps.rp1_delay;
> +   dev_priv->rps.cur_delay = 0;
> +}
> +
>  static void gen8_enable_rps(struct drm_device *dev)
>  {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3202,6 +3213,7 @@ static void gen8_enable_rps(struct drm_device *dev)
> I915_WRITE(GEN6_RC_CONTROL, 0);
>
> rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> +   parse_rp_state_cap(dev_priv, rp_state_cap);
>
> /* 2b: Program RC6 thresholds.*/
> I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
> @@ -3288,13 +3300,7 @@ static void gen6_enable_rps(struct drm_device *dev)
> rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
>
> -   /* In units of 50MHz */
> -   dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff;
> -   dev_priv->rps.min_delay = (rp_state_cap >> 16) & 0xff;
> -   dev_priv->rps.rp1_delay = (rp_state_cap >>  8) & 0xff;
> -   dev_priv->rps.rp0_delay = (rp_state_cap >>  0) & 0xff;
> -   dev_priv->rps.rpe_delay = dev_priv->rps.rp1_delay;
> -   dev_priv->rps.cur_delay = 0;
> +   parse_rp_state_cap(dev_priv, rp_state_cap);
>
> /* disable the counters and set deterministic thresholds */
> I915_WRITE(GEN6_RC_CONTROL, 0);
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


Re: [Intel-gfx] [PATCH 4/9] drm/i915/bdw: Use centralized rc6 info print

2014-02-06 Thread Rodrigo Vivi
Reviewed-by: Rodrigo Vivi 

On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 944b99c..6acb429 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct drm_device *dev)
> /* 3: Enable RC6 */
> if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
> rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
> -   DRM_INFO("RC6 %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : 
> "off");
> +   intel_print_rc6_info(dev, rc6_mask);
> I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -   GEN6_RC_CTL_EI_MODE(1) |
> -   rc6_mask);
> +   GEN6_RC_CTL_EI_MODE(1) |
> +   rc6_mask);
>
> /* 4 Program defaults and thresholds for RPS*/
> I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request 500 MHz */
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


Re: [Intel-gfx] [PATCH 3/9] drm/i915: Just print rc6 facts

2014-02-06 Thread Rodrigo Vivi
Reviewed-by: Rodrigo Vivi 

On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> Everything can be overridden by module parameters, so don't confuse the
> users that are using them.
>
> We have RC6 turned on for all platforms which support it, but Ironlake,
> so the need to explain the situation is no longer pressing.
>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 258241b..944b99c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3135,16 +3135,10 @@ static void valleyview_disable_rps(struct drm_device 
> *dev)
>
>  static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
>  {
> -   if (IS_GEN6(dev))
> -   DRM_DEBUG_DRIVER("Sandybridge: deep RC6 disabled\n");
> -
> -   if (IS_HASWELL(dev))
> -   DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
> -
> DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
> -   (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> -   (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
> -   (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
> +(mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> +(mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
> +(mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
>  }
>
>  int intel_enable_rc6(const struct drm_device *dev)
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


Re: [Intel-gfx] [PATCH 2/9] drm/i915: Stop pretending VLV has rc6+

2014-02-06 Thread Rodrigo Vivi
hehehe I liked the title! :)

Reviewed-by: Rodrigo Vivi 

On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> It wasn't ever used by the caller anyway with the exception of what we
> show in sysfs.
>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bcbdac2..258241b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3161,7 +3161,7 @@ int intel_enable_rc6(const struct drm_device *dev)
> if (INTEL_INFO(dev)->gen == 5)
> return 0;
>
> -   if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev))
> +   if (IS_IVYBRIDGE(dev))
> return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
> else
> return INTEL_RC6_ENABLE;
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


Re: [Intel-gfx] [PATCH 1/9] drm/i915: Clarify RC6 enabling

2014-02-06 Thread Rodrigo Vivi
Reviewed-by: Rodrigo Vivi 

On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky
 wrote:
> At one time, we though all future platforms would have the deeper RC6
> states. As it turned out, they killed it after Ivybridge, and began
> using other means to achieve the power savings (the stuff we need to get
> to PC7+).
>
> The enable function was left in a weird state of odd corner cases as a
> result. Since the future is now, and we also have some insight into
> what's currently the future, we have an opportunity to simplify, and
> future proof the function.
>
> NOTE: VLV will be addressed in a subsequent patch. This patch was trying
> not to change functionality.
>
> NOTE2: All callers sanitize the return value anyway, so this patch is
> simply to have the code make a bit more sense.
>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 53d64bb..bcbdac2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3161,14 +3161,10 @@ int intel_enable_rc6(const struct drm_device *dev)
> if (INTEL_INFO(dev)->gen == 5)
> return 0;
>
> -   if (IS_HASWELL(dev))
> -   return INTEL_RC6_ENABLE;
> -
> -   /* snb/ivb have more than one rc6 state. */
> -   if (INTEL_INFO(dev)->gen == 6)
> +   if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev))
> +   return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
> +   else
> return INTEL_RC6_ENABLE;
> -
> -   return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
>  }
>
>  static void gen6_enable_rps_interrupts(struct drm_device *dev)
> --
> 1.8.5.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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


[Intel-gfx] [PATCH] drm/i915: Record pid/comm of hanging task

2014-02-06 Thread Chris Wilson
After finding the guilty batch and request, we can use it to find the
process that submitted the batch and then add the culprit into the error
state.

This is a slightly different approach from Ben's in that instead of
adding the extra information into the struct i915_hw_context, we use the
information already captured in struct drm_file which is then referenced
from the request.

v2: Also capture the workaround buffer for gen2, so that we can compare
its contents against the intended batch for the active request.

Link: http://lists.freedesktop.org/archives/intel-gfx/2013-August/032280.html
Signed-off-by: Chris Wilson 
Cc: Ben Widawsky 
Cc: Mika Kuoppala 
---

Jesse would like similar information included with the error message, as
Mika is working on improving that error message, I delegate that task to
him. ;-)
-Chris

---
 drivers/gpu/drm/i915/i915_drv.h   |   6 +-
 drivers/gpu/drm/i915/i915_gem.c   |   1 +
 drivers/gpu/drm/i915/i915_gpu_error.c | 115 +-
 3 files changed, 78 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1d317f8c9f05..159f94637f3a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -359,7 +359,7 @@ struct drm_i915_error_state {
int page_count;
u32 gtt_offset;
u32 *pages[0];
-   } *ringbuffer, *batchbuffer, *ctx, *hws_page;
+   } *ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page;
 
struct drm_i915_error_request {
long jiffies;
@@ -374,6 +374,9 @@ struct drm_i915_error_state {
u32 pp_dir_base;
};
} vm_info;
+
+   pid_t pid;
+   char comm[TASK_COMM_LEN];
} ring[I915_NUM_RINGS];
 
struct drm_i915_error_buffer {
@@ -1825,6 +1828,7 @@ struct drm_i915_gem_request {
 
 struct drm_i915_file_private {
struct drm_i915_private *dev_priv;
+   struct drm_file *file;
 
struct {
spinlock_t lock;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 157877bd7a0b..605e4ab636e8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4981,6 +4981,7 @@ int i915_gem_open(struct drm_device *dev, struct drm_file 
*file)
 
file->driver_priv = file_priv;
file_priv->dev_priv = dev->dev_private;
+   file_priv->file = file;
 
spin_lock_init(&file_priv->mm.lock);
INIT_LIST_HEAD(&file_priv->mm.request_list);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0ff158939f1d..c1da48b5189d 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -302,13 +302,28 @@ void i915_error_printf(struct drm_i915_error_state_buf 
*e, const char *f, ...)
va_end(args);
 }
 
+static void print_error_obj(struct drm_i915_error_state_buf *m,
+   struct drm_i915_error_object *obj)
+{
+   int page, offset, elt;
+
+   for (page = offset = 0; page < obj->page_count; page++) {
+   for (elt = 0; elt < PAGE_SIZE/4; elt++) {
+   err_printf(m, "%08x :  %08x\n", offset,
+  obj->pages[page][elt]);
+   offset += 4;
+   }
+   }
+}
+
 int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
const struct i915_error_state_file_priv *error_priv)
 {
struct drm_device *dev = error_priv->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_error_state *error = error_priv->error;
-   int i, j, page, offset, elt;
+   int i, j, offset, elt;
+   int max_hangcheck_score;
 
if (!error) {
err_printf(m, "no error state collected\n");
@@ -318,6 +333,20 @@ int i915_error_state_to_str(struct 
drm_i915_error_state_buf *m,
err_printf(m, "Time: %ld s %ld us\n", error->time.tv_sec,
   error->time.tv_usec);
err_printf(m, "Kernel: " UTS_RELEASE "\n");
+   max_hangcheck_score = 0;
+   for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
+   if (error->ring[i].hangcheck_score > max_hangcheck_score)
+   max_hangcheck_score = error->ring[i].hangcheck_score;
+   }
+   for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
+   if (error->ring[i].hangcheck_score == max_hangcheck_score &&
+   error->ring[i].pid != -1) {
+   err_printf(m, "Active process (on ring %s): %s [%d]\n",
+  ring_str(i),
+  error->ring[i].comm,
+  error->ring[i].pid);
+   }
+   }
err_printf(m, "PCI ID: 0x%04x\n", d

Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

2014-02-06 Thread Chris Wilson
On Thu, Feb 06, 2014 at 10:22:28AM +, Goel, Akash wrote:
> Please kindly review this patch.
> 
> Best regards
> Akash
> -Original Message-
> From: Goel, Akash 
> Sent: Thursday, January 09, 2014 5:55 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Goel, Akash
> Subject: [PATCH] drm/i915/vlv: Added write-enable pte bit support
> 
> From: Akash Goel 
> 
> This adds support for using the write-enable bit in the GTT entry for VLV.
> This is handled via a read-only flag in the GEM buffer object which is then 
> used to check if the write-enable bit has to be set or not when writing the 
> GTT entries.
> Currently by default only the Batch buffer & Ring buffers are being marked as 
> read only.

Don't cause us to rewrite the PTE for the batch buffer between each
execbuffer (ro for the batch, rw next time it gets used as a texture).
In fact, do not change ro without user intervention.

gt_old_ro is unused

Use byt_pte_encode() instead of hacking the result of
ppgtt->pte_encode().

Consider expanding i915_cache_level so that it included the concept of
PROT_READ | PROT_WRITE.
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

2014-02-06 Thread Goel, Akash
Please kindly review this patch.

Best regards
Akash
-Original Message-
From: Goel, Akash 
Sent: Thursday, January 09, 2014 5:55 PM
To: intel-gfx@lists.freedesktop.org
Cc: Goel, Akash
Subject: [PATCH] drm/i915/vlv: Added write-enable pte bit support

From: Akash Goel 

This adds support for using the write-enable bit in the GTT entry for VLV.
This is handled via a read-only flag in the GEM buffer object which is then 
used to check if the write-enable bit has to be set or not when writing the GTT 
entries.
Currently by default only the Batch buffer & Ring buffers are being marked as 
read only.

Signed-off-by: Akash Goel 
---
 drivers/gpu/drm/i915/i915_drv.h| 10 ++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 
 drivers/gpu/drm/i915/i915_gem_gtt.c| 45 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c|  3 ++
 4 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h 
index cc8afff..a3ab8a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -620,7 +620,8 @@ struct i915_address_space {
void (*insert_entries)(struct i915_address_space *vm,
   struct sg_table *st,
   unsigned int first_entry,
-  enum i915_cache_level cache_level);
+  enum i915_cache_level cache_level,
+  bool gt_ro);
void (*cleanup)(struct i915_address_space *vm);  };
 
@@ -1671,6 +1672,13 @@ struct drm_i915_gem_object {
unsigned int pin_display:1;
 
/*
+* Is the object to be mapped as read-only to the GPU
+* Only honoured if hardware has relevant pte bit
+*/
+   unsigned long gt_ro:1;
+   unsigned long gt_old_ro:1;
+
+   /*
 * Is the GPU currently using a fence to access this buffer,
 */
unsigned int pending_fenced_gpu_access:1; diff --git 
a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index bbff8f9..3a15aec 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -164,6 +164,14 @@ eb_lookup_vmas(struct eb_vmas *eb,
list_add_tail(&vma->exec_list, &eb->vmas);
list_del_init(&obj->obj_exec_link);
 
+   /*
+* Currently mark each buffer as r/w by default.
+* If we are changing gt_ro, we need to make sure that it
+* gets re-mapped on gtt to update the entries.
+*/
+   obj->gt_old_ro = obj->gt_ro;
+   obj->gt_ro = 0;
+
vma->exec_entry = &exec[i];
if (eb->and < 0) {
eb->lut[i] = vma;
@@ -1153,6 +1161,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
/* take note of the batch buffer before we might reorder the lists */
batch_obj = list_entry(eb->vmas.prev, struct i915_vma, exec_list)->obj;
 
+   /* Mark exec buffers as read-only from GPU side by default */
+   batch_obj->gt_ro = 1;
+
/* Move the objects en-masse into the GTT, evicting if necessary. */
need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, &need_relocs); diff 
--git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 998f9a0..d87add4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -290,7 +290,8 @@ static void gen8_ppgtt_clear_range(struct 
i915_address_space *vm,  static void gen8_ppgtt_insert_entries(struct 
i915_address_space *vm,
  struct sg_table *pages,
  unsigned first_entry,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ bool unused)
 {
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base); @@ -792,11 
+793,13 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,  
static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
  struct sg_table *pages,
  unsigned first_entry,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ bool gt_ro)
 {
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
gen6_gtt_pte_t *pt_vaddr;
+   gen6_gtt_pte_t pte;
unsigned act_pt = first_entry / I915_PPGTT_PT_ENTRIES;
unsigned act_pte = first_entry % I915_PPGTT_PT_ENTRIES;
struct sg_

Re: [Intel-gfx] [PATCH] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact

2014-02-06 Thread Tvrtko Ursulin


On 02/05/2014 05:51 PM, Daniel Vetter wrote:

On Wed, Feb 05, 2014 at 05:33:06PM +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

This adds a small benchmark for the new userptr functionality.

Apart from basic surface creation and destruction, also tested is the
impact of having userptr surfaces in the process address space. Reason
for that is the impact of MMU notifiers on common address space
operations like munmap() which is per process.

v2:
   * Moved to benchmarks.


I'd just keep it as an igt testcase, beating on the kernel a bit can't
hurt. And we have piles of other benchmark-like testcase already around.


Are you sure? Ben suggested to move it there and I actually agree it 
makes more sense since it is mostly testing indirect effects on 
(seemingly) unrelated operations. Not to mention benchmark directory 
already exists and it is rather empty compared to tests...


Tvrtko

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


Re: [Intel-gfx] [PATCH 2/7] drm/i915: factor out valleyview_pipestat_irq_handler

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 09:14:09AM +0200, Jani Nikula wrote:
> On Wed, 05 Feb 2014, Daniel Vetter  wrote:
> > On Tue, Feb 04, 2014 at 09:35:46PM +0200, Imre Deak wrote:
> >> This will be used by other platforms too, so factor it out.
> >> 
> >> The only functional change is the reordeing of gmbus_irq_handler() wrt.
> >> the hotplug handling, but since it only schedules a work, it isn't an
> >> issue.
> >> 
> >> Signed-off-by: Imre Deak 
> >> ---
> >>  drivers/gpu/drm/i915/i915_irq.c | 76 
> >> +++--
> >>  1 file changed, 42 insertions(+), 34 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> >> b/drivers/gpu/drm/i915/i915_irq.c
> >> index 137ac65..b5524ea 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -1477,15 +1477,53 @@ static void gen6_rps_irq_handler(struct 
> >> drm_i915_private *dev_priv, u32 pm_iir)
> >>}
> >>  }
> >>  
> >> +static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 
> >> iir)
> >> +{
> >> +  drm_i915_private_t *dev_priv = dev->dev_private;
> >
> > typedefs for structs are frowned upon. I've fixed this while applying.
> 
> sed -i -e 's/drm_i915_private_t/struct drm_i915_private/' *.[ch]
> 
> plus a little hand-editing and be done with it...?

Split as per-file patches I guess we could do. A quick grep indicates that
we've fought down this typedef a lot already, most of the hits are in
areas no one touches anyway.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 3/5] drm/i915: Make sprite updates atomic

2014-02-06 Thread Daniel Vetter
On Thu, Feb 06, 2014 at 11:25:17AM +0200, Ville Syrjälä wrote:
> On Mon, Jan 27, 2014 at 05:03:39PM +0100, Daniel Vetter wrote:
> > On Mon, Jan 27, 2014 at 01:06:33PM +0200, Ville Syrjälä wrote:
> > > On Sun, Jan 26, 2014 at 06:29:06PM +0100, Daniel Vetter wrote:
> > > > On Tue, Jan 21, 2014 at 04:12:38PM +0200, ville.syrj...@linux.intel.com 
> > > > wrote:
> > > > > From: Ville Syrjälä 
> > > > >
> > > > > Add a mechanism by which we can evade the leading edge of vblank. This
> > > > > guarantees that no two sprite register writes will straddle on either
> > > > > side of the vblank start, and that means all the writes will be 
> > > > > latched
> > > > > together in one atomic operation.
> > > > >
> > > > > We do the vblank evade by checking the scanline counter, and if it's 
> > > > > too
> > > > > close to the start of vblank (too close has been hardcoded to 100usec
> > > > > for now), we will wait for the vblank start to pass. In order to
> > > > > eliminate random delayes from the rest of the system, we operate with
> > > > > interrupts disabled, except when waiting for the vblank obviously.
> > > > >
> > > > > Note that we now go digging through pipe_to_crtc_mapping[] in the
> > > > > vblank interrupt handler, which is a bit dangerous since we set up
> > > > > interrupts before the crtcs. However in this case since it's the 
> > > > > vblank
> > > > > interrupt, we don't actually unmask it until some piece of code
> > > > > requests it.
> > > > >
> > > > > v2: preempt_check_resched() calls after local_irq_enable() (Jesse)
> > > > > Hook up the vblank irq stuff on BDW as well
> > > > > v3: Pass intel_crtc instead of drm_crtc (Daniel)
> > > > > Warn if crtc.mutex isn't locked (Daniel)
> > > > > Add an explicit compiler barrier and document the barriers 
> > > > > (Daniel)
> > > > > Note the irq vs. modeset setup madness in the commit message 
> > > > > (Daniel)
> > > > >
> > > > > Reviewed-by: Jesse Barnes 
> > > > > Signed-off-by: Ville Syrjälä 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_irq.c  |  29 --
> > > > >  drivers/gpu/drm/i915/intel_display.c |   2 +
> > > > >  drivers/gpu/drm/i915/intel_drv.h |   3 +
> > > > >  drivers/gpu/drm/i915/intel_sprite.c  | 103 
> > > > > +++
> > > > >  4 files changed, 133 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > > > > b/drivers/gpu/drm/i915/i915_irq.c
> > > > > index ffb56a9..8ef9edb 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > > @@ -1439,6 +1439,15 @@ static void gen6_rps_irq_handler(struct 
> > > > > drm_i915_private *dev_priv, u32 pm_iir)
> > > > >   }
> > > > >  }
> > > > >  
> > > > > +static void intel_pipe_handle_vblank(struct drm_device *dev, enum 
> > > > > pipe pipe)
> > > > > +{
> > > > > + struct intel_crtc *intel_crtc =
> > > > > + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> > > > > +
> > > > > + intel_crtc->vbl_received = true;
> > > > > + wake_up(&intel_crtc->vbl_wait);
> > > > > +}
> > > > > +
> > > > >  static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> > > > >  {
> > > > >   struct drm_device *dev = (struct drm_device *) arg;
> > > > > @@ -1479,8 +1488,10 @@ static irqreturn_t valleyview_irq_handler(int 
> > > > > irq, void *arg)
> > > > >   spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> > > > >  
> > > > >   for_each_pipe(pipe) {
> > > > > - if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
> > > > > + if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS) {
> > > > >   drm_handle_vblank(dev, pipe);
> > > > > + intel_pipe_handle_vblank(dev, pipe);
> > > > > + }
> > > > >  
> > > > >   if (pipe_stats[pipe] & PLANE_FLIPDONE_INT_STATUS_VLV) {
> > > > >   intel_prepare_page_flip(dev, pipe);
> > > > > @@ -1679,8 +1690,10 @@ static void ilk_display_irq_handler(struct 
> > > > > drm_device *dev, u32 de_iir)
> > > > >   DRM_ERROR("Poison interrupt\n");
> > > > >  
> > > > >   for_each_pipe(pipe) {
> > > > > - if (de_iir & DE_PIPE_VBLANK(pipe))
> > > > > + if (de_iir & DE_PIPE_VBLANK(pipe)) {
> > > > >   drm_handle_vblank(dev, pipe);
> > > > > + intel_pipe_handle_vblank(dev, pipe);
> > > > > + }
> > > > >  
> > > > >   if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
> > > > >   if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> > > > > @@ -1729,8 +1742,10 @@ static void ivb_display_irq_handler(struct 
> > > > > drm_device *dev, u32 de_iir)
> > > > >   intel_opregion_asle_intr(dev);
> > > > >  
> > > > >   for_each_pipe(i) {
> > > > > - if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
> > > > > + if (de_iir & (DE_PIPE_VBLANK_IVB(i))) {
> > > > >   drm_handle_vblank(dev, i);
> > > > > + intel_pipe_handle_vblank(dev, i);
> > > > > + }
> > > > >  
> > > > >   /* plane/pipes map 1:1 on ilk+ */
> > > > >   if (de_iir & DE_PLANE_FLIP_DONE_IVB(i)) {
> > > > > @@ -1872,8 +1887,10 @@ static irqreturn_t gen8_irq_handler(int irq, 
> 

Re: [Intel-gfx] [PATCH v3 3/5] drm/i915: Make sprite updates atomic

2014-02-06 Thread Ville Syrjälä
On Mon, Jan 27, 2014 at 05:03:39PM +0100, Daniel Vetter wrote:
> On Mon, Jan 27, 2014 at 01:06:33PM +0200, Ville Syrjälä wrote:
> > On Sun, Jan 26, 2014 at 06:29:06PM +0100, Daniel Vetter wrote:
> > > On Tue, Jan 21, 2014 at 04:12:38PM +0200, ville.syrj...@linux.intel.com 
> > > wrote:
> > > > From: Ville Syrjälä 
> > > >
> > > > Add a mechanism by which we can evade the leading edge of vblank. This
> > > > guarantees that no two sprite register writes will straddle on either
> > > > side of the vblank start, and that means all the writes will be latched
> > > > together in one atomic operation.
> > > >
> > > > We do the vblank evade by checking the scanline counter, and if it's too
> > > > close to the start of vblank (too close has been hardcoded to 100usec
> > > > for now), we will wait for the vblank start to pass. In order to
> > > > eliminate random delayes from the rest of the system, we operate with
> > > > interrupts disabled, except when waiting for the vblank obviously.
> > > >
> > > > Note that we now go digging through pipe_to_crtc_mapping[] in the
> > > > vblank interrupt handler, which is a bit dangerous since we set up
> > > > interrupts before the crtcs. However in this case since it's the vblank
> > > > interrupt, we don't actually unmask it until some piece of code
> > > > requests it.
> > > >
> > > > v2: preempt_check_resched() calls after local_irq_enable() (Jesse)
> > > > Hook up the vblank irq stuff on BDW as well
> > > > v3: Pass intel_crtc instead of drm_crtc (Daniel)
> > > > Warn if crtc.mutex isn't locked (Daniel)
> > > > Add an explicit compiler barrier and document the barriers (Daniel)
> > > > Note the irq vs. modeset setup madness in the commit message 
> > > > (Daniel)
> > > >
> > > > Reviewed-by: Jesse Barnes 
> > > > Signed-off-by: Ville Syrjälä 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_irq.c  |  29 --
> > > >  drivers/gpu/drm/i915/intel_display.c |   2 +
> > > >  drivers/gpu/drm/i915/intel_drv.h |   3 +
> > > >  drivers/gpu/drm/i915/intel_sprite.c  | 103 
> > > > +++
> > > >  4 files changed, 133 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > > > b/drivers/gpu/drm/i915/i915_irq.c
> > > > index ffb56a9..8ef9edb 100644
> > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > @@ -1439,6 +1439,15 @@ static void gen6_rps_irq_handler(struct 
> > > > drm_i915_private *dev_priv, u32 pm_iir)
> > > >   }
> > > >  }
> > > >  
> > > > +static void intel_pipe_handle_vblank(struct drm_device *dev, enum pipe 
> > > > pipe)
> > > > +{
> > > > + struct intel_crtc *intel_crtc =
> > > > + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> > > > +
> > > > + intel_crtc->vbl_received = true;
> > > > + wake_up(&intel_crtc->vbl_wait);
> > > > +}
> > > > +
> > > >  static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> > > >  {
> > > >   struct drm_device *dev = (struct drm_device *) arg;
> > > > @@ -1479,8 +1488,10 @@ static irqreturn_t valleyview_irq_handler(int 
> > > > irq, void *arg)
> > > >   spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> > > >  
> > > >   for_each_pipe(pipe) {
> > > > - if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
> > > > + if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS) {
> > > >   drm_handle_vblank(dev, pipe);
> > > > + intel_pipe_handle_vblank(dev, pipe);
> > > > + }
> > > >  
> > > >   if (pipe_stats[pipe] & PLANE_FLIPDONE_INT_STATUS_VLV) {
> > > >   intel_prepare_page_flip(dev, pipe);
> > > > @@ -1679,8 +1690,10 @@ static void ilk_display_irq_handler(struct 
> > > > drm_device *dev, u32 de_iir)
> > > >   DRM_ERROR("Poison interrupt\n");
> > > >  
> > > >   for_each_pipe(pipe) {
> > > > - if (de_iir & DE_PIPE_VBLANK(pipe))
> > > > + if (de_iir & DE_PIPE_VBLANK(pipe)) {
> > > >   drm_handle_vblank(dev, pipe);
> > > > + intel_pipe_handle_vblank(dev, pipe);
> > > > + }
> > > >  
> > > >   if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
> > > >   if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> > > > @@ -1729,8 +1742,10 @@ static void ivb_display_irq_handler(struct 
> > > > drm_device *dev, u32 de_iir)
> > > >   intel_opregion_asle_intr(dev);
> > > >  
> > > >   for_each_pipe(i) {
> > > > - if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
> > > > + if (de_iir & (DE_PIPE_VBLANK_IVB(i))) {
> > > >   drm_handle_vblank(dev, i);
> > > > + intel_pipe_handle_vblank(dev, i);
> > > > + }
> > > >  
> > > >   /* plane/pipes map 1:1 on ilk+ */
> > > >   if (de_iir & DE_PLANE_FLIP_DONE_IVB(i)) {
> > > > @@ -1872,8 +1887,10 @@ static irqreturn_t gen8_irq_handler(int irq, 
> > > > void *arg)
> > > >   continue;
> > > >  
> > > >   pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
> > > > - if (pipe_iir & GEN8_PIPE_VBLANK)
> > > > + if (pipe_iir & GEN8_PIPE_VBLANK) {
> > > >   drm_handle_vblank(dev, pipe);
> > > > + intel_pipe_handle_vblank(dev, pipe);
> > > > + }
> > > >  
> > > >   if (pip