Re: [Intel-gfx] [PATCH 00/11]: Color manager framework for I915 driver

[Matt Roper]

On Fri, Jul 25, 2014 at 10:06:27AM +0530, Sharma, Shashank wrote:
> Thanks for your time, and review comments Matt.
> 
> I appreciate the suggestions by you, Daniel.
> But I need a few more details, and I have a few concerns with the
> design you suggest, please find my comments inline.
> 
> Regards
> Shashank
> On 7/25/2014 6:13 AM, Matt Roper wrote:
> >On Thu, Jul 24, 2014 at 04:08:41AM +, Sharma, Shashank wrote:
> >>Hi Daniel,
> >>Thanks for your time and comments.
> >>
> >>The current design is exactly same as we discussed previously in the mail 
> >>chain, color manager is just the framework which does this job:
> >>1.  Create a DRM property for requesting object.
> >>2.   Attach the DRM property with the object.
> >
> >I didn't see Daniel's response when I sent my other message, but I had a
> >lot of the same thoughts that he brought up.  I think my previous email
> >explains one of the concerns here --- properties don't hold values, so
> >you only need to create a property once total (well, technically once
> >per DRM device), not once per object.
> >
> >Once you stop creating duplicate properties, you don't really need the
> >color manager framework at all.  Just find an appropriate driver init
> >function and create each property once, storing the property pointer
> >somewhere in dev_priv (or, if these properties can become cross-driver
> >standard properties, they'd be created once by the DRM core and stored
> >in drm_device).
> >
> Matt, do you suggest to create one DRM property named CSC for all
> pipes ? And one drm property named Gamma for all pipes ?

Yep, that's what I meant.

> Can you please elaborate a bit more in this part: "Create a DRM
> property and attach to ech CRTC, managing their own values."
> 
> In this design the current design, I have few concerns here, on your
> suggestion:
> 1. If I enable gamma correction on one pipe (pipe A, driving DSI
> display) but don't apply gamma correction on other (pipe B, driving
> HDMI), how to maintain the state of each, without adding additional
> complexity in the driver. I have to create some additional data
> structure and attach to dev_priv.

The property stuff can be pretty confusing at first glance since it
doesn't quite work like a lot of people intuitively expect it to ---
when you set a property value, that value gets stored with the object
itself, not with the property.  I think it's actually a little bit
easier to understand if you dig through the DRM object data structures.
Note that drm_crtc/drm_plane/drm_connector are all derived from
drm_mode_object (using C-style inheritance).  And drm_mode_object
contains a field

struct drm_object_properties *properties;

The definition of drm_object_properties is:

struct drm_object_properties {
int count;
uint32_t ids[DRM_OBJECT_MAX_PROPERTY];
uint64_t values[DRM_OBJECT_MAX_PROPERTY];
};

so you can see that each object (crtc, plane, connector) has a table of
property ID's and values inside of it.

When you call one of the drm_property_create functions, the important
thing that happens is an ID number gets assigned to the new property you
created.  Then you can go attach that single property to as many other
objects (planes, crtc's, connectors) as you want.  The
drm_object_attach_property() function just adds a new entry to the ids[]
and values[] arrays shown above for the specific object.

So for example, let's assume that you modified your patch to only create
a single gamma property, as we described.  And for the purposes of this
example, let's pretend that when your code actually runs, gamma happens
to get assigned an ID number 7 by the DRM core code.  You would then
proceed to attach that gamma property to both of your BayTrail CRTC's.
After doing so, there would be some values i and j such that
crtc1->base.properties->ids[i] == 7
crtc2->base.properties->ids[j] == 7
In this case, your two gamma values for the two CRTC's would be stored
internally in
crtc1->base.properties->values[i]
and
crtc2->base.properties->values[j]
and can be updated independently.

(Note that there are some patches on the dri-devel mailing list from Rob
Clark that refactor some of the internal structures described above in
preparation for atomic modeset, but the general idea remains the same.)

> 
> 2. The previously applied values are to be stored somewhere, to be
> re-stored in case of suspend/resume.

Right, the values that you apply get stored in each CRTC itself as

crtc->base.properties->values[i]

If you need to get the value back again later in your driver code, you
can retrieve the value for a property associated with a specific CRTC
with the drm_object_property_get_value() function.

> 
> Plus, If I create a core DRM property for each of the color
> corrections, not all HWs running DRM driver will have properties
> like CSC, Gamma, Hue and Saturation, Contrast, Brightness. 

Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load & thaw

[Ben Widawsky]

On Wed, Jul 16, 2014 at 04:05:59PM +0100, alistair.mcau...@intel.com wrote:
> From: "McAulay, Alistair" 
> 
> This patch is to address Daniels concerns over different code during reset:
> 
> http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
> 
> "The reason for aiming as hard as possible to use the exact same code for
> driver load, gpu reset and runtime pm/system resume is that we've simply
> seen too many bugs due to slight variations and unintended omissions."
> 
> Tested using igt drv_hangman.
> 
> Signed-off-by: McAulay, Alistair 

2 quick comments before I actually do a real review.
1. Did you actually run this with and without full ppgtt?
2. I don't think this is actually fulfilling what Daniel is requesting,
though we can let him comment.
3. Did you reall do #1?

Assuming you satisifed #1, can you please post the igt results for the
permutations (pre patch w/ and w/o ppgtt; post patch w/ and w/o ppgtt)

I really want this data because I spent *a lot* of time with these
specific areas in the PPGTT work, and I am somewhat skeptical enough of
the code has changed that this will magically work. I also tried the
trickiness with the ring handling functions, and never succeeded. Also,
with the context stuff, I'm simply not convinced it can magically
vanish. If igt looks good, and Daniel agrees that this is what he
actually wanted, I will go fishing for corner cases and do the review.

Thanks.

> ---
>  drivers/gpu/drm/i915/i915_gem.c |  2 -
>  drivers/gpu/drm/i915/i915_gem_context.c | 42 +
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 67 
> +
>  drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +-
>  5 files changed, 14 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ef047bc..b38e086 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2590,8 +2590,6 @@ void i915_gem_reset(struct drm_device *dev)
>   for_each_ring(ring, dev_priv, i)
>   i915_gem_reset_ring_cleanup(dev_priv, ring);
>  
> - i915_gem_context_reset(dev);
> -
>   i915_gem_restore_fences(dev);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index de72a28..d96219f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -372,42 +372,6 @@ err_destroy:
>   return ERR_PTR(ret);
>  }
>  
> -void i915_gem_context_reset(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - int i;
> -
> - /* Prevent the hardware from restoring the last context (which hung) on
> -  * the next switch */
> - for (i = 0; i < I915_NUM_RINGS; i++) {
> - struct intel_engine_cs *ring = &dev_priv->ring[i];
> - struct intel_context *dctx = ring->default_context;
> - struct intel_context *lctx = ring->last_context;
> -
> - /* Do a fake switch to the default context */
> - if (lctx == dctx)
> - continue;
> -
> - if (!lctx)
> - continue;
> -
> - if (dctx->legacy_hw_ctx.rcs_state && i == RCS) {
> - 
> WARN_ON(i915_gem_obj_ggtt_pin(dctx->legacy_hw_ctx.rcs_state,
> -   
> get_context_alignment(dev), 0));
> - /* Fake a finish/inactive */
> - dctx->legacy_hw_ctx.rcs_state->base.write_domain = 0;
> - dctx->legacy_hw_ctx.rcs_state->active = 0;
> - }
> -
> - if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
> - 
> i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state);
> -
> - i915_gem_context_unreference(lctx);
> - i915_gem_context_reference(dctx);
> - ring->last_context = dctx;
> - }
> -}
> -
>  int i915_gem_context_init(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -498,10 +462,6 @@ int i915_gem_context_enable(struct drm_i915_private 
> *dev_priv)
>   ppgtt->enable(ppgtt);
>   }
>  
> - /* FIXME: We should make this work, even in reset */
> - if (i915_reset_in_progress(&dev_priv->gpu_error))
> - return 0;
> -
>   BUG_ON(!dev_priv->ring[RCS].default_context);
>  
>   for_each_ring(ring, dev_priv, i) {
> @@ -645,7 +605,7 @@ static int do_switch(struct intel_engine_cs *ring,
>   from = ring->last_context;
>  
>   if (USES_FULL_PPGTT(ring->dev)) {
> - ret = ppgtt->switch_mm(ppgtt, ring, false);
> + ret = ppgtt->switch_mm(ppgtt, ring);
>   if (ret)
>   goto unpin_out;
>   }
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index

Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: BDW Software Turbo

[Sun, Daisy]

we have reconsidered good suggestions and evaluated performance and complexity 
again.

Timer Constant callback would continuously wake up CPU and entire
package, results in lower CPU and package C-state and shorter battery life,
especially for standby time.

execbuf is a good one, and we had taken it into account too. execbuf
can happen much more frequent than flips. Synchronization and calculation
overhead were the main reasons that we tried to avoid using too much IA
resource to benefit GT.

Here's is a revised version of software turbo for BDW, please take a
look and see if there's any concern.

For software turbo, it can be tough to find out a perfect solution
, may need some trade-off.

Revised design:
GT busyness will still be calculated when page_flip comes in, then GT frequency
will be adjusted accordingly. This point stays the same as previous design.
For the cases no flip will happen(server or background task with no display 
activity)
which is a previous concern,  set GT frequency to RP0(no turbo algorithm 
interfered in this
case).

Implementation details:
1)  Driver start with RP0 as GT frequency.
2)  When the flip comes, do the regular software turbo busyness calculation. Also set a timer with 250ms;  
3)  If the flip keep coming in time, keep turbo algorithm, reset timer;

4)  When the timer is fired, set RP frequency to RP0 so that the background 
task will still be taken
care of(the RPS boost and idle need to be disabled in this situation).
5)If the flip comes again, go to 2).

To recap,
For most common cases, GT will run at a desired frequency as a
result of software turbo algorithm;
For background workloads or no flip environment, GT will be running at RP0 with
shorter execution time to extend RC6 and pkg C state residency as long as power
is concerned.

I'll start with the implementation if all concerns are ironed out.

- Daisy


On 7/25/2014 12:22 AM, Daniel Vetter wrote:

On Thu, Jul 24, 2014 at 01:28:21PM -0700, Jesse Barnes wrote:

If that won't work, you could just use a timer, or tie into some other
event that happens when the GPU is busy (e.g. execbuf or retire) instead
of trying to tie into the display side of things.

Yes, tying into a normal timer is probably best. At least I get the
impression that we only need something regular. Of course once the gpu is
idle we need to stop rearming that timer and restart it upon first batch
when transitioning out of idle.
-Daniel


Jesse

On Tue, 15 Jul 2014 06:35:20 +
"Sun, Daisy"  wrote:


Hi Daniel, Chris

The concern for traditional X and media server do make sense. I'll update the 
patch with RP_UP_EI_INTERRUPT as trigger instead of the page flip.
Thanks for the valuable input.

- Daisy

-Original Message-
From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of 
Daniel Vetter
Sent: Monday, July 14, 2014 12:04 AM
To: Sun, Daisy
Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: BDW Software Turbo

On Mon, Jul 14, 2014 at 8:59 AM, Daniel Vetter  wrote:

On Mon, Jul 14, 2014 at 04:22:44AM +, Sun, Daisy wrote:

3) The function will be called when flip happened, this should cover
most of the cases. One exception is background media process without
any display output, it's relatively rare.  Please let me know if you
have concern on other cases, I will try to cover it definitely.

Traditional X never flips. And we kinda have to keep this working.
Instead of checking when flipping we need to check at regular time
intervals I guess, for as long as the gt is busy.

Oh and transcode servers are a real thing apparently. They also never flip, and 
we actually care from a business pov ...
-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



--
Jesse Barnes, Intel Open Source Technology Center


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

Re: [Intel-gfx] drm/i915: CONFIG_DRM_I915_UMS

[Daniel Vetter]

On Fri, Jul 25, 2014 at 2:14 PM, Paul Bolle  wrote:
> Your commit 2225a28fd916 ("drm/i915: Ditch UMS config option") is
> included in today's linux-next (ie, next-20140725). It removes the
> Kconfig symbol DRM_I915_UMS.
>
> It didn't remove the two (negative) checks for CONFIG_DRM_I915_UMS.
> These checks are superfluous as they now will always evaluate to true.
> Is the trivial cleanup to remove them already queued somewhere?

No, and intentionally. Actually removing the code for
user-mode-setting isn't just removing these two blocks, but requires
the gutting of roughly 10k lines splattered all over the driver.
Essentially all the code that checks for
!drm_core_check_feature(DRIVER_MODESET) needs to go. That's not quite
as trivial, and before I do that I want to make really sure that
really no one misses this option.

So probably after 3.17 is out the door for a bit.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Kill intel_crtc->vbl_wait

[Daniel Vetter]

On Thu, May 22, 2014 at 07:00:50PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Share the waitqueue that drm_irq uses when performing the vblank evade
> trick for atomic pipe updates.
> 
> v2: Keep intel_pipe_handle_vblank() (Chris)
> 
> Suggested-by: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 

Both patches merged to dinq, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c  | 5 -
>  drivers/gpu/drm/i915/intel_display.c | 2 --
>  drivers/gpu/drm/i915/intel_drv.h | 2 --
>  drivers/gpu/drm/i915/intel_sprite.c  | 5 +++--
>  4 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 304f86a..bc4cdbd 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1726,14 +1726,9 @@ static void gen6_rps_irq_handler(struct 
> drm_i915_private *dev_priv, u32 pm_iir)
>  
>  static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
>  {
> - struct intel_crtc *crtc;
> -
>   if (!drm_handle_vblank(dev, pipe))
>   return false;
>  
> - crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> - wake_up(&crtc->vbl_wait);
> -
>   return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 019e9e1..aab638c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10966,8 +10966,6 @@ static void intel_crtc_init(struct drm_device *dev, 
> int pipe)
>   intel_crtc->plane = !pipe;
>   }
>  
> - init_waitqueue_head(&intel_crtc->vbl_wait);
> -
>   BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>   dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index d7c52b2..0071138 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -408,8 +408,6 @@ struct intel_crtc {
>   struct intel_pipe_wm active;
>   } wm;
>  
> - wait_queue_head_t vbl_wait;
> -
>   int scanline_offset;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index d6acd6b..7cd6a4f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -53,6 +53,7 @@ static bool intel_pipe_update_start(struct intel_crtc 
> *crtc, uint32_t *start_vbl
>   enum pipe pipe = crtc->pipe;
>   long timeout = msecs_to_jiffies_timeout(1);
>   int scanline, min, max, vblank_start;
> + wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base);
>   DEFINE_WAIT(wait);
>  
>   WARN_ON(!mutex_is_locked(&crtc->base.mutex));
> @@ -81,7 +82,7 @@ static bool intel_pipe_update_start(struct intel_crtc 
> *crtc, uint32_t *start_vbl
>* other CPUs can see the task state update by the time we
>* read the scanline.
>*/
> - prepare_to_wait(&crtc->vbl_wait, &wait, TASK_UNINTERRUPTIBLE);
> + prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE);
>  
>   scanline = intel_get_crtc_scanline(crtc);
>   if (scanline < min || scanline > max)
> @@ -100,7 +101,7 @@ static bool intel_pipe_update_start(struct intel_crtc 
> *crtc, uint32_t *start_vbl
>   local_irq_disable();
>   }
>  
> - finish_wait(&crtc->vbl_wait, &wait);
> + finish_wait(wq, &wait);
>  
>   drm_vblank_put(dev, pipe);
>  
> -- 
> 1.8.5.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] Updated drm-intel-testing

[Daniel Vetter]

Hi all,

New -testing cycle with cool stuff:
- Ditch UMS support (well just the config option for now)
- Prep work for future platforms (Sonika Jindal, Damien)
- runtime pm/soix fixes (Paulo, Jesse)
- psr tracking improvements, locking fixes, now enabled by default!
- rps fixes for chv (Deepak, Ville)
- drm core patches for rotation support (Ville, Sagar Kamble) - the i915 parts
  unfortunately didn't make it yet
- userptr fixes (Chris)
- minimum backlight brightness (Jani), acked long ago by Matthew Garret on irc -
  I've forgotten about this patch :(

Happy testing!

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

Re: [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

[Konrad Rzeszutek Wilk]

On Thu, Jul 24, 2014 at 09:44:41AM +0800, Chen, Tiejun wrote:
> On 2014/7/24 4:54, Konrad Rzeszutek Wilk wrote:
> >On Sat, Jul 19, 2014 at 12:27:21AM +, Kay, Allen M wrote:
> >>>For the MCH PCI registers that do need to be read - can you tell us which 
> >>>ones those are?
> >>
> >>In qemu/hw/xen_pt_igd.c/igd_pci_read(), following MCH PCI config register 
> >>reads are passthrough to the host HW.   Some of the registers are needed by 
> >>Ironlake GFX driver which we probably can remove.  I did a trace recently 
> >>on Broadwell,  the number of register accessed are even smaller (0, 2, 2c, 
> >>2e, 50, 52, a0, a4).  Given that we now have integrated MCH and GPU in the 
> >>same socket, looks like driver can easily remove reads for offsets 0 - 0x2e.
> >>
> >>case 0x00:/* vendor id */
> >>case 0x02:/* device id */
> >>case 0x08:/* revision id */
> >>case 0x2c:/* sybsystem vendor id */
> >>case 0x2e:/* sybsystem id */
> >
> >Right. We can fix the i915 to use the mechanism that Michael mentioned.
> >(see attached RFC patches)
> >
> >>case 0x50:/* SNB: processor graphics control register */
> >>case 0x52:/* processor graphics control register */
> >>case 0xa0:/* top of memory */
> >>case 0xb0:/* ILK: BSM: should read from dev 2 offset 
> >> 0x5c */
> >>case 0x58:/* SNB: PAVPC Offset */
> >>case 0xa4:/* SNB: graphics base of stolen memory */
> >>case 0xa8:/* SNB: base of GTT stolen memory */
> >
> >I dug in the intel-gtt.c (see ironlake_gtt_driver) to figure out this
> >a bit more. As in, I speculated, what if we returned 0 (and not implement
> >any support for reading from these registers). What would happen?
> >
> >0x52 for Ironlake (g5):
> >--
> >It looks like intel_gmch_probe is called when i915_gem_gtt_init
> >starts (there is a lot of code that looks to be used between
> >intel-gtt.c and i915.c).
> >
> >Anyhow the interesting parts are that i9xx_setup ends up calling
> >ioremap the i915 BAR to setup some of these registers for older generations.
> >
> >Then i965_gtt_total_entries gets which reads 0x52, but it is only
> >needed for v5 generation. For other (v4 and G33) it reads it from the GPU's
> >0x2020  register.
> >
> >If there is a mismatch, it writes to the GPU at 0x2020 to update the
> >the size based on the bridge. And then it reads from 0x2020 and that
> >is returned and stuck in  intel_private.gtt_total_entries.
> >
> >So 0x52 in the emulated bridge could be populated with what the
> >GPU has at 0x2020. And the writes go in the emulated area.
> >
> >0x52 for v6 -> v8:
> >-
> >We seem to go to gen6_gmch_probe which just figures out the
> >the GTT based on the GPU's BAR sizes. The stolen values
> >are read from 0x50 from the GPU. So no need to touch the bridge
> >(see gen6_gmch_probe)
> >
> >OK, so no need to have 0x52 or 0x50 in the bridge.
> >
> >0xA0:
> >-
> >Could not find any reference in the Linux code. Why would
> >Windows driver need this? If we returned the _real_ TOM would
> >it matter? Is it used to figure out the device should use 32-bit
> >DMA operations or 40-bit?
> >
> >0xb0 or 0x5c:
> >-
> >No mention of them in the Linux code.
> >
> >0x58, 0xa4, 0xa8:
> >-
> >No usage of them in the Linux code. We seem to be using the 0x52
> >from the bridge and 0x2020 from the GPU for this functionality.
> >
> >
> >So in theory*, if using Ironlake we need to have a proper value
> >in 0x52 in the bridge. But for the later chipsets we do not need
> >these values (I am assuming that intel_setup_mchbar can safely
> >return as it does that for Ironlake and could very well for later
> >generations).
> >
> >Can this be reflected in the Windows driver as well?
> >
> >P.S.
> >*theory: That is assuming we modify the Linux i915_drv.c:intel_detect_pch
> >to pick up the id as suggested earlier. See the RFC patches attached.
> >(Not compile tested at all!)
> 
> I take a look these patches, looks we still scan all PCI Bridge to walk all
> PCHs. So this means we still need to fake a PCI bridge, right? Or maybe you
> don't cover this problem this time.

I totally forgot. Feel free to fix that.
> 
> I prefer we should check dev slot to get that PCH like my previous patch,

Uh? Your patch was checking for 0:1f.0, not the slot of the device.

(see https://lkml.org/lkml/2014/6/19/121)
> "gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class
> type". Because Windows always use this way, so I think this point should be
> same between Linux and Windows.

Didn't we discuss that we did not want to pass in PCH at all if we can?

And from the observation I made above it seems that we can safely do it
under Linux. With Windows I don't know - but I presume the answer is yes too.


> 
> Or we need anthe

Re: [Intel-gfx] [PATCH i-g-t] lib: avoid getopt value conflicts with tests

[Paulo Zanoni]

2014-07-25 13:08 GMT-03:00 Thomas Wood :
> Most tests use a printable character as the value for getopt to return,
> so avoid conflicts by using non-printing values for the standard options.

Instead of this patch, isn't there any way to verify if the tests are
using any character that is "reserved" to these general options?
Having "-r" instead of "--run-subtest" is quite nice. That said, I'm
not against your patch either.

>
> Signed-off-by: Thomas Wood 
> ---
>  lib/igt_core.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index a0c9499..882614a 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -218,6 +218,13 @@ int num_test_children;
>  int test_children_sz;
>  bool test_child;
>
> +enum {
> + OPT_LIST_SUBTESTS,
> + OPT_RUN_SUBTEST,
> + OPT_DEBUG,
> + OPT_HELP
> +};
> +
>  __attribute__((format(printf, 1, 2)))
>  static void kmsg(const char *format, ...)
>  #define KERN_INFO "<5>"
> @@ -320,10 +327,10 @@ static int common_init(int argc, char **argv,
>  {
> int c, option_index = 0;
> static struct option long_options[] = {
> -   {"list-subtests", 0, 0, 'l'},
> -   {"run-subtest", 1, 0, 'r'},
> -   {"debug", 0, 0, 'd'},
> -   {"help", 0, 0, 'h'},
> +   {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
> +   {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> +   {"debug", 0, 0, OPT_DEBUG},
> +   {"help", 0, 0, OPT_HELP},
> };
> char *short_opts;
> struct option *combined_opts;
> @@ -370,18 +377,18 @@ static int common_init(int argc, char **argv,
> while ((c = getopt_long(argc, argv, short_opts, combined_opts,
>&option_index)) != -1) {
> switch(c) {
> -   case 'd':
> +   case OPT_DEBUG:
> igt_log_level = IGT_LOG_DEBUG;
> break;
> -   case 'l':
> +   case OPT_LIST_SUBTESTS:
> if (!run_single_subtest)
> list_subtests = true;
> break;
> -   case 'r':
> +   case OPT_RUN_SUBTEST:
> if (!list_subtests)
> run_single_subtest = strdup(optarg);
> break;
> -   case 'h':
> +   case OPT_HELP:
> print_usage(help_str, false);
> ret = -1;
> goto out;
> --
> 1.9.3
>
> ___
> 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

[Intel-gfx] [PATCH i-g-t] lib: avoid getopt value conflicts with tests

[Thomas Wood]

Most tests use a printable character as the value for getopt to return,
so avoid conflicts by using non-printing values for the standard options.

Signed-off-by: Thomas Wood 
---
 lib/igt_core.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index a0c9499..882614a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -218,6 +218,13 @@ int num_test_children;
 int test_children_sz;
 bool test_child;
 
+enum {
+ OPT_LIST_SUBTESTS,
+ OPT_RUN_SUBTEST,
+ OPT_DEBUG,
+ OPT_HELP
+};
+
 __attribute__((format(printf, 1, 2)))
 static void kmsg(const char *format, ...)
 #define KERN_INFO "<5>"
@@ -320,10 +327,10 @@ static int common_init(int argc, char **argv,
 {
int c, option_index = 0;
static struct option long_options[] = {
-   {"list-subtests", 0, 0, 'l'},
-   {"run-subtest", 1, 0, 'r'},
-   {"debug", 0, 0, 'd'},
-   {"help", 0, 0, 'h'},
+   {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
+   {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
+   {"debug", 0, 0, OPT_DEBUG},
+   {"help", 0, 0, OPT_HELP},
};
char *short_opts;
struct option *combined_opts;
@@ -370,18 +377,18 @@ static int common_init(int argc, char **argv,
while ((c = getopt_long(argc, argv, short_opts, combined_opts,
   &option_index)) != -1) {
switch(c) {
-   case 'd':
+   case OPT_DEBUG:
igt_log_level = IGT_LOG_DEBUG;
break;
-   case 'l':
+   case OPT_LIST_SUBTESTS:
if (!run_single_subtest)
list_subtests = true;
break;
-   case 'r':
+   case OPT_RUN_SUBTEST:
if (!list_subtests)
run_single_subtest = strdup(optarg);
break;
-   case 'h':
+   case OPT_HELP:
print_usage(help_str, false);
ret = -1;
goto out;
-- 
1.9.3

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

Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: BDW Software Turbo

[Sun, Daisy]

Yes, timer can be helpful. A revised proposal is that flip trigger + 
timer  to cover together. I'll come up with more details soon.


- Daisy

On 7/25/2014 12:22 AM, Daniel Vetter wrote:

On Thu, Jul 24, 2014 at 01:28:21PM -0700, Jesse Barnes wrote:

If that won't work, you could just use a timer, or tie into some other
event that happens when the GPU is busy (e.g. execbuf or retire) instead
of trying to tie into the display side of things.

Yes, tying into a normal timer is probably best. At least I get the
impression that we only need something regular. Of course once the gpu is
idle we need to stop rearming that timer and restart it upon first batch
when transitioning out of idle.
-Daniel


Jesse

On Tue, 15 Jul 2014 06:35:20 +
"Sun, Daisy"  wrote:


Hi Daniel, Chris

The concern for traditional X and media server do make sense. I'll update the 
patch with RP_UP_EI_INTERRUPT as trigger instead of the page flip.
Thanks for the valuable input.

- Daisy

-Original Message-
From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of 
Daniel Vetter
Sent: Monday, July 14, 2014 12:04 AM
To: Sun, Daisy
Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: BDW Software Turbo

On Mon, Jul 14, 2014 at 8:59 AM, Daniel Vetter  wrote:

On Mon, Jul 14, 2014 at 04:22:44AM +, Sun, Daisy wrote:

3) The function will be called when flip happened, this should cover
most of the cases. One exception is background media process without
any display output, it's relatively rare.  Please let me know if you
have concern on other cases, I will try to cover it definitely.

Traditional X never flips. And we kinda have to keep this working.
Instead of checking when flipping we need to check at regular time
intervals I guess, for as long as the gt is busy.

Oh and transcode servers are a real thing apparently. They also never flip, and 
we actually care from a business pov ...
-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



--
Jesse Barnes, Intel Open Source Technology Center


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

[Intel-gfx] [PATCH i-g-t] testdisplay: set a non-zero exit code if getopt detected an error

[Thomas Wood]

Signed-off-by: Thomas Wood 
---
 tests/testdisplay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index f60e66d..20b0615 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -670,7 +670,7 @@ static void __attribute__((noreturn)) usage(char *name)
igt_info("\t\t,,,\n");
igt_info("\t\ttest force mode\n");
igt_info("\tDefault is to test all modes.\n");
-   exit(0);
+   exit((optopt) ? -1 : 0);
 }
 
 #define dump_resource(res) if (res) dump_##res()
-- 
1.9.3

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

Re: [Intel-gfx] [PATCH v2] intel-gpu-tools: fix version.h creation in android

[Thomas Wood]

On 25 July 2014 12:33, Thomas Wood  wrote:
> On 24 July 2014 17:38,   wrote:
>> From: Tim Gore 
>>
>> commit 743dc7997aa9f5210055896940d87c88983dcda6
>> breaks the build under Android because version.h
>> is not created. This happens because the android
>> make executes from the ANDROID_BUILD_TOP directory
>> rather than from the directory containing the source
>> files, so we need to differentiate between Android
>> and linux builds. This is V2 of this patch based on
>> Thomas Wood's suggestion.
>>
>> Signed-off-by: Tim Gore 
>> ---
>>  lib/Android.mk   |  3 ++-
>>  lib/Makefile.am  |  3 +++
>>  lib/Makefile.sources | 20 +++-
>>  3 files changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/Android.mk b/lib/Android.mk
>> index 6f444a0..5739c80 100644
>> --- a/lib/Android.mk
>> +++ b/lib/Android.mk
>> @@ -1,6 +1,7 @@
>>  LOCAL_PATH := $(call my-dir)
>>
>>  GPU_TOOLS_PATH := $(LOCAL_PATH)/..
>> +IGT_LIB_PATH := $(LOCAL_PATH)
>>
>>  # FIXME: autogenerate this info #
>>  $(GPU_TOOLS_PATH)/config.h:
>> @@ -13,7 +14,7 @@ include $(LOCAL_PATH)/Makefile.sources
>>  include $(CLEAR_VARS)
>>
>>  LOCAL_GENERATED_SOURCES :=   \
>> -   $(GPU_TOOLS_PATH)/lib/version.h  \
>> +   $(IGT_LIB_PATH)/version.h  \
>> $(GPU_TOOLS_PATH)/config.h
>>
>>  LOCAL_C_INCLUDES +=  \
>> diff --git a/lib/Makefile.am b/lib/Makefile.am
>> index 4d4efe4..9f6a021 100644
>> --- a/lib/Makefile.am
>> +++ b/lib/Makefile.am
>> @@ -1,3 +1,6 @@
>> +IGT_LIB_PATH := .
>> +GPU_TOOLS_PATH := ..
>> +
>
> Automake supports out-of-tree builds and this is tested during
> distcheck, so relative paths don't work. The above two lines need to
> be:
>
> IGT_LIB_PATH := $(top_builddir)/lib

This can also just be $(builddir). I've pushed the patch with these
changes and also pushed another small distcheck fix for version.h used
from quick_dump.


> GPU_TOOLS_PATH := $(top_srcdir)
>
>
>
>>  include Makefile.sources
>>
>>  noinst_LTLIBRARIES = libintel_tools.la
>> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
>> index 2d971c5..96786e0 100644
>> --- a/lib/Makefile.sources
>> +++ b/lib/Makefile.sources
>> @@ -48,12 +48,14 @@ libintel_tools_la_SOURCES = \
>> $(NULL)
>>
>>  .PHONY: version.h.tmp
>> -version.h.tmp:
>> +
>> +$(IGT_LIB_PATH)/version.h.tmp:
>> @touch $@
>> -   @if test -d $(top_srcdir)/.git; then \
>> -   if which git > /dev/null 2>&1; then git log -n 1 --oneline | 
>> \
>> +   @if test -d $(GPU_TOOLS_PATH)/.git; then \
>> +   if which git > /dev/null 2>&1; then cd $(@D); \
>> +   git log -n 1 --oneline | \
>> sed 's/^\([^ ]*\) .*/#define IGT_GIT_SHA1 "g\1"/' \
>> -   >> $@ ; \
>> +   >> $(@F) ; \
>> else \
>> echo '#define IGT_GIT_SHA1 "NO-GIT"' >> $@ ; \
>> fi \
>> @@ -61,12 +63,12 @@ version.h.tmp:
>> echo '#define IGT_GIT_SHA1 "NOT-GIT"' >> $@ ; \
>> fi
>>
>> -version.h: version.h.tmp
>> -   @if ! cmp -s version.h.tmp version.h; then \
>> -   echo "updating version.h"; \
>> -   mv version.h.tmp version.h ;\
>> +
>> +$(IGT_LIB_PATH)/version.h: $(IGT_LIB_PATH)/version.h.tmp
>> +   @if ! cmp -s $(IGT_LIB_PATH)/version.h.tmp 
>> $(IGT_LIB_PATH)/version.h; then \
>> +   mv $(IGT_LIB_PATH)/version.h.tmp $(IGT_LIB_PATH)/version.h ; 
>> \
>> else \
>> -   rm version.h.tmp ;\
>> +   rm $(IGT_LIB_PATH)/version.h.tmp ; \
>> fi
>>
>>  BUILT_SOURCES = version.h
>
> This now needs to match the rule above (i.e. $(IGT_LIB_PATH)/version.h).
>
>> --
>> 1.9.2
>>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 22/40] drm/i915: Add chv port D TX wells

[Imre Deak]

On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Add the TX wells for port D. The Punit subsystem numbers are a total
> guess at this time. Also I'm not sure these even exist. Certainly the
> Punit in current hardware doesn't deal with these.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  4 
>  drivers/gpu/drm/i915/intel_pm.c | 23 +++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3d1fef4..191df9e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -525,6 +525,10 @@ enum punit_power_well {
>   PUNIT_POWER_WELL_DPIO_RX0   = 10,
>   PUNIT_POWER_WELL_DPIO_RX1   = 11,
>   PUNIT_POWER_WELL_DPIO_CMN_D = 12,
> + /* FIXME: guesswork below */
> + PUNIT_POWER_WELL_DPIO_TX_D_LANES_01 = 13,
> + PUNIT_POWER_WELL_DPIO_TX_D_LANES_23 = 14,
> + PUNIT_POWER_WELL_DPIO_RX2   = 15,
>  
>   PUNIT_POWER_WELL_NUM,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index cae936c..55f3e6b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6540,6 +6540,15 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
>   BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
>   BIT(POWER_DOMAIN_INIT))
>  
> +#define CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS (   \
> + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |  \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
> + BIT(POWER_DOMAIN_INIT))
> +
> +#define CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS (   \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \

Atm, for all other ports we power up all lanes regardless of the actual
configuration (until the PHY side setup is proved to work fine). So for
consistency I'd do the same here too. With that change:

Reviewed-by: Imre Deak 

> + BIT(POWER_DOMAIN_INIT))
> +
>  static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>   .sync_hw = i9xx_always_on_power_well_noop,
>   .enable = i9xx_always_on_power_well_noop,
> @@ -6757,6 +6766,20 @@ static struct i915_power_well chv_power_wells[] = {
>   .ops = &vlv_dpio_power_well_ops,
>   .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
>   },
> + {
> + .name = "dpio-tx-d-01",
> + .domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
> +CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
> + .ops = &vlv_dpio_power_well_ops,
> + .data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_01,
> + },
> + {
> + .name = "dpio-tx-d-23",
> + .domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
> +CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
> + .ops = &vlv_dpio_power_well_ops,
> + .data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_23,
> + },
>  #endif
>  };
>  



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

Re: [Intel-gfx] [PATCH 21/40] drm/i915: Add chv port B and C TX wells

[Imre Deak]

On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Add the TX wells for ports B and C just like on VLV.
> 
> Again Punit doesn't seem ready (or the wells don't even exist anymore)
> so leave it iffed out.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index de5416b..cae936c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6728,6 +6728,36 @@ static struct i915_power_well chv_power_wells[] = {
>   .data = PUNIT_POWER_WELL_DPIO_CMN_D,
>   .ops = &chv_dpio_cmn_power_well_ops,
>   },
> +#if 0
> + {
> + .name = "dpio-tx-b-01",
> + .domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS |
> +VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS,
> + .ops = &vlv_dpio_power_well_ops,
> + .data = PUNIT_POWER_WELL_DPIO_TX_B_LANES_01,
> + },
> + {
> + .name = "dpio-tx-b-23",
> + .domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS |
> +VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS,
> + .ops = &vlv_dpio_power_well_ops,
> + .data = PUNIT_POWER_WELL_DPIO_TX_B_LANES_23,
> + },
> + {
> + .name = "dpio-tx-c-01",
> + .domains = VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS |
> +VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS,
> + .ops = &vlv_dpio_power_well_ops,
> + .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_01,
> + },
> + {
> + .name = "dpio-tx-c-23",
> + .domains = VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS |
> +VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS,
> + .ops = &vlv_dpio_power_well_ops,
> + .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
> + },
> +#endif
>  };
>  
>  static struct i915_power_well *lookup_power_well(struct drm_i915_private 
> *dev_priv,



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

Re: [Intel-gfx] [PATCH 20/40] drm/i915: Add per-pipe power wells for chv

[Imre Deak]

On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> CHV has a power well for each pipe. Add the code to deal with them.
> 
> The Punit in current hardware doesn't seem ready for this yet, so
> leave it iffed out.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/i915_reg.h |  12 
>  drivers/gpu/drm/i915/intel_pm.c | 126 
> 
>  2 files changed, 138 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 19e68d6..3d1fef4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -499,6 +499,18 @@
>  #define   DSPFREQSTAT_MASK   (0x3 << DSPFREQSTAT_SHIFT)
>  #define   DSPFREQGUAR_SHIFT  14
>  #define   DSPFREQGUAR_MASK   (0x3 << DSPFREQGUAR_SHIFT)
> +#define   _DP_SSC(val, pipe) ((val) << (2 * (pipe)))
> +#define   DP_SSC_MASK(pipe)  _DP_SSC(0x3, (pipe))
> +#define   DP_SSC_PWR_ON(pipe)_DP_SSC(0x0, (pipe))
> +#define   DP_SSC_CLK_GATE(pipe)  _DP_SSC(0x1, (pipe))
> +#define   DP_SSC_RESET(pipe) _DP_SSC(0x2, (pipe))
> +#define   DP_SSC_PWR_GATE(pipe)  _DP_SSC(0x3, (pipe))
> +#define   _DP_SSS(val, pipe) ((val) << (2 * (pipe) + 16))
> +#define   DP_SSS_MASK(pipe)  _DP_SSS(0x3, (pipe))
> +#define   DP_SSS_PWR_ON(pipe)_DP_SSS(0x0, (pipe))
> +#define   DP_SSS_CLK_GATE(pipe)  _DP_SSS(0x1, (pipe))
> +#define   DP_SSS_RESET(pipe) _DP_SSS(0x2, (pipe))
> +#define   DP_SSS_PWR_GATE(pipe)  _DP_SSS(0x3, (pipe))
>  
>  /* See the PUNIT HAS v0.8 for the below bits */
>  enum punit_power_well {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 46394fc..de5416b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6258,6 +6258,95 @@ static void chv_dpio_cmn_power_well_disable(struct 
> drm_i915_private *dev_priv,
>   vlv_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static bool chv_pipe_power_well_enabled(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + enum pipe pipe = power_well->data;
> + bool enabled;
> + u32 state, ctrl;
> +
> + mutex_lock(&dev_priv->rps.hw_lock);
> +
> + state = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & DP_SSS_MASK(pipe);
> + /*
> +  * We only ever set the power-on and power-gate states, anything
> +  * else is unexpected.
> +  */
> + WARN_ON(state != DP_SSS_PWR_ON(pipe) && state != DP_SSS_PWR_GATE(pipe));
> + enabled = state == DP_SSS_PWR_ON(pipe);
> +
> + /*
> +  * A transient state at this point would mean some unexpected party
> +  * is poking at the power controls too.
> +  */
> + ctrl = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & DP_SSC_MASK(pipe);
> + WARN_ON(ctrl << 16 != state);
> +
> + mutex_unlock(&dev_priv->rps.hw_lock);
> +
> + return enabled;
> +}
> +
> +static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well,
> + bool enable)
> +{
> + enum pipe pipe = power_well->data;
> + u32 state;
> + u32 ctrl;
> +
> + state = enable ? DP_SSS_PWR_ON(pipe) : DP_SSS_PWR_GATE(pipe);
> +
> + mutex_lock(&dev_priv->rps.hw_lock);
> +
> +#define COND \
> + ((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & DP_SSS_MASK(pipe)) == 
> state)
> +
> + if (COND)
> + goto out;
> +
> + ctrl = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> + ctrl &= ~DP_SSC_MASK(pipe);
> + ctrl |= enable ? DP_SSC_PWR_ON(pipe) : DP_SSC_PWR_GATE(pipe);
> + vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, ctrl);
> +
> + if (wait_for(COND, 100))
> + DRM_ERROR("timout setting power well state %08x (%08x)\n",
> +   state,
> +   vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ));
> +
> +#undef COND
> +
> +out:
> + mutex_unlock(&dev_priv->rps.hw_lock);
> +}
> +
> +static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + chv_set_pipe_power_well(dev_priv, power_well, power_well->count > 0);
> +}
> +
> +static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
> +struct i915_power_well *power_well)
> +{
> + WARN_ON_ONCE(power_well->data != PIPE_A &&
> +  power_well->data != PIPE_B &&
> +  power_well->data != PIPE_C);
> +
> + chv_set_pipe_power_well(dev_priv, power_well, true);
> +}
> +
> +static void chv_pipe_power_w

Re: [Intel-gfx] [PATCH 19/40] drm/i915: Add disp2d power well for chv

[Imre Deak]

On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Not sure if it's still there since chv has per-pipe power wells.
> At least with current Punit this doesn't work. Also the display
> irq handling would need to be adjusted for pipe C. So leave the
> code iffed out for now.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f88490b..46394fc 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6582,6 +6582,14 @@ static struct i915_power_well chv_power_wells[] = {
>   .domains = VLV_ALWAYS_ON_POWER_DOMAINS,
>   .ops = &i9xx_always_on_power_well_ops,
>   },
> +#if 0
> + {
> + .name = "display",
> + .domains = VLV_DISPLAY_POWER_DOMAINS,
> + .data = PUNIT_POWER_WELL_DISP2D,
> + .ops = &vlv_display_power_well_ops,
> + },
> +#endif
>   {
>   .name = "dpio-common-bc",
>   .domains = CHV_DPIO_CMN_BC_POWER_DOMAINS,



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

[Intel-gfx] [PATCH] drm/i915: s/seqno/request/ tracking inside objects

[Chris Wilson]

At the heart of this change is that the seqno is a too low level of an
abstraction to handle the growing complexities of command tracking, both
with the introduction of multiple command queues with execbuffer and the
potential for reordering with a scheduler. On top of the seqno we have
the request. Conceptually this is just a fence, but it also has
substantial bookkeeping of its own in order to track the context and
batch in flight, for example. It is the central structure upon which we
can extend with dependency tracking et al.

As regards the objects, they were using the seqno as a simple fence,
upon which is check or even wait upon for command completion. This patch
exchanges that seqno/ring pair with the request itself. For the
majority, lifetime of the request is ordered by how we retire objects
then requests. However, both the unlocked waits and probing elsewhere do
not tie into the normal request lifetimes and so we need to introduce a
kref. Extending the objects to use the request as the fence naturally
extends to segregrating read/write fence tracking. This has significance
for it reduces the number of semaphores we need to emit, reducing the
likelihood of #54226, and improving performance overall.

NOTE: this is not against bare drm-intel-nightly and is likely to
conflict with execlists...

Signed-off-by: Chris Wilson 
Cc: Jesse Barnes 
Cc: Daniel Vetter 
Cc: Oscar Mateo 
Cc: Brad Volkin 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  37 +-
 drivers/gpu/drm/i915/i915_drv.h  | 108 ++--
 drivers/gpu/drm/i915/i915_gem.c  | 769 ---
 drivers/gpu/drm/i915/i915_gem_context.c  |  19 +-
 drivers/gpu/drm/i915/i915_gem_exec.c |  10 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |  37 +-
 drivers/gpu/drm/i915/i915_gem_render_state.c |   5 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c   |   2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c|  35 +-
 drivers/gpu/drm/i915/i915_irq.c  |   6 +-
 drivers/gpu/drm/i915/i915_perf.c |   6 +-
 drivers/gpu/drm/i915/i915_trace.h|   2 +-
 drivers/gpu/drm/i915/intel_display.c |  50 +-
 drivers/gpu/drm/i915/intel_drv.h |   3 +-
 drivers/gpu/drm/i915/intel_overlay.c | 118 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c  |  83 +--
 drivers/gpu/drm/i915/intel_ringbuffer.h  |  11 +-
 17 files changed, 745 insertions(+), 556 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 406e630..676d5f1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -122,10 +122,11 @@ static inline const char *get_global_flag(struct 
drm_i915_gem_object *obj)
 static void
 describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 {
+   struct i915_gem_request *rq = i915_gem_object_last_read(obj);
struct i915_vma *vma;
int pin_count = 0;
 
-   seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %u %u %u%s%s%s",
+   seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %x %x %x%s%s%s",
   &obj->base,
   get_pin_flag(obj),
   get_tiling_flag(obj),
@@ -133,9 +134,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object 
*obj)
   obj->base.size / 1024,
   obj->base.read_domains,
   obj->base.write_domain,
-  obj->last_read_seqno,
-  obj->last_write_seqno,
-  obj->last_fenced_seqno,
+  i915_request_seqno(rq),
+  i915_request_seqno(obj->last_write.request),
+  i915_request_seqno(obj->last_fence.request),
   i915_cache_level_str(obj->cache_level),
   obj->dirty ? " dirty" : "",
   obj->madv == I915_MADV_DONTNEED ? " purgeable" : "");
@@ -168,8 +169,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object 
*obj)
*t = '\0';
seq_printf(m, " (%s mappable)", s);
}
-   if (obj->ring != NULL)
-   seq_printf(m, " (%s)", obj->ring->name);
+   if (rq)
+   seq_printf(m, " (%s)", rq->ring->name);
if (obj->frontbuffer_bits)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
 }
@@ -336,7 +337,7 @@ static int per_file_stats(int id, void *ptr, void *data)
if (ppgtt->ctx && ppgtt->ctx->file_priv != 
stats->file_priv)
continue;
 
-   if (obj->ring) /* XXX per-vma statistic */
+   if (obj->active) /* XXX per-vma statistic */
stats->active += obj->base.size;
else
stats->inactive += obj->base.size;
@@ -346,7 +347,7 @@ static int per_file_stats(int id, void *ptr, void *data)
} else {
if (i915_gem_obj_ggtt_bound(ob

[Intel-gfx] [PATCH] drm/i915: s/seqno/request/ tracking inside objects

[Chris Wilson]

At the heart of this change is that the seqno is a too low level of an
abstraction to handle the growing complexities of command tracking, both
with the introduction of multiple command queues with execbuffer and the
potential for reordering with a scheduler. On top of the seqno we have
the request. Conceptually this is just a fence, but it also has
substantial bookkeeping of its own in order to track the context and
batch in flight, for example. It is the central structure upon which we
can extend with dependency tracking et al.

As regards the objects, they were using the seqno as a simple fence,
upon which is check or even wait upon for command completion. This patch
exchanges that seqno/ring pair with the request itself. For the
majority, lifetime of the request is ordered by how we retire objects
then requests. However, both the unlocked waits and probing elsewhere do
not tie into the normal request lifetimes and so we need to introduce a
kref. Extending the objects to use the request as the fence naturally
extends to segregrating read/write fence tracking. This has significance
for it reduces the number of semaphores we need to emit, reducing the
likelihood of #54226, and improving performance overall.

NOTE: this is not against bare drm-intel-nightly and is likely to
conflict with execlists...

Signed-off-by: Chris Wilson 
Cc: Jesse Barnes 
Cc: Daniel Vetter 
Cc: Oscar Mateo 
Cc: Brad Volkin 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  37 +-
 drivers/gpu/drm/i915/i915_drv.h  | 108 ++--
 drivers/gpu/drm/i915/i915_gem.c  | 769 ---
 drivers/gpu/drm/i915/i915_gem_context.c  |  19 +-
 drivers/gpu/drm/i915/i915_gem_exec.c |  10 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |  37 +-
 drivers/gpu/drm/i915/i915_gem_render_state.c |   5 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c   |   2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c|  35 +-
 drivers/gpu/drm/i915/i915_irq.c  |   6 +-
 drivers/gpu/drm/i915/i915_perf.c |   6 +-
 drivers/gpu/drm/i915/i915_trace.h|   2 +-
 drivers/gpu/drm/i915/intel_display.c |  50 +-
 drivers/gpu/drm/i915/intel_drv.h |   3 +-
 drivers/gpu/drm/i915/intel_overlay.c | 118 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c  |  83 +--
 drivers/gpu/drm/i915/intel_ringbuffer.h  |  11 +-
 17 files changed, 745 insertions(+), 556 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 406e630..676d5f1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -122,10 +122,11 @@ static inline const char *get_global_flag(struct 
drm_i915_gem_object *obj)
 static void
 describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 {
+   struct i915_gem_request *rq = i915_gem_object_last_read(obj);
struct i915_vma *vma;
int pin_count = 0;
 
-   seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %u %u %u%s%s%s",
+   seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %x %x %x%s%s%s",
   &obj->base,
   get_pin_flag(obj),
   get_tiling_flag(obj),
@@ -133,9 +134,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object 
*obj)
   obj->base.size / 1024,
   obj->base.read_domains,
   obj->base.write_domain,
-  obj->last_read_seqno,
-  obj->last_write_seqno,
-  obj->last_fenced_seqno,
+  i915_request_seqno(rq),
+  i915_request_seqno(obj->last_write.request),
+  i915_request_seqno(obj->last_fence.request),
   i915_cache_level_str(obj->cache_level),
   obj->dirty ? " dirty" : "",
   obj->madv == I915_MADV_DONTNEED ? " purgeable" : "");
@@ -168,8 +169,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object 
*obj)
*t = '\0';
seq_printf(m, " (%s mappable)", s);
}
-   if (obj->ring != NULL)
-   seq_printf(m, " (%s)", obj->ring->name);
+   if (rq)
+   seq_printf(m, " (%s)", rq->ring->name);
if (obj->frontbuffer_bits)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
 }
@@ -336,7 +337,7 @@ static int per_file_stats(int id, void *ptr, void *data)
if (ppgtt->ctx && ppgtt->ctx->file_priv != 
stats->file_priv)
continue;
 
-   if (obj->ring) /* XXX per-vma statistic */
+   if (obj->active) /* XXX per-vma statistic */
stats->active += obj->base.size;
else
stats->inactive += obj->base.size;
@@ -346,7 +347,7 @@ static int per_file_stats(int id, void *ptr, void *data)
} else {
if (i915_gem_obj_ggtt_bound(ob

[Intel-gfx] drm/i915: CONFIG_DRM_I915_UMS

[Paul Bolle]

Daniel,

Your commit 2225a28fd916 ("drm/i915: Ditch UMS config option") is
included in today's linux-next (ie, next-20140725). It removes the
Kconfig symbol DRM_I915_UMS.

It didn't remove the two (negative) checks for CONFIG_DRM_I915_UMS.
These checks are superfluous as they now will always evaluate to true.
Is the trivial cleanup to remove them already queued somewhere?


Paul Bolle

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

Re: [Intel-gfx] [PATCH 18/40] drm/i915: Kill intel_reset_dpio()

[Imre Deak]

On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Both VLV and CHV handle the cmnreset stuff in the power well code now,
> so intel_reset_dpio() is no longer needed.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/intel_display.c | 31 ---
>  1 file changed, 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index a16f635..3cd73f4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1511,34 +1511,6 @@ static void intel_init_dpio(struct drm_device *dev)
>   }
>  }
>  
> -static void intel_reset_dpio(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - if (IS_CHERRYVIEW(dev)) {
> - enum dpio_phy phy;
> - u32 val;
> -
> - for (phy = DPIO_PHY0; phy < I915_NUM_PHYS_VLV; phy++) {
> - /* Poll for phypwrgood signal */
> - if (wait_for(I915_READ(DISPLAY_PHY_STATUS) &
> - PHY_POWERGOOD(phy), 1))
> - DRM_ERROR("Display PHY %d is not power up\n", 
> phy);
> -
> - /*
> -  * Deassert common lane reset for PHY.
> -  *
> -  * This should only be done on init and resume from S3
> -  * with both PLLs disabled, or we risk losing DPIO and
> -  * PLL synchronization.
> -  */
> - val = I915_READ(DISPLAY_PHY_CONTROL);
> - I915_WRITE(DISPLAY_PHY_CONTROL,
> - PHY_COM_LANE_RESET_DEASSERT(phy, val));
> - }
> - }
> -}
> -
>  static void vlv_enable_pll(struct intel_crtc *crtc)
>  {
>   struct drm_device *dev = crtc->base.dev;
> @@ -12473,8 +12445,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
>  
>   intel_init_clock_gating(dev);
>  
> - intel_reset_dpio(dev);
> -
>   intel_enable_gt_powersave(dev);
>  }
>  
> @@ -12545,7 +12515,6 @@ void intel_modeset_init(struct drm_device *dev)
>   }
>  
>   intel_init_dpio(dev);
> - intel_reset_dpio(dev);
>  
>   intel_cpu_pll_init(dev);
>   intel_shared_dpll_init(dev);



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

Re: [Intel-gfx] [PATCH 17/40] drm/i915: Add chv cmnlane power wells

[Imre Deak]

On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> CHV has two display PHYs so there are also two cmnlane power wells. Add
> the approriate code to power the wells up/down.
> 
> Like on VLV we do the cmnreset assert/deassert and the DPLL refclock
> enabling at approriate times.
> 
> This code actually works on my bsw.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 89 
> +
>  2 files changed, 90 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d246609..19e68d6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -512,6 +512,7 @@ enum punit_power_well {
>   PUNIT_POWER_WELL_DPIO_TX_C_LANES_23 = 9,
>   PUNIT_POWER_WELL_DPIO_RX0   = 10,
>   PUNIT_POWER_WELL_DPIO_RX1   = 11,
> + PUNIT_POWER_WELL_DPIO_CMN_D = 12,
>  
>   PUNIT_POWER_WELL_NUM,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e2b956e..f88490b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6200,6 +6200,64 @@ static void vlv_dpio_cmn_power_well_disable(struct 
> drm_i915_private *dev_priv,
>   vlv_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> +struct i915_power_well *power_well)
> +{
> + enum dpio_phy phy;
> +
> + WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC &&
> +  power_well->data != PUNIT_POWER_WELL_DPIO_CMN_D);
> +
> + /*
> +  * Enable the CRI clock source so we can get at the
> +  * display and the reference clock for VGA
> +  * hotplug / manual detection.
> +  */
> + if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> + phy = DPIO_PHY0;
> + I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +DPLL_REFA_CLK_ENABLE_VLV);
> + I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +DPLL_REFA_CLK_ENABLE_VLV | 
> DPLL_INTEGRATED_CRI_CLK_VLV);

Any reason the two clocks are enabled sequentially? For PHY1 you don't
do this.. In any case:
Reviewed-by: Imre Deak 

> + } else {
> + phy = DPIO_PHY1;
> + I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
> +DPLL_REFA_CLK_ENABLE_VLV | 
> DPLL_INTEGRATED_CRI_CLK_VLV);
> + }
> + udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> + vlv_set_power_well(dev_priv, power_well, true);
> +
> + /* Poll for phypwrgood signal */
> + if (wait_for(I915_READ(DISPLAY_PHY_STATUS) & PHY_POWERGOOD(phy), 1))
> + DRM_ERROR("Display PHY %d is not power up\n", phy);
> +
> + I915_WRITE(DISPLAY_PHY_CONTROL,
> +PHY_COM_LANE_RESET_DEASSERT(phy, 
> I915_READ(DISPLAY_PHY_CONTROL)));
> +}
> +
> +static void chv_dpio_cmn_power_well_disable(struct drm_i915_private 
> *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + enum dpio_phy phy;
> +
> + WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC &&
> +  power_well->data != PUNIT_POWER_WELL_DPIO_CMN_D);
> +
> + if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> + phy = DPIO_PHY0;
> + assert_pll_disabled(dev_priv, PIPE_A);
> + assert_pll_disabled(dev_priv, PIPE_B);
> + } else {
> + phy = DPIO_PHY1;
> + assert_pll_disabled(dev_priv, PIPE_C);
> + }
> +
> + I915_WRITE(DISPLAY_PHY_CONTROL,
> +PHY_COM_LANE_RESET_ASSERT(phy, 
> I915_READ(DISPLAY_PHY_CONTROL)));
> +
> + vlv_set_power_well(dev_priv, power_well, false);
> +}
> +
>  static void check_power_well_state(struct drm_i915_private *dev_priv,
>  struct i915_power_well *power_well)
>  {
> @@ -6369,6 +6427,18 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
>   BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |  \
>   BIT(POWER_DOMAIN_INIT))
>  
> +#define CHV_DPIO_CMN_BC_POWER_DOMAINS (  \
> + BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |  \
> + BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |  \
> + BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |  \
> + BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |  \
> + BIT(POWER_DOMAIN_INIT))
> +
> +#define CHV_DPIO_CMN_D_POWER_DOMAINS (   \
> + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |  \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
> + BIT(POWER_DOMAIN_INIT))
> +
>  static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>   .sync_hw = i9xx_always_on_power_well_noop,
>   .enable = i9xx_always_on_power_well_noop,
> @@ -6498,6 +6568,13 @@ static struct i915_p

Re: [Intel-gfx] [PATCH v2] intel-gpu-tools: fix version.h creation in android

[Thomas Wood]

On 24 July 2014 17:38,   wrote:
> From: Tim Gore 
>
> commit 743dc7997aa9f5210055896940d87c88983dcda6
> breaks the build under Android because version.h
> is not created. This happens because the android
> make executes from the ANDROID_BUILD_TOP directory
> rather than from the directory containing the source
> files, so we need to differentiate between Android
> and linux builds. This is V2 of this patch based on
> Thomas Wood's suggestion.
>
> Signed-off-by: Tim Gore 
> ---
>  lib/Android.mk   |  3 ++-
>  lib/Makefile.am  |  3 +++
>  lib/Makefile.sources | 20 +++-
>  3 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/lib/Android.mk b/lib/Android.mk
> index 6f444a0..5739c80 100644
> --- a/lib/Android.mk
> +++ b/lib/Android.mk
> @@ -1,6 +1,7 @@
>  LOCAL_PATH := $(call my-dir)
>
>  GPU_TOOLS_PATH := $(LOCAL_PATH)/..
> +IGT_LIB_PATH := $(LOCAL_PATH)
>
>  # FIXME: autogenerate this info #
>  $(GPU_TOOLS_PATH)/config.h:
> @@ -13,7 +14,7 @@ include $(LOCAL_PATH)/Makefile.sources
>  include $(CLEAR_VARS)
>
>  LOCAL_GENERATED_SOURCES :=   \
> -   $(GPU_TOOLS_PATH)/lib/version.h  \
> +   $(IGT_LIB_PATH)/version.h  \
> $(GPU_TOOLS_PATH)/config.h
>
>  LOCAL_C_INCLUDES +=  \
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 4d4efe4..9f6a021 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -1,3 +1,6 @@
> +IGT_LIB_PATH := .
> +GPU_TOOLS_PATH := ..
> +

Automake supports out-of-tree builds and this is tested during
distcheck, so relative paths don't work. The above two lines need to
be:

IGT_LIB_PATH := $(top_builddir)/lib
GPU_TOOLS_PATH := $(top_srcdir)



>  include Makefile.sources
>
>  noinst_LTLIBRARIES = libintel_tools.la
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 2d971c5..96786e0 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -48,12 +48,14 @@ libintel_tools_la_SOURCES = \
> $(NULL)
>
>  .PHONY: version.h.tmp
> -version.h.tmp:
> +
> +$(IGT_LIB_PATH)/version.h.tmp:
> @touch $@
> -   @if test -d $(top_srcdir)/.git; then \
> -   if which git > /dev/null 2>&1; then git log -n 1 --oneline | \
> +   @if test -d $(GPU_TOOLS_PATH)/.git; then \
> +   if which git > /dev/null 2>&1; then cd $(@D); \
> +   git log -n 1 --oneline | \
> sed 's/^\([^ ]*\) .*/#define IGT_GIT_SHA1 "g\1"/' \
> -   >> $@ ; \
> +   >> $(@F) ; \
> else \
> echo '#define IGT_GIT_SHA1 "NO-GIT"' >> $@ ; \
> fi \
> @@ -61,12 +63,12 @@ version.h.tmp:
> echo '#define IGT_GIT_SHA1 "NOT-GIT"' >> $@ ; \
> fi
>
> -version.h: version.h.tmp
> -   @if ! cmp -s version.h.tmp version.h; then \
> -   echo "updating version.h"; \
> -   mv version.h.tmp version.h ;\
> +
> +$(IGT_LIB_PATH)/version.h: $(IGT_LIB_PATH)/version.h.tmp
> +   @if ! cmp -s $(IGT_LIB_PATH)/version.h.tmp $(IGT_LIB_PATH)/version.h; 
> then \
> +   mv $(IGT_LIB_PATH)/version.h.tmp $(IGT_LIB_PATH)/version.h ; \
> else \
> -   rm version.h.tmp ;\
> +   rm $(IGT_LIB_PATH)/version.h.tmp ; \
> fi
>
>  BUILT_SOURCES = version.h

This now needs to match the rule above (i.e. $(IGT_LIB_PATH)/version.h).

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

[Intel-gfx] [PATCH i-g-t] testdisplay: only set terminal attributes when in foreground process group

[Thomas Wood]

The Piglit test runner for intel-gpu-tools creates a new process group
for the test processes, so attempting to set terminal attributes causes
the process to receive SIGTTOU and be stopped. Since the test is not run
interactively in this case, the issue can be avoided by not setting
terminal attributes if the process is not in the foreground process
group.

Signed-off-by: Thomas Wood 
---
 tests/testdisplay.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index 6d8fe3a..f60e66d 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -725,6 +725,11 @@ static void set_termio_mode(void)
 {
struct termios tio;
 
+   /* don't attempt to set terminal attributes if not in the foreground
+* process group */
+   if (getpgrp() != tcgetpgrp(STDOUT_FILENO))
+   return;
+
tio_fd = dup(STDIN_FILENO);
tcgetattr(tio_fd, &saved_tio);
igt_install_exit_handler(restore_termio_mode);
-- 
1.9.3

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

Re: [Intel-gfx] [PATCH] drm: Fix race when checking for fb in the generic kms obj lookup

[Rob Clark]

On Thu, Jul 24, 2014 at 6:12 AM, Daniel Vetter  wrote:
> In my review of
>
> commit 98f75de40e9d83c3a90d294b8fd25fa2874212a9
> Author: Rob Clark 
> Date:   Fri May 30 11:37:03 2014 -0400
>
> drm: add object property typ
>
> I asked for a check to make sure that we never leak an fb from the
> generic mode object lookup since those have completely different
> lifetime rules. Rob added it, but outside of the idr mutex, which
> means that our dereference of obj->type can already chase free'd
> memory.
>
> Somehow I didn't spot this, so fix this asap.
>
> v2: Simplify the conditionals as suggested by Chris.
>
> Cc: Rob Clark 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Rob Clark 

> ---
>  drivers/gpu/drm/drm_crtc.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0a47907..d87df8836aa5 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -426,8 +426,12 @@ static struct drm_mode_object *_object_find(struct 
> drm_device *dev,
>
> mutex_lock(&dev->mode_config.idr_mutex);
> obj = idr_find(&dev->mode_config.crtc_idr, id);
> -   if (!obj || (type != DRM_MODE_OBJECT_ANY && obj->type != type) ||
> -   (obj->id != id))
> +   if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type)
> +   obj = NULL;
> +   if (obj && obj->id != id)
> +   obj = NULL;
> +   /* don't leak out unref'd fb's */
> +   if (obj && (obj->type == DRM_MODE_OBJECT_FB))
> obj = NULL;
> mutex_unlock(&dev->mode_config.idr_mutex);
>
> @@ -454,9 +458,6 @@ struct drm_mode_object *drm_mode_object_find(struct 
> drm_device *dev,
>  * function.*/
> WARN_ON(type == DRM_MODE_OBJECT_FB);
> obj = _object_find(dev, id, type);
> -   /* don't leak out unref'd fb's */
> -   if (obj && (obj->type == DRM_MODE_OBJECT_FB))
> -   obj = NULL;
> return obj;
>  }
>  EXPORT_SYMBOL(drm_mode_object_find);
> --
> 2.0.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [BUG?] 3.16-rc6 ... at drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()

[Ian Kumlien]

On fre, 2014-07-25 at 12:28 +0300, Imre Deak wrote:
> On Thu, 2014-07-24 at 01:33 +0200, Ian Kumlien wrote:
> > Try four, now including CC lists for the intel driver...
> 
> Could you give a try to the 2 patches at:
> https://patchwork.kernel.org/patch/4437061/
> 
> If these don't fix the issue, could you send a full dmesg log with the
> drm.debug=14 kernel option set?

I will, but the tests will be a bit delayed (earliest tomorrow evening)

> Thanks,
> Imre
> 
> > 
> > ---
> > 
> > Hi again,
> > 
> > 
> > Playing some more with the new kernel release i noticed this:
> > [ 9064.008821] WARNING: CPU: 3 PID: 22929 at 
> > drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()
> > [ 9064.008822] Modules linked in: uas bnep b43 mac80211 cfg80211 
> > snd_hda_codec_hdmi btusb bluetooth 6lowpan_iphc rfkill snd_hda_codec_cirrus 
> > uvcvideo snd_hda_codec_generic videobuf2_vmalloc videobuf2_memops 
> > videobuf2_core snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep 
> > snd_pcm snd_timer sdhci_pci snd sdhci soundcore mmc_core bcma
> > [ 9064.008839] CPU: 3 PID: 22929 Comm: kworker/3:2 Tainted: GW 
> > 3.16.0-rc6 #23
> > [ 9064.008840] Hardware name: Apple Inc. 
> > MacBookPro10,2/Mac-AFD8A9D944EA4843, BIOS MBP102.88Z.0106.B03.1211161133 
> > 11/16/2012
> > [ 9064.008843] Workqueue: events edp_panel_vdd_work
> > [ 9064.008844]  0009 88015ba77d28 8198ea2d 
> > 
> > [ 9064.008846]  88015ba77d60 810cbac8 8802610b002c 
> > 000c7204
> > [ 9064.008848]  0001 8802610b80f0 8802610b 
> > 88015ba77d70
> > [ 9064.008850] Call Trace:
> > [ 9064.008854]  [] dump_stack+0x4e/0x7a
> > [ 9064.008857]  [] warn_slowpath_common+0x78/0xa0
> > [ 9064.008858]  [] warn_slowpath_null+0x15/0x20
> > [ 9064.008860]  [] intel_display_power_put+0x12d/0x160
> > [ 9064.008862]  [] edp_panel_vdd_off_sync+0xf4/0x1c0
> > [ 9064.008863]  [] edp_panel_vdd_work+0x2f/0x40
> > [ 9064.008866]  [] process_one_work+0x16e/0x480
> > [ 9064.008868]  [] worker_thread+0x11b/0x520
> > [ 9064.008870]  [] ? create_and_start_worker+0x50/0x50
> > [ 9064.008872]  [] kthread+0xc4/0xe0
> > [ 9064.008874]  [] ? kthread_create_on_node+0x170/0x170
> > [ 9064.008877]  [] ret_from_fork+0x7c/0xb0
> > [ 9064.008878]  [] ? kthread_create_on_node+0x170/0x170
> > [ 9064.008880] ---[ end trace 17f9738f20aec288 ]---
> > 
> > 
> > 
> > I had multiples of them in my dmesg, however, this seems to fix it:
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 8a1a4fb..4c3249d 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1252,6 +1252,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp 
> > *intel_dp)
> > intel_dp->last_power_cycle = jiffies;
> >  
> > power_domain = 
> > intel_display_port_power_domain(intel_encoder);
> > +   intel_display_power_get(dev_priv, power_domain);
> > intel_display_power_put(dev_priv, power_domain);
> > }
> >  }
> > @@ -1371,6 +1372,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
> >  
> > /* We got a reference when we enabled the VDD. */
> > power_domain = intel_display_port_power_domain(intel_encoder);
> > +   intel_display_power_get(dev_priv, power_domain);
> > intel_display_power_put(dev_priv, power_domain);
> >  }
> > ---
> > 
> > 
> > The question however is: Is this the correct approach? Should it be done
> > differently?
> > (This handles the "close and open lid" usecase, i don't know if there
> > are others, a grep indicated that there might be two more missing...)
> > 
> > 
> > 
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


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

Re: [Intel-gfx] [PATCH] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[Tvrtko Ursulin]

On 07/25/2014 10:37 AM, Daniel Vetter wrote:

On Fri, Jul 25, 2014 at 10:17:26AM +0100, Chris Wilson wrote:

On Fri, Jul 25, 2014 at 11:06:38AM +0200, Daniel Vetter wrote:

On Fri, Jul 25, 2014 at 10:00:19AM +0100, tim.g...@intel.com wrote:

From: Tim Gore 

gem_mmap_offset_exhaustion relies on purgeable memory
allocations getting swapped out, freeing up physical
memory for further allocations. On Android we have no
swap partition so this cannot happen and the test gets
killed by the low memory killer before mmap offset
exhaustion can happen, thus defeating the tests purpose.

Signed-off-by: Tim Gore 




/* we happily leak objects to exhaust mmap offset space, the kernel will
 * reap backing storage. */
gem_madvise(fd, handle, I915_MADV_DONTNEED);

There's really no way you should be able to run out of memory. I suspect
android kernel's will fall over even with swap.


No, it's just the android lowmemkiller hates i915 by design. The two are
more or less incompatible.


Well someone should fix up the lowmemkiller then. Disabling the test is
not really fixing it.


AFAIR lowmemorykiller is a weird thing which tries to prevent pagecache 
dropping below a configurable threshold by killing processes. If free 
pages are below a threshold _and_ page cache is below the same threshold 
it will try to kill something. I suppose this is to prevent kernel in 
purging the page cache on its own. Interactions must be quite complex 
here given that thresholds (in num pages) and oom scores triggers per 
each threshold are multiple and configurable.


I was just disabling it when running IGT (echo 0 
>/sys/module/lowmemorykiller/parameters/minfree). It is probably 
debatable whether this is OK or not, depending how you look at IGT in 
the overall pool of testing.


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

Re: [Intel-gfx] [PATCH] drm/i915: Fix crash when failing to parse MIPI VBT

[Kumar, Shobhit]

On 7/25/2014 1:20 PM, Daniel Vetter wrote:

On Fri, Jul 25, 2014 at 09:47:06AM +0200, Daniel Vetter wrote:

On Thu, Jul 24, 2014 at 03:16:12PM +0100, rafael.barba...@intel.com wrote:

From: Rafael Barbalho 

This particular nasty presented itself while trying to register the
intelfb device (intel_fbdev.c). During the process of registering the device
the driver will disable the crtc via i9xx_crtc_disable. These will
also disable the panel using the generic mipi panel functions in
dsi_mod_vbt_generic.c. The stale MIPI generic data sequence pointers would
cause a crash within those functions. However, all of this is happening
while console_lock is held from do_register_framebuffer inside fbcon.c. Which
means that you got kernel log and just the device appearing to reboot/hang for
no apparent reason.


CONFIG_I915_FBDEV=n for when the console_lock gets in the way.


The fault started from the FB_EVENT_FB_REGISTERED event using the
fb_notifier_call_chain call in fbcon.c.

Cc: Shobhit Kumar 
Signed-off-by: Rafael Barbalho 


Queued for -next, thanks for the patch.


Actually this is for fixes since 3.16 has dsi support. Also for
regressions please cite the commit that introduced the offending
behaviour. I've added that.


Also this reminds me that there is still a WARN dump in 3.16 which will 
be fixed by -

[v2] drm/i915: Add correct hw/sw config check for DSI encoder

Assuming this can go in -fixes if it okay, this is waiting for review

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

Re: [Intel-gfx] [PATCH] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[Daniel Vetter]

On Fri, Jul 25, 2014 at 09:14:51AM +, Gore, Tim wrote:
> 
> 
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Friday, July 25, 2014 10:07 AM
> > To: Gore, Tim
> > Cc: intel-gfx@lists.freedesktop.org; daniel.vet...@ffwll.ch
> > Subject: Re: [PATCH] intel-gpu-tools: skip gem_mmap_offset_exhaustion on
> > Android
> > 
> > On Fri, Jul 25, 2014 at 10:00:19AM +0100, tim.g...@intel.com wrote:
> > > From: Tim Gore 
> > >
> > > gem_mmap_offset_exhaustion relies on purgeable memory allocations
> > > getting swapped out, freeing up physical memory for further
> > > allocations. On Android we have no swap partition so this cannot
> > > happen and the test gets killed by the low memory killer before mmap
> > > offset exhaustion can happen, thus defeating the tests purpose.
> > >
> > > Signed-off-by: Tim Gore 
> > 
> > 
> > 
> > /* we happily leak objects to exhaust mmap offset space, the kernel
> > will
> >  * reap backing storage. */
> > gem_madvise(fd, handle, I915_MADV_DONTNEED);
> > 
> > There's really no way you should be able to run out of memory. I suspect
> > android kernel's will fall over even with swap.
> > -Daniel
> > 
> Well, not sure I fully understand how GEM works, but I can clearly see the 
> free memory
> Shrinking until the OOM killer steps in. Since the bo's are not destroyed, 
> surely the only
> Way for the physical memory to be reclaimed is if it gets swapped out, which 
> Android
> Wont do. Perhaps I misunderstand "purgeable". Should kswapd free such memory?

Well that's how i915 is designed. The shrinker is officially the right
interface for the core vm to tell various other pieces in the kernel when
they should tighten up. If the lowmemkiller in android is a bit too
enthusiastic about this and starts shooting down random process even
before we run out of memory for real then that's a design-screw up.

I've never looked at it, but a possible fix might be to remove the
lowmemkiller from the shrinker and instead wire it up as a true OOM
callback.

Disabling the test because memory pressure handling on android is busted
is certainly not the right fix.
-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] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[Daniel Vetter]

On Fri, Jul 25, 2014 at 10:17:26AM +0100, Chris Wilson wrote:
> On Fri, Jul 25, 2014 at 11:06:38AM +0200, Daniel Vetter wrote:
> > On Fri, Jul 25, 2014 at 10:00:19AM +0100, tim.g...@intel.com wrote:
> > > From: Tim Gore 
> > > 
> > > gem_mmap_offset_exhaustion relies on purgeable memory
> > > allocations getting swapped out, freeing up physical
> > > memory for further allocations. On Android we have no
> > > swap partition so this cannot happen and the test gets
> > > killed by the low memory killer before mmap offset
> > > exhaustion can happen, thus defeating the tests purpose.
> > > 
> > > Signed-off-by: Tim Gore 
> > 
> > 
> > 
> > /* we happily leak objects to exhaust mmap offset space, the kernel will
> >  * reap backing storage. */
> > gem_madvise(fd, handle, I915_MADV_DONTNEED);
> > 
> > There's really no way you should be able to run out of memory. I suspect
> > android kernel's will fall over even with swap.
> 
> No, it's just the android lowmemkiller hates i915 by design. The two are
> more or less incompatible.

Well someone should fix up the lowmemkiller then. Disabling the test is
not really fixing it.
-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] [BUG?] 3.16-rc6 ... at drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()

[Imre Deak]

On Thu, 2014-07-24 at 01:33 +0200, Ian Kumlien wrote:
> Try four, now including CC lists for the intel driver...

Could you give a try to the 2 patches at:
https://patchwork.kernel.org/patch/4437061/

If these don't fix the issue, could you send a full dmesg log with the
drm.debug=14 kernel option set?

Thanks,
Imre

> 
> ---
> 
> Hi again,
> 
> 
> Playing some more with the new kernel release i noticed this:
> [ 9064.008821] WARNING: CPU: 3 PID: 22929 at 
> drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()
> [ 9064.008822] Modules linked in: uas bnep b43 mac80211 cfg80211 
> snd_hda_codec_hdmi btusb bluetooth 6lowpan_iphc rfkill snd_hda_codec_cirrus 
> uvcvideo snd_hda_codec_generic videobuf2_vmalloc videobuf2_memops 
> videobuf2_core snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep 
> snd_pcm snd_timer sdhci_pci snd sdhci soundcore mmc_core bcma
> [ 9064.008839] CPU: 3 PID: 22929 Comm: kworker/3:2 Tainted: GW 
> 3.16.0-rc6 #23
> [ 9064.008840] Hardware name: Apple Inc. MacBookPro10,2/Mac-AFD8A9D944EA4843, 
> BIOS MBP102.88Z.0106.B03.1211161133 11/16/2012
> [ 9064.008843] Workqueue: events edp_panel_vdd_work
> [ 9064.008844]  0009 88015ba77d28 8198ea2d 
> 
> [ 9064.008846]  88015ba77d60 810cbac8 8802610b002c 
> 000c7204
> [ 9064.008848]  0001 8802610b80f0 8802610b 
> 88015ba77d70
> [ 9064.008850] Call Trace:
> [ 9064.008854]  [] dump_stack+0x4e/0x7a
> [ 9064.008857]  [] warn_slowpath_common+0x78/0xa0
> [ 9064.008858]  [] warn_slowpath_null+0x15/0x20
> [ 9064.008860]  [] intel_display_power_put+0x12d/0x160
> [ 9064.008862]  [] edp_panel_vdd_off_sync+0xf4/0x1c0
> [ 9064.008863]  [] edp_panel_vdd_work+0x2f/0x40
> [ 9064.008866]  [] process_one_work+0x16e/0x480
> [ 9064.008868]  [] worker_thread+0x11b/0x520
> [ 9064.008870]  [] ? create_and_start_worker+0x50/0x50
> [ 9064.008872]  [] kthread+0xc4/0xe0
> [ 9064.008874]  [] ? kthread_create_on_node+0x170/0x170
> [ 9064.008877]  [] ret_from_fork+0x7c/0xb0
> [ 9064.008878]  [] ? kthread_create_on_node+0x170/0x170
> [ 9064.008880] ---[ end trace 17f9738f20aec288 ]---
> 
> 
> 
> I had multiples of them in my dmesg, however, this seems to fix it:
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8a1a4fb..4c3249d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1252,6 +1252,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp 
> *intel_dp)
> intel_dp->last_power_cycle = jiffies;
>  
> power_domain = intel_display_port_power_domain(intel_encoder);
> +   intel_display_power_get(dev_priv, power_domain);
> intel_display_power_put(dev_priv, power_domain);
> }
>  }
> @@ -1371,6 +1372,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
>  
> /* We got a reference when we enabled the VDD. */
> power_domain = intel_display_port_power_domain(intel_encoder);
> +   intel_display_power_get(dev_priv, power_domain);
> intel_display_power_put(dev_priv, power_domain);
>  }
> ---
> 
> 
> The question however is: Is this the correct approach? Should it be done
> differently?
> (This handles the "close and open lid" usecase, i don't know if there
> are others, a grep indicated that there might be two more missing...)
> 
> 
> 
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [Intel-gfx] [PATCH] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[Chris Wilson]

On Fri, Jul 25, 2014 at 11:06:38AM +0200, Daniel Vetter wrote:
> On Fri, Jul 25, 2014 at 10:00:19AM +0100, tim.g...@intel.com wrote:
> > From: Tim Gore 
> > 
> > gem_mmap_offset_exhaustion relies on purgeable memory
> > allocations getting swapped out, freeing up physical
> > memory for further allocations. On Android we have no
> > swap partition so this cannot happen and the test gets
> > killed by the low memory killer before mmap offset
> > exhaustion can happen, thus defeating the tests purpose.
> > 
> > Signed-off-by: Tim Gore 
> 
> 
> 
>   /* we happily leak objects to exhaust mmap offset space, the kernel will
>* reap backing storage. */
>   gem_madvise(fd, handle, I915_MADV_DONTNEED);
> 
> There's really no way you should be able to run out of memory. I suspect
> android kernel's will fall over even with swap.

No, it's just the android lowmemkiller hates i915 by design. The two are
more or less incompatible.
-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 01/43] drm/i915: Reorder the actual workload submission so that args checking is done earlier

[Chris Wilson]

On Fri, Jul 25, 2014 at 10:30:09AM +0200, Daniel Vetter wrote:
> On Thu, Jul 24, 2014 at 05:04:09PM +0100, Thomas Daniel wrote:
> > From: Oscar Mateo 
> > 
> > In this patch:
> > 
> > commit 78382593e921c88371abd019aca8978db3248a8f
> > Author: Oscar Mateo 
> > Date:   Thu Jul 3 16:28:05 2014 +0100
> > 
> > drm/i915: Extract the actual workload submission mechanism from 
> > execbuffer
> > 
> > So that we isolate the legacy ringbuffer submission mechanism, which 
> > becomes
> > a good candidate to be abstracted away. This is prep-work for Execlists 
> > (which
> > will its own workload submission mechanism).
> > 
> > No functional changes.
> > 
> > I changed the order in which the args checking is done. I don't know why I 
> > did (brain
> > fade?) but itś not right. I haven't seen any ill effect from this, but the 
> > Execlists
> > version of this function will have problems if the order is not correct.
> > 
> > Signed-off-by: Oscar Mateo 
> 
> I don't think this matters - the point of no return for legacy execbuf is
> the call to ring->dispatch. After that nothing may fail any more. But as
> long as we track state correctly (e.g. if we've switched the context
> already) we'll be fine.

Right. Except that I think our tracking is buggy - or at least
insufficient to address the needs of future dispatch mechanisms. I think
that we confuse some bookkeeping that should be at the request level and
place it on the ring. At the moment, we have one request per-ring and so
it doesn't matter, but transitioning to one request per-logical-ring we
start to have issues as that state is being tracked on the wrong struct.

Anyway, that's part of the motivation to fixing up requests and making
them central to accessing the rings/dispatch (whereas at the moment they
are behind the scenes).
-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] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[Gore, Tim]

> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Friday, July 25, 2014 10:07 AM
> To: Gore, Tim
> Cc: intel-gfx@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: Re: [PATCH] intel-gpu-tools: skip gem_mmap_offset_exhaustion on
> Android
> 
> On Fri, Jul 25, 2014 at 10:00:19AM +0100, tim.g...@intel.com wrote:
> > From: Tim Gore 
> >
> > gem_mmap_offset_exhaustion relies on purgeable memory allocations
> > getting swapped out, freeing up physical memory for further
> > allocations. On Android we have no swap partition so this cannot
> > happen and the test gets killed by the low memory killer before mmap
> > offset exhaustion can happen, thus defeating the tests purpose.
> >
> > Signed-off-by: Tim Gore 
> 
> 
> 
>   /* we happily leak objects to exhaust mmap offset space, the kernel
> will
>* reap backing storage. */
>   gem_madvise(fd, handle, I915_MADV_DONTNEED);
> 
> There's really no way you should be able to run out of memory. I suspect
> android kernel's will fall over even with swap.
> -Daniel
> 
Well, not sure I fully understand how GEM works, but I can clearly see the free 
memory
Shrinking until the OOM killer steps in. Since the bo's are not destroyed, 
surely the only
Way for the physical memory to be reclaimed is if it gets swapped out, which 
Android
Wont do. Perhaps I misunderstand "purgeable". Should kswapd free such memory?
  Tim

> > ---
> >  tests/gem_mmap_offset_exhaustion.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tests/gem_mmap_offset_exhaustion.c
> > b/tests/gem_mmap_offset_exhaustion.c
> > index 914fe6e..016143d 100644
> > --- a/tests/gem_mmap_offset_exhaustion.c
> > +++ b/tests/gem_mmap_offset_exhaustion.c
> > @@ -77,6 +77,10 @@ igt_simple_main
> >  {
> > int fd, i;
> >
> > +#ifdef ANDROID
> > +   igt_skip("Test not valid on Android\n"); #endif
> > +
> > igt_skip_on_simulation();
> >
> > fd = drm_open_any();
> > --
> > 1.9.2
> >
> 
> --
> 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] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[Daniel Vetter]

On Fri, Jul 25, 2014 at 10:00:19AM +0100, tim.g...@intel.com wrote:
> From: Tim Gore 
> 
> gem_mmap_offset_exhaustion relies on purgeable memory
> allocations getting swapped out, freeing up physical
> memory for further allocations. On Android we have no
> swap partition so this cannot happen and the test gets
> killed by the low memory killer before mmap offset
> exhaustion can happen, thus defeating the tests purpose.
> 
> Signed-off-by: Tim Gore 



/* we happily leak objects to exhaust mmap offset space, the kernel will
 * reap backing storage. */
gem_madvise(fd, handle, I915_MADV_DONTNEED);

There's really no way you should be able to run out of memory. I suspect
android kernel's will fall over even with swap.
-Daniel

> ---
>  tests/gem_mmap_offset_exhaustion.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/gem_mmap_offset_exhaustion.c 
> b/tests/gem_mmap_offset_exhaustion.c
> index 914fe6e..016143d 100644
> --- a/tests/gem_mmap_offset_exhaustion.c
> +++ b/tests/gem_mmap_offset_exhaustion.c
> @@ -77,6 +77,10 @@ igt_simple_main
>  {
>   int fd, i;
>  
> +#ifdef ANDROID
> + igt_skip("Test not valid on Android\n");
> +#endif
> +
>   igt_skip_on_simulation();
>  
>   fd = drm_open_any();
> -- 
> 1.9.2
> 

-- 
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] intel-gpu-tools: skip gem_mmap_offset_exhaustion on Android

[tim . gore]

From: Tim Gore 

gem_mmap_offset_exhaustion relies on purgeable memory
allocations getting swapped out, freeing up physical
memory for further allocations. On Android we have no
swap partition so this cannot happen and the test gets
killed by the low memory killer before mmap offset
exhaustion can happen, thus defeating the tests purpose.

Signed-off-by: Tim Gore 
---
 tests/gem_mmap_offset_exhaustion.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/gem_mmap_offset_exhaustion.c 
b/tests/gem_mmap_offset_exhaustion.c
index 914fe6e..016143d 100644
--- a/tests/gem_mmap_offset_exhaustion.c
+++ b/tests/gem_mmap_offset_exhaustion.c
@@ -77,6 +77,10 @@ igt_simple_main
 {
int fd, i;
 
+#ifdef ANDROID
+   igt_skip("Test not valid on Android\n");
+#endif
+
igt_skip_on_simulation();
 
fd = drm_open_any();
-- 
1.9.2

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

Re: [Intel-gfx] [PATCH 0/4] Introduce a new create ioctl for user specified

[Daniel Vetter]

On Mon, Jul 14, 2014 at 09:53:38AM +, Gupta, Sourab wrote:
> On Sun, 2014-07-06 at 18:29 +0530, sourab gupta wrote:
> > On Fri, 2014-06-20 at 10:02 +, Gupta, Sourab wrote:
> > > From: Sourab Gupta 
> > > 
> > > This patch series introduces a new gem create ioctl for user specified
> > > placement.
> > > 
> > > Despite being a unified memory architecture (UMA) some bits of memory
> > > are more equal than others. In particular we have the thorny issue of
> > > stolen memory, memory stolen from the system by the BIOS and reserved
> > > for igfx use. Stolen memory is required for some functions of the GPU
> > > and display engine, but in general it goes wasted. Whilst we cannot
> > > return it back to the system, we need to find some other method for
> > > utilising it. As we do not support direct access to the physical address
> > > in the stolen region, it behaves like a different class of memory,
> > > closer in kin to local GPU memory. This strongly suggests that we need a
> > > placement model like TTM if we are to fully utilize these discrete
> > > chunks of differing memory.
> > > 
> > > This new create ioctl therefore exists to allow the user to create these
> > > second class buffer objects from stolen memory. At the moment direct
> > > access by the CPU through mmaps and pread/pwrite are verboten on the
> > > objects, and so the user must be aware of the limitations of the objects
> > > created. Yet, those limitations rarely reduce the desired functionality
> > > in many use cases and so the user should be able to easily fill the
> > > stolen memory and so help to reduce overall memory pressure.
> > > 
> > > The most obvious use case for stolen memory is for the creation of objects
> > > for the display engine which already have very similar restrictions on
> > > access. However, we want a reasonably general ioctl in order to cater
> > > for diverse scenarios beyond the author's imagination.
> > > 
> > > Chris Wilson (3):
> > >   drm/i915: Clearing buffer objects via blitter engine
> > >   drm/i915: Introduce a new create ioctl for user specified placement
> > >   drm/i915: Add support for stealing purgable stolen pages
> > > 
> > > Deepak S (1):
> > >   drm/i915: Clearing buffer objects via blitter engine for Gen8
> > > 
> > >  drivers/gpu/drm/i915/Makefile  |   1 +
> > >  drivers/gpu/drm/i915/i915_dma.c|   5 +-
> > >  drivers/gpu/drm/i915/i915_drv.h|  18 ++-
> > >  drivers/gpu/drm/i915/i915_gem.c| 208 
> > > ++---
> > >  drivers/gpu/drm/i915/i915_gem_exec.c   | 139 ++
> > >  drivers/gpu/drm/i915/i915_gem_stolen.c | 121 +--
> > >  drivers/gpu/drm/i915/i915_gem_tiling.c | 106 +
> > >  include/uapi/drm/i915_drm.h| 107 +
> > >  8 files changed, 623 insertions(+), 82 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/i915_gem_exec.c
> > > 
> > 
> > Hi,
> > Can somebody please review this patch series, alongwith the libdrm
> > changes(http://lists.freedesktop.org/archives/intel-gfx/2014-June/047296.html)
> >  and igt 
> > (http://lists.freedesktop.org/archives/intel-gfx/2014-June/047295.html)
> > 
> > Thanks,
> > Sourab
> 
> Hi,
> Can you please review this patch series.

So on a quick look the kernel side looks sane. The async blitter clear
will have integration issues with the execlist stuff, so having a cpu
clear might be useful and adding the blt clear as a second step. Please
coordinate with the execlist owner.

What's definitely missing is igt coverage. I think we need at least:
- Basic ioctl coverage for create2, including cross-checking with older
  ioctls.
- Testcase for stolen memory including checking that impossible operations
  are all caught correctly.
- Exercising the stolen reaping of purgeable objects.
- Checking that stolen objects are properly cleared.

See http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html for
general testing requirements and
http://blog.ffwll.ch/2013/11/botching-up-ioctls.html for the special
considerations ioctls require.

Thanks, 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 00/43] Execlists v5

[Daniel Vetter]

Please format mails to a width of 75 chars or so. Decent mailers should do
that for you when hitting send.

On Thu, Jul 24, 2014 at 05:04:08PM +0100, Thomas Daniel wrote:
> From: Thomas Daniel 
> The previous comment about the WAs still applies. I reproduce it here
> for completeness:
> 
> "One other caveat I have noticed is that many WAs in
> gen8_init_clock_gating (those that affect registers that now exist
> per-context) can get lost in the render default context. The reason is,
> in Execlists, a context is saved as soon as head = tail (with
> MI_SET_CONTEXT, however, the context wouldn't be saved until you tried
> to restore a different context). As we are sending the golden state
> batchbuffer to the render ring as soon as the rings are initialized, we
> are effectively saving the default context before gen8_init_clock_gating
> has an opportunity to set the WAs. I haven't noticed any ill-effect from
> this (yet) but it would be a good idea to move the WAs somewhere else
> (ring init looks like a good place). I believe there is already work in
> progress to create a new WA architecture, so this can be tackled there."

This sounds like the w/a test patch to compare wa reg state after system
s/r, runtime pm, gpu reset and driver reload should also have a test for
multiple contexts. I'll add it to the wishlist.
-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] [Announcement] Updates to XenGT - a Mediated Graphics Passthrough Solution from Intel

[Jike Song]

Hi all,

We're pleased to announce an update to Intel Graphics Virtualization Technology 
(Intel GVT-g, formerly known as XenGT). Intel GVT-g is a complete vGPU solution 
with mediated pass-through, supported today on 4th generation Intel Core(TM) 
processors with Intel Graphics processors. A virtual GPU instance is maintained 
for each VM, with part of performance critical resources directly assigned. The 
capability of running native graphics driver inside a VM, without hypervisor 
intervention in performance critical paths, achieves a good balance among 
performance, feature, and sharing capability. Though we only support Xen on 
Intel Processor Graphics so far, the core logic can be easily ported to other 
hypervisors.

The news of this update:

- Project code name is "XenGT", now official name is Intel Graphics 
Virtualization Technology (Intel GVT-g)
- Currently Intel GVT-g supports Intel Processor Graphics built into 
4th generation Intel Core processors - Haswell platform
- Moving forward, XenGT will change to quarterly release cadence. Next 
update will be around early October, 2014.

This update consists of:

- Stability fixes, e.g. stable DP support
- Display enhancements, e.g. virtual monitor support. Users can define 
a virtual monitor type with customized EDID for virtual machines, not 
necessarily the same as physical monitors.
- Improved support for GPU recovery
- Experimental Windows HVM support. To download the experimental 
version, see setup guide for details
- Experimental Hardware Media Acceleration for decoding.


Please refer to the new setup guide, which provides step-to-step details about 
building/configuring/running Intel GVT-g:


https://github.com/01org/XenGT-Preview-kernel/blob/master/XenGT_Setup_Guide.pdf


The new source codes are available at the updated github repos:

Linux: https://github.com/01org/XenGT-Preview-kernel.git
Xen: https://github.com/01org/XenGT-Preview-xen.git
Qemu: https://github.com/01org/XenGT-Preview-qemu.git


More information about Intel GVT-g background, architecture, etc can be found 
at:


https://www.usenix.org/conference/atc14/technical-sessions/presentation/tian

http://events.linuxfoundation.org/sites/events/files/slides/XenGT-Xen%20Summit-v7_0.pdf
https://01.org/xen/blogs/srclarkx/2013/graphics-virtualization-xengt

The previous update can be found here:

http://lists.xen.org/archives/html/xen-devel/2014-02/msg01848.html

Appreciate your comments!




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

Re: [Intel-gfx] [PATCH 01/43] drm/i915: Reorder the actual workload submission so that args checking is done earlier

[Daniel Vetter]

On Thu, Jul 24, 2014 at 05:04:09PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo 
> 
> In this patch:
> 
> commit 78382593e921c88371abd019aca8978db3248a8f
> Author: Oscar Mateo 
> Date:   Thu Jul 3 16:28:05 2014 +0100
> 
> drm/i915: Extract the actual workload submission mechanism from execbuffer
> 
> So that we isolate the legacy ringbuffer submission mechanism, which 
> becomes
> a good candidate to be abstracted away. This is prep-work for Execlists 
> (which
> will its own workload submission mechanism).
> 
> No functional changes.
> 
> I changed the order in which the args checking is done. I don't know why I 
> did (brain
> fade?) but itś not right. I haven't seen any ill effect from this, but the 
> Execlists
> version of this function will have problems if the order is not correct.
> 
> Signed-off-by: Oscar Mateo 

I don't think this matters - the point of no return for legacy execbuf is
the call to ring->dispatch. After that nothing may fail any more. But as
long as we track state correctly (e.g. if we've switched the context
already) we'll be fine.

So presuming I'm not blind I dont' think this is needed. But maybe Chris
spots something.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   86 
> ++--
>  1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 60998fc..c5115957 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1042,6 +1042,43 @@ legacy_ringbuffer_submission(struct drm_device *dev, 
> struct drm_file *file,
>   u32 instp_mask;
>   int i, ret = 0;
>  
> + instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
> + instp_mask = I915_EXEC_CONSTANTS_MASK;
> + switch (instp_mode) {
> + case I915_EXEC_CONSTANTS_REL_GENERAL:
> + case I915_EXEC_CONSTANTS_ABSOLUTE:
> + case I915_EXEC_CONSTANTS_REL_SURFACE:
> + if (instp_mode != 0 && ring != &dev_priv->ring[RCS]) {
> + DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + if (instp_mode != dev_priv->relative_constants_mode) {
> + if (INTEL_INFO(dev)->gen < 4) {
> + DRM_DEBUG("no rel constants on pre-gen4\n");
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + if (INTEL_INFO(dev)->gen > 5 &&
> + instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
> + DRM_DEBUG("rel surface constants mode invalid 
> on gen5+\n");
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + /* The HW changed the meaning on this bit on gen6 */
> + if (INTEL_INFO(dev)->gen >= 6)
> + instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
> + }
> + break;
> + default:
> + DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode);
> + ret = -EINVAL;
> + goto error;
> + }
> +
>   if (args->num_cliprects != 0) {
>   if (ring != &dev_priv->ring[RCS]) {
>   DRM_DEBUG("clip rectangles are only valid with the 
> render ring\n");
> @@ -1085,6 +1122,12 @@ legacy_ringbuffer_submission(struct drm_device *dev, 
> struct drm_file *file,
>   }
>   }
>  
> + if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
> + ret = i915_reset_gen7_sol_offsets(dev, ring);
> + if (ret)
> + goto error;
> + }
> +
>   ret = i915_gem_execbuffer_move_to_gpu(ring, vmas);
>   if (ret)
>   goto error;
> @@ -1093,43 +1136,6 @@ legacy_ringbuffer_submission(struct drm_device *dev, 
> struct drm_file *file,
>   if (ret)
>   goto error;
>  
> - instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
> - instp_mask = I915_EXEC_CONSTANTS_MASK;
> - switch (instp_mode) {
> - case I915_EXEC_CONSTANTS_REL_GENERAL:
> - case I915_EXEC_CONSTANTS_ABSOLUTE:
> - case I915_EXEC_CONSTANTS_REL_SURFACE:
> - if (instp_mode != 0 && ring != &dev_priv->ring[RCS]) {
> - DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
> - ret = -EINVAL;
> - goto error;
> - }
> -
> - if (instp_mode != dev_priv->relative_constants_mode) {
> - if (INTEL_INFO(dev)->gen < 4) {
> - DRM_DEBUG("no rel constants on pre-gen4\n");
> - ret = -EINVAL;
> - goto error;
> - }
> -
> - if (INTEL

Re: [Intel-gfx] [PATCH] drm/i915: Fix crash when failing to parse MIPI VBT

[Daniel Vetter]

On Fri, Jul 25, 2014 at 09:47:06AM +0200, Daniel Vetter wrote:
> On Thu, Jul 24, 2014 at 03:16:12PM +0100, rafael.barba...@intel.com wrote:
> > From: Rafael Barbalho 
> > 
> > This particular nasty presented itself while trying to register the
> > intelfb device (intel_fbdev.c). During the process of registering the device
> > the driver will disable the crtc via i9xx_crtc_disable. These will
> > also disable the panel using the generic mipi panel functions in
> > dsi_mod_vbt_generic.c. The stale MIPI generic data sequence pointers would
> > cause a crash within those functions. However, all of this is happening
> > while console_lock is held from do_register_framebuffer inside fbcon.c. 
> > Which
> > means that you got kernel log and just the device appearing to reboot/hang 
> > for
> > no apparent reason.
> 
> CONFIG_I915_FBDEV=n for when the console_lock gets in the way.
> 
> > The fault started from the FB_EVENT_FB_REGISTERED event using the
> > fb_notifier_call_chain call in fbcon.c.
> > 
> > Cc: Shobhit Kumar 
> > Signed-off-by: Rafael Barbalho 
> 
> Queued for -next, thanks for the patch.

Actually this is for fixes since 3.16 has dsi support. Also for
regressions please cite the commit that introduced the offending
behaviour. I've added that.
-Daniel

> -Daniel
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index 608ed30..a669550 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -878,7 +878,7 @@ err:
> >  
> > /* error during parsing so set all pointers to null
> >  * because of partial parsing */
> > -   memset(dev_priv->vbt.dsi.sequence, 0, MIPI_SEQ_MAX);
> > +   memset(dev_priv->vbt.dsi.sequence, 0, 
> > sizeof(dev_priv->vbt.dsi.sequence));
> >  }
> >  
> >  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port 
> > port,
> > -- 
> > 2.0.2
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

Re: [Intel-gfx] [PATCH] drm/i915: Fix crash when failing to parse MIPI VBT

[Daniel Vetter]

On Thu, Jul 24, 2014 at 03:16:12PM +0100, rafael.barba...@intel.com wrote:
> From: Rafael Barbalho 
> 
> This particular nasty presented itself while trying to register the
> intelfb device (intel_fbdev.c). During the process of registering the device
> the driver will disable the crtc via i9xx_crtc_disable. These will
> also disable the panel using the generic mipi panel functions in
> dsi_mod_vbt_generic.c. The stale MIPI generic data sequence pointers would
> cause a crash within those functions. However, all of this is happening
> while console_lock is held from do_register_framebuffer inside fbcon.c. Which
> means that you got kernel log and just the device appearing to reboot/hang for
> no apparent reason.

CONFIG_I915_FBDEV=n for when the console_lock gets in the way.

> The fault started from the FB_EVENT_FB_REGISTERED event using the
> fb_notifier_call_chain call in fbcon.c.
> 
> Cc: Shobhit Kumar 
> Signed-off-by: Rafael Barbalho 

Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 608ed30..a669550 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -878,7 +878,7 @@ err:
>  
>   /* error during parsing so set all pointers to null
>* because of partial parsing */
> - memset(dev_priv->vbt.dsi.sequence, 0, MIPI_SEQ_MAX);
> + memset(dev_priv->vbt.dsi.sequence, 0, 
> sizeof(dev_priv->vbt.dsi.sequence));
>  }
>  
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> -- 
> 2.0.2
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH] drm/i915/userptr: Keep spin_lock/unlock in the same block

[Daniel Vetter]

On Thu, Jul 24, 2014 at 01:28:44PM +0100, Chris Wilson wrote:
> Move the code around in order to acquire and release the spinlock in the
> same function and in the same block. This keeps static analysers happy
> and the reader sane.
> 
> Suggested-by: Julia Lawall 
> Signed-off-by: Chris Wilson 
> Cc: Julia Lawall 

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_userptr.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
> b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index 12358fd..4ef5a92 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -96,10 +96,10 @@ static unsigned long cancel_userptr(struct 
> drm_i915_gem_object *obj)
>   return end;
>  }
>  
> -static void invalidate_range__linear(struct i915_mmu_notifier *mn,
> -  struct mm_struct *mm,
> -  unsigned long start,
> -  unsigned long end)
> +static void *invalidate_range__linear(struct i915_mmu_notifier *mn,
> +   struct mm_struct *mm,
> +   unsigned long start,
> +   unsigned long end)
>  {
>   struct i915_mmu_object *mo;
>   unsigned long serial;
> @@ -123,7 +123,7 @@ restart:
>   goto restart;
>   }
>  
> - spin_unlock(&mn->lock);
> + return NULL;
>  }
>  
>  static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier 
> *_mn,
> @@ -138,13 +138,12 @@ static void 
> i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
>  
>   end--; /* interval ranges are inclusive, but invalidate range is 
> exclusive */
>   while (next < end) {
> - struct drm_i915_gem_object *obj;
> + struct drm_i915_gem_object *obj = NULL;
>  
> - obj = NULL;
>   spin_lock(&mn->lock);
>   if (mn->has_linear)
> - return invalidate_range__linear(mn, mm, start, end);
> - if (serial == mn->serial)
> + it = invalidate_range__linear(mn, mm, start, end);
> + else if (serial == mn->serial)
>   it = interval_tree_iter_next(it, next, end);
>   else
>   it = interval_tree_iter_first(&mn->objects, start, end);
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] Intel Linux Graphic Installer doesn't work on ubuntu 14.04

[Vivek Dasmohapatra]

I'm pretty sure that it can be done because it is already worked on
the fedora 20. Infact I have already installed the graphic installer
and I have activated two monitors simultaneously. Now I would like to
do the same on Ubuntu 14.04. Actually I get the errors that you see on
the screenshots attached.


Hi there - the current installer doesn't actually update the i915
module - 14.04 came out with most components already at or later
than the planned quarterly Intel release, except for vaapi video
playback acceleration.

I should note here that the i915 driver we ship isn't proprietary:
It's the mainline kernel i915 driver - the installer is just a way
for use to get it to distro users a little ahead of [distro] schedule.

The next release, due out soon, will have an updated i915 driver in it
for 14.04.

But let's get back to your problems:

When the installer starts, it should ask you for your password: It
does this so it can get the permissions it needs from policykit
to add apt sources to your system and upgrade packages.

So, a couple of questions:

a - Did the installer ask you for your password?
b - If it did, are the permissions on /etc/apt/sources.list.d
reasonable? Or is there some reason you would not be able
to edit them even with sudo [for example]?

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

[Intel-gfx] [BUG?] 3.16-rc6 ... at drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()

[Ian Kumlien]

Try four, now including CC lists for the intel driver...

---

Hi again,


Playing some more with the new kernel release i noticed this:
[ 9064.008821] WARNING: CPU: 3 PID: 22929 at 
drivers/gpu/drm/i915/intel_pm.c:5997 intel_display_power_put+0x12d/0x160()
[ 9064.008822] Modules linked in: uas bnep b43 mac80211 cfg80211 
snd_hda_codec_hdmi btusb bluetooth 6lowpan_iphc rfkill snd_hda_codec_cirrus 
uvcvideo snd_hda_codec_generic videobuf2_vmalloc videobuf2_memops 
videobuf2_core snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep snd_pcm 
snd_timer sdhci_pci snd sdhci soundcore mmc_core bcma
[ 9064.008839] CPU: 3 PID: 22929 Comm: kworker/3:2 Tainted: GW 
3.16.0-rc6 #23
[ 9064.008840] Hardware name: Apple Inc. MacBookPro10,2/Mac-AFD8A9D944EA4843, 
BIOS MBP102.88Z.0106.B03.1211161133 11/16/2012
[ 9064.008843] Workqueue: events edp_panel_vdd_work
[ 9064.008844]  0009 88015ba77d28 8198ea2d 

[ 9064.008846]  88015ba77d60 810cbac8 8802610b002c 
000c7204
[ 9064.008848]  0001 8802610b80f0 8802610b 
88015ba77d70
[ 9064.008850] Call Trace:
[ 9064.008854]  [] dump_stack+0x4e/0x7a
[ 9064.008857]  [] warn_slowpath_common+0x78/0xa0
[ 9064.008858]  [] warn_slowpath_null+0x15/0x20
[ 9064.008860]  [] intel_display_power_put+0x12d/0x160
[ 9064.008862]  [] edp_panel_vdd_off_sync+0xf4/0x1c0
[ 9064.008863]  [] edp_panel_vdd_work+0x2f/0x40
[ 9064.008866]  [] process_one_work+0x16e/0x480
[ 9064.008868]  [] worker_thread+0x11b/0x520
[ 9064.008870]  [] ? create_and_start_worker+0x50/0x50
[ 9064.008872]  [] kthread+0xc4/0xe0
[ 9064.008874]  [] ? kthread_create_on_node+0x170/0x170
[ 9064.008877]  [] ret_from_fork+0x7c/0xb0
[ 9064.008878]  [] ? kthread_create_on_node+0x170/0x170
[ 9064.008880] ---[ end trace 17f9738f20aec288 ]---



I had multiples of them in my dmesg, however, this seems to fix it:
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8a1a4fb..4c3249d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1252,6 +1252,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp 
*intel_dp)
intel_dp->last_power_cycle = jiffies;
 
power_domain = intel_display_port_power_domain(intel_encoder);
+   intel_display_power_get(dev_priv, power_domain);
intel_display_power_put(dev_priv, power_domain);
}
 }
@@ -1371,6 +1372,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
 
/* We got a reference when we enabled the VDD. */
power_domain = intel_display_port_power_domain(intel_encoder);
+   intel_display_power_get(dev_priv, power_domain);
intel_display_power_put(dev_priv, power_domain);
 }
---


The question however is: Is this the correct approach? Should it be done
differently?
(This handles the "close and open lid" usecase, i don't know if there
are others, a grep indicated that there might be two more missing...)




commit 423d01e192504764d5cb59effe5da68b035a0d41
Author: Ian Kumlien 
Date:   Tue Jul 22 21:10:50 2014 +0200

Add intel_display_power_get before _put

When closing the lid when using 3.16-rc6 on my laptop i got
three "WARN_ON" back traces.

Example:
[ 9064.008821] WARNING: CPU: 3 PID: 22929 at
drivers/gpu/drm/i915/intel_pm.c:5997
intel_display_power_put+0x12d/0x160()
[ 9064.008822] Modules linked in: uas bnep b43 mac80211 cfg80211
snd_hda_codec_hdmi btusb bluetooth 6lowpan_iphc rfkill
snd_hda_codec_cirrus uvcvideo snd_hda_codec_generic videobuf2_vmalloc
videobuf2_memops videobuf2_core snd_hda_intel snd_hda_controller
snd_hda_codec snd_hwdep snd_pcm snd_timer sdhci_pci snd sdhci soundcore
mmc_core bcma
[ 9064.008839] CPU: 3 PID: 22929 Comm: kworker/3:2 Tainted: GW
3.16.0-rc6 #23
[ 9064.008840] Hardware name: Apple Inc.
MacBookPro10,2/Mac-AFD8A9D944EA4843, BIOS MBP102.88Z.0106.B03.1211161133
11/16/2012
[ 9064.008843] Workqueue: events edp_panel_vdd_work
[ 9064.008844]  0009 88015ba77d28 8198ea2d

[ 9064.008846]  88015ba77d60 810cbac8 8802610b002c
000c7204
[ 9064.008848]  0001 8802610b80f0 8802610b
88015ba77d70
[ 9064.008850] Call Trace:
[ 9064.008854]  [] dump_stack+0x4e/0x7a
[ 9064.008857]  [] warn_slowpath_common+0x78/0xa0
[ 9064.008858]  [] warn_slowpath_null+0x15/0x20
[ 9064.008860]  [] intel_display_power_put+0x12d/0x160
[ 9064.008862]  [] edp_panel_vdd_off_sync+0xf4/0x1c0
[ 9064.008863]  [] edp_panel_vdd_work+0x2f/0x40
[ 9064.008866]  [] process_one_work+0x16e/0x480
[ 9064.008868]  [] worker_thread+0x11b/0x520
[ 9064.008870]  [] ? create_and_start_worker+0x50/0x50
[ 9064.008872]  [] kthread+0xc4/0xe0
[ 9064.008874]  [] ?
kthread_create_on_node+0x170/0x170
[ 9064.008877]  [] 

Re: [Intel-gfx] [PATCH] intel-gpu-tools: add sys/wait.h to pm_rps.c

[Daniel Vetter]

On Thu, Jul 24, 2014 at 02:54:27PM +0100, tim.g...@intel.com wrote:
> From: Tim Gore 
> 
> commit 745945546f7366a413a3a51a37f90caa3a227b1d
> breaks the build under Android because some of the
> macros used in pm_rps.c are defined in sys/wait.h
> which is not included.
> 
> Signed-off-by: Tim Gore 

Merged, thanks.
-Daniel
> ---
>  tests/pm_rps.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> index 8593e36..5264436 100644
> --- a/tests/pm_rps.c
> +++ b/tests/pm_rps.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "drmtest.h"
>  #include "intel_io.h"
> -- 
> 1.9.2
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH] NEWS: Updates

[Daniel Vetter]

On Thu, Jul 24, 2014 at 05:49:29PM -0700, Matt Roper wrote:
> On Wed, Jul 23, 2014 at 10:32:43PM +0200, Daniel Vetter wrote:
> > ---
> >  NEWS | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/NEWS b/NEWS
> > index 1b5ee83ec849..4866d59b5619 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -10,6 +10,12 @@ Release 1.8 (-xx-xx)
> >  
> >  - As usual piles of new tests.
> >  
> > +- Improved plane/pipe handling in the igt_kms library (Damien).
> > +
> > +- Unified option parsing between simple tests and tests with subtests 
> > (Thomas).
> > +  This will allow us to merge the different Makefile targets once test 
> > runners
> > +  are converted.
> > +
> 
> Might be worth adding a bullet for the new "commit style" support in
> igt_kms (igt_display_commit2 / igt_display_try_commit2).  That allows us
> to expose the universal plane interfaces today and also provides a
> natural way to expose atomic interfaces in the future.

Added and pushed out.
-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] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

[Daniel Vetter]

On Fri, Jul 25, 2014 at 01:28:48PM +1000, Dave Airlie wrote:
> On 23 July 2014 15:11, Daniel Vetter  wrote:
> > On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kam...@intel.com wrote:
> >> From: Borun Fu 
> >>
> >> On VLV, after i915_pm_suspend display power wells are staying
> >> power ungated. So, after initiating mem sleep "echo mem > /sys/power/state"
> >> Display is staing D0 State. There might be better way/place to power gate
> >> these wells. Also, we need to make sure that if wells are power gated due 
> >> to
> >> DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.
> >>
> >> v2: Extracted helper for intel_crtc_disable and power gating CRTC power 
> >> wells.
> >> [Daniel]
> >>
> >> Cc: Imre Deak 
> >> Cc: Paulo Zanoni 
> >> Cc: Daniel Vetter 
> >> Cc: Jani Nikula 
> >> Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848
> >
> > s-o-b from the original author (Borun Fu) missing. Added myself since we
> > all work for the same company, but please don't forget this. Every person
> > including the original author, who handles a patch must add their sob
> > line.
> > -Daniel
> 
> Is this queued or on its way, I was getting a warning on HSW about not
> entering pc8+
> due to display with MST enabled, and I thought it was MSTs fault, but I 
> suspect
> its just this,
> 
> mode set turns the global resources power well on, but nothing ever
> turns it off.

mst doesn't factor into this. Might need to double-check but from what
I've seen mst was only wired into the system s/r paths, not the runtime
pm. They're unfortunately not sufficiently shared. So I'd suspect that.

The bug here happens when you move/update the cursor (or sprite/primary
plane) while the display is off. You'll get unclaimed register read/write
errors, no WARNINGs if you hit this one.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: BDW Software Turbo

[Daniel Vetter]

On Thu, Jul 24, 2014 at 01:28:21PM -0700, Jesse Barnes wrote:
> If that won't work, you could just use a timer, or tie into some other
> event that happens when the GPU is busy (e.g. execbuf or retire) instead
> of trying to tie into the display side of things.

Yes, tying into a normal timer is probably best. At least I get the
impression that we only need something regular. Of course once the gpu is
idle we need to stop rearming that timer and restart it upon first batch
when transitioning out of idle.
-Daniel

> 
> Jesse
> 
> On Tue, 15 Jul 2014 06:35:20 +
> "Sun, Daisy"  wrote:
> 
> > Hi Daniel, Chris
> > 
> > The concern for traditional X and media server do make sense. I'll update 
> > the patch with RP_UP_EI_INTERRUPT as trigger instead of the page flip.
> > Thanks for the valuable input.
> > 
> > - Daisy
> > 
> > -Original Message-
> > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of 
> > Daniel Vetter
> > Sent: Monday, July 14, 2014 12:04 AM
> > To: Sun, Daisy
> > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: BDW Software Turbo
> > 
> > On Mon, Jul 14, 2014 at 8:59 AM, Daniel Vetter  wrote:
> > > On Mon, Jul 14, 2014 at 04:22:44AM +, Sun, Daisy wrote:
> > >> 3) The function will be called when flip happened, this should cover 
> > >> most of the cases. One exception is background media process without 
> > >> any display output, it's relatively rare.  Please let me know if you 
> > >> have concern on other cases, I will try to cover it definitely.
> > >
> > > Traditional X never flips. And we kinda have to keep this working. 
> > > Instead of checking when flipping we need to check at regular time 
> > > intervals I guess, for as long as the gt is busy.
> > 
> > Oh and transcode servers are a real thing apparently. They also never flip, 
> > and we actually care from a business pov ...
> > -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
> > 
> 
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center

-- 
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] [RFC 15/44] drm/i915: Added deferred work handler for scheduler

[Daniel Vetter]

On Thu, Jul 24, 2014 at 04:42:55PM +0100, John Harrison wrote:
> 
> On 23/07/2014 19:50, Daniel Vetter wrote:
> >On Wed, Jul 23, 2014 at 5:37 PM, John Harrison
> > wrote:
>    diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 0977653..fbafa68 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1075,6 +1075,16 @@ struct i915_gem_mm {
>  struct delayed_work idle_work;
>  /**
> +* New scheme is to get an interrupt after every work packet
> +* in order to allow the low latency scheduling of pending
> +* packets. The idea behind adding new packets to a pending
> +* queue rather than directly into the hardware ring buffer
> +* is to allow high priority packets to over take low priority
> +* ones.
> +*/
> +   struct work_struct scheduler_work;
> >>>Latency for work items isn't too awesome, and e.g. Oscar's execlist code
> >>>latches the next context right away from the irq handler. Why can't we do
> >>>something similar for the scheduler? Fishing the next item out of a
> >>>priority queue shouldn't be expensive ...
> >>>-Daniel
> >>
> >>The problem is that taking batch buffers from the scheduler's queue and
> >>submitting them to the hardware requires lots of processing that is not IRQ
> >>compatible. It isn't just a simple register write. Half of the code in
> >>'i915_gem_do_execbuffer()' must be executed. Probably/possibly it could be
> >>made IRQ friendly but that would place a lot of restrictions on a lot of
> >>code that currently doesn't expect to be restricted. Instead, the submission
> >>is done via a work handler that acquires the driver mutex lock.
> >>
> >>In order to cover the extra latency, the scheduler operates in a
> >>multi-buffered mode and aims to keep eight batch buffers in flight at all
> >>times. That number being obtained empirically by running lots of benchmarks
> >>on Android with lots of different settings and seeing where the buffer size
> >>stopped making a difference.
> >So I've tried to stitch together that part of the scheduler from the
> >patch series. Afaics you do the actual scheduling under the protection
> >of irqsave spinlocks (well you also hold the dev->struct_mutex). That
> >means you disable local interrupts. Up to the actual submit point I
> >spotted two such critcial sections encompassing pretty much all the
> >code.
> >
> >If we'd run the same code from the interrupt handler then only our own
> >interrupt handler is blocked, all other interrupt processing can
> >continue. So that's actually a lot nicer than what you have. In any
> >case you can't do expensive operations under an irqsave spinlock
> >anyway.
> >
> >So either I've missed something big here, or this justification doesn't hold 
> >up.
> 
> The irqsave spinlock is only held while manipulating the internal scheduler
> data structures. It is released immediately prior to calling
> i915_gem_do_execbuffer_final(). So the actual submission code path is done
> with the driver mutex but no spinlocks. I'm sure I got 'scheduling while
> atomic' bug checks the one time I accidentally left the spinlock held.

Ok, missed something ;-)

btw for big patch series please upload them somewhere on a public git
(github or so). Generally I review patches only by reading them because
trying to apply them usually results in some conflicts. But with big patch
series like this here that doesn't work, especially when everything is
tightly coupled (iirc I had to jump around in about 10 different patches
to figure out what the work handler looks like). Or maybe I didn't
understand the patch split flow and read it backwards ;-)
-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