[Intel-gfx] [PATCH] drm/i915: only hook up hpd pulse for DP outputs

2014-08-04 Thread Chris Wilson
On HSW+, the digital encoders are shared between HDMI and DP outputs,
with one encoder masquerading as both. The VBT should tell us if we need
to have DP or HDMI support on a particular port, but if we don't have DP
support and we enable the DP hpd pulse handler then we cause an oops.

Don't hook up the DP hpd handling if we don't have a DP port.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81856
Reported-by: Intel QA Team.
Signed-off-by: Dave Airlie airl...@redhat.com # v1
[ickle: Fix the error handling after a malloc failure]
Cc: Dave Airlie airl...@redhat.com
Cc: Paulo Zanoni przan...@gmail.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/intel_ddi.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a6024de..3634575 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1557,15 +1557,13 @@ void intel_ddi_init(struct drm_device *dev, enum port 
port)
struct intel_digital_port *intel_dig_port;
struct intel_encoder *intel_encoder;
struct drm_encoder *encoder;
-   struct intel_connector *hdmi_connector = NULL;
-   struct intel_connector *dp_connector = NULL;
bool init_hdmi, init_dp;
 
init_hdmi = (dev_priv-vbt.ddi_port_info[port].supports_dvi ||
 dev_priv-vbt.ddi_port_info[port].supports_hdmi);
init_dp = dev_priv-vbt.ddi_port_info[port].supports_dp;
if (!init_dp  !init_hdmi) {
-   DRM_DEBUG_KMS(VBT says port %c is not DVI/HDMI/DP 
compatible\n,
+   DRM_DEBUG_KMS(VBT says port %c is not DVI/HDMI/DP compatible, 
assuming it is\n,
  port_name(port));
init_hdmi = true;
init_dp = true;
@@ -1595,23 +1593,28 @@ void intel_ddi_init(struct drm_device *dev, enum port 
port)
   DDI_A_4_LANES);
 
intel_encoder-type = INTEL_OUTPUT_UNKNOWN;
-   intel_encoder-crtc_mask =  (1  0) | (1  1) | (1  2);
+   intel_encoder-crtc_mask = (1  0) | (1  1) | (1  2);
intel_encoder-cloneable = 0;
intel_encoder-hot_plug = intel_ddi_hot_plug;
 
-   intel_dig_port-hpd_pulse = intel_dp_hpd_pulse;
-   dev_priv-hpd_irq_port[port] = intel_dig_port;
+   if (init_dp) {
+   if (!intel_ddi_init_dp_connector(intel_dig_port))
+   goto err;
 
-   if (init_dp)
-   dp_connector = intel_ddi_init_dp_connector(intel_dig_port);
+   intel_dig_port-hpd_pulse = intel_dp_hpd_pulse;
+   dev_priv-hpd_irq_port[port] = intel_dig_port;
+   }
 
/* In theory we don't need the encoder-type check, but leave it just in
 * case we have some really bad VBTs... */
-   if (intel_encoder-type != INTEL_OUTPUT_EDP  init_hdmi)
-   hdmi_connector = intel_ddi_init_hdmi_connector(intel_dig_port);
-
-   if (!dp_connector  !hdmi_connector) {
-   drm_encoder_cleanup(encoder);
-   kfree(intel_dig_port);
+   if (intel_encoder-type != INTEL_OUTPUT_EDP  init_hdmi) {
+   if (!intel_ddi_init_hdmi_connector(intel_dig_port))
+   goto err;
}
+
+   return;
+
+err:
+   drm_encoder_cleanup(encoder);
+   kfree(intel_dig_port);
 }
-- 
1.9.1

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


Re: [Intel-gfx] [PATCH] drm/i915: only hook up hpd pulse for DP outputs

2014-08-04 Thread Dave Airlie
On 4 August 2014 16:15, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On HSW+, the digital encoders are shared between HDMI and DP outputs,
 with one encoder masquerading as both. The VBT should tell us if we need
 to have DP or HDMI support on a particular port, but if we don't have DP
 support and we enable the DP hpd pulse handler then we cause an oops.

 Don't hook up the DP hpd handling if we don't have a DP port.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81856

Reviewed-by: Dave Airlie airl...@redhat.com

Thanks,

I'll put this into -next straight, since when I merged MST I agreed
I'd be fast about fixing my crap.

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


[Intel-gfx] [PULL] drm-intel-next

2014-08-04 Thread Daniel Vetter
Hi Dave,

Final feature pull for 3.17.

drm-intel-next-2014-07-25:
- 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 :(

QA is a bit unhappy about the DP MST stuff since it broke hpd testing a
bit, but otherwise looks sane. I've backmerged drm-next to resolve
conflicts with the mst stuff, which means the new tag itself doesn't
contain the overview as usual.

Cheers, Daniel


The following changes since commit e05444be705b5c7c7f85d7722b6f97f3a6732d54:

  drm/i915: fix initial fbdev setup warnings (2014-07-24 10:27:42 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2014-07-25-merged

for you to fetch changes up to 4dac3edfe68e5e1b3c2216b84ba160572420fa40:

  Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next 
(2014-07-29 20:49:36 +0200)


Armin Reese (1):
  drm/i915: Do not unmap object unless no other VMAs reference it

Ben Widawsky (2):
  drm/i915/error: Check the potential ctx obj's vm
  drm/i915: Reorder ctx unref on ppgtt cleanup

Borun Fu (1):
  drm/i915: Power gating display wells during i915_pm_suspend

Chris Wilson (5):
  drm/i915: Handle failure to kick out a conflicting fb driver
  drm/i915: Abandon oom quickly if killed by a signal
  drm/i915: Initialise userptr mmu_notifier serial to 1
  drm/i915: Allow overlapping userptr objects
  drm/i915/userptr: Keep spin_lock/unlock in the same block

Damien Lespiau (3):
  drm/i915: PM irq enabling is generic on gen8, too
  drm/i915: Also give the sprite width for WM computation
  drm/i915: Make the WRPLL names const

Daniel Vetter (14):
  drm/i915: ddi: enable runtime pm during dpms
  drm/i915: Run psr_setup unconditionally
  drm/i915: Add a FIXME about drrs/psr interactions
  drm/i915: Track the psr dp connector in dev_priv-psr.enabled
  drm/i915: Don't try to disable psr harder from the work item
  drm/i915: Lock down psr sw/hw state tracking
  drm/i915: More checks for psr.enabled
  drm/i915: Add locking to psr code
  drm/i915: Fix up PSR frontbuffer tracking
  drm/i915: Improve PSR debugfs output
  drm/i915: Remove redundant HAS_PSR checks
  drm/i915: Use genX_ prefix for gt irq enable/disable functions
  drm/i915: Ditch UMS config option
  Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next

Deepak S (9):
  drm/i915: Read guaranteed freq for valleyview
  drm/i915: Add RP0/RP1/RPn render P state thresholds in VLV sysfs
  drm/i915: populate mem_freq/cz_clock for chv
  drm/i915: CHV GPU frequency to opcode functions
  drm/i915/chv: Add basic PM interrupt support for CHV
  drm/i915: Add RP1 render P state thresholds in CHV
  drm/i915: Force GPU Freq to lowest while suspending.
  drm/i915/chv: Drop WaGsvBringDownFreqInRc6
  drm/i915: Fix printing proper min/min/rpe values in debugfs

Fengguang Wu (1):
  drm/i915: byt_gpu_freq() can be static

Jani Nikula (2):
  drm/i915: extract backlight minimum brightness from VBT
  drm/i915: respect the VBT minimum backlight brightness

Jesse Barnes (5):
  drm/i915: don't warn if IRQs are disabled when shutting down display IRQs
  drm/i915: add helper for checking whether IRQs are enabled
  drm/i915: set pm._irqs_disabled at IRQ init time
  drm/i915: clear pm._irqs_disabled field after installing IRQs
  drm/i915: mark IRQs as disabled on unload

Mika Kuoppala (1):
  drm/i915/chv: calculate rc6 residency correctly

Paulo Zanoni (7):
  drm/i915: remove useless runtime PM get calls
  drm/i915: don't write powered down IRQ registers on Gen 8
  drm/i915: HSW_BLC_PWM2_CTL doesn't exist on BDW
  drm/i915: extract and improve gen8_irq_power_well_post_enable
  drm/i915: reorganize the unclaimed register detection code
  drm/i915: BDW can also detect unclaimed registers
  drm/i915: remove plane/cursor/pipe assertions from intel_crtc_disable

Rodrigo Vivi (2):
  drm/i915: Enable PSR by default.
  drm/i915: Fix possible overflow when recording semaphore states.

Sagar Kamble (2):
  Documentation: drm: Removing placeholders for generic drm properties 
description
  Documentation: drm: describing rotation property

Sonika Jindal (8):
  drm/i915: Adding HAS_GMCH_DISPLAY macro
  drm/i915: Allowing changing of wm latencies for valid platforms
  drm/i915: Returning the right VGA control reg for 

Re: [Intel-gfx] [PATCH 1/1] Documentation: drm: describing drm properties exposed by various drivers

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 02:58:21PM +0200, Laurent Pinchart wrote:
 Hi Randy,
 
 On Thursday 31 July 2014 15:16:21 Randy Dunlap wrote:
  On 05/12/14 11:04, Randy Dunlap wrote:
   On 05/12/2014 08:54 AM, Daniel Vetter wrote:
   On Mon, May 12, 2014 at 08:23:45AM -0700, Randy Dunlap wrote:
   On 05/12/2014 01:58 AM, Daniel Vetter wrote:
   On Mon, May 12, 2014 at 06:24:57PM +1000, Dave Airlie wrote:
   If we decide to go for property documentation inside the source code
   then I believe we'll have to create our own format, as creating a
   properties table from kerneldoc information extracted from comments
   is probably not possible.
   
   Can comeone pick up the ball here and figure out what needs to be
   done?
   
   The reason why I want a central place for the documentation is to
   force people to collaborate outside their own sandbox when adding
   properties. Whether that's docbook or some text file I don't care so
   much at this point. The fact that it's a central place should mandate
   that the patches changing it will go through dri-devel and so
   everyone should se them, and when adding new properties it would make
   the patch author more likely to look around a bit before adding
   another slighty incompatible version of the same property. If someone
   has a better suggestion how to encforce this I'm all ears.
   
   Of course this idea can still fail if our esteemed maintainer merges
   stuff without checking for violations of this policy. Dave, any
   thoughts on the subject?
   
   Yeah I'm happy to block merging stuff, if we can spot new properties
   when stuff is posted on dri-devel, so much the better,
   
   most drivers still send everything via dri-devel anyways, its only
   really Intel I have to worry about so far,
   
   I'll enforce that all prop stuff gets cc: dri-devel and that it has
   updates for the prop docs.
   
   But we should definitely add it to the new driver review checklist as
   well.
   
   I'm also on the side of this patch is ugly and makes my eyes burn,
   please please get a plan to use something else ASAP, I'm willing to
   merge this but I'm tempted to give it a lifetime of a kernel or two
   before I burn it.
   
   Ok, I'll try to move make kerneldoc suck less up the task list and
   maybe find someone to do it for me internally ;-)
   
   I certainly have no objections to making kerneldoc suck less.
   There was already an attempt to use asciidoc (like git uses) for
   kernel-doc (a few years ago, by Sam Ravnborg).  I support(ed) that
   effort.
   
   Hm, do you have pointers to those? My google-fu seems lacking ...
   
   I googled for /kernel doc asciidoc ravnborg/ and found several hits for
   them.
  
   Ok, let's move this to the top and start discussions. The past few months
   I've written piles of kerneldoc comments for the DRM DocBook (all pulled
   in as kerneldoc, docbook .tmpl has just the chapter structure). DOC:
   sections are really useful to pull all the actual documentation out of
   the docbook xml into kerneldoc.
   
   But I've also done piles of docs for intel-gpu-tools, which is using
   gtkdoc. And there are some clear deficiencies:
   
   - No markdown for inline coments. Lack of lists and tables is hurting
 especially badly. If we add this (and I don't care one iota whether
 it's
   
   Yes, I've tried to add lists to kernel-doc notation but have failed so
   far.
   
 markdown or asciidoc or something else as long as it's readable plain
 text in comments) we should be able to move all the existing docbook
 xml paragraphs/lists/tables into kerneldoc comments.
   
   - Automatic cross-referencing of functions. If you place e.g. function()
 or #struct anywhere in a documentation comment gtk-doc automatically
 inserts a hyperlink to the relevant documentation page across the
 entire project. Really powerful and makes overview sections much more
 useful entry points for beginners since they can easily jump backforth
 betweeen the high-level overview and low-level per-function
 documentation.
   
   That's a nice-to-have IMO, but a really nice one.
   
   - In a really perfect world it would help if kerneldoc could collect
 structure member kerneldoc from per-member comments. Especially for
 large structures with lots of comments this would bring the kerneldoc
 and struct member much closer together.
   
   So that's my wishlist.
   
   OTOH, I would only want to add another way to do kernel-doc if it can be
   a full replacement for all of our docbook usage, i.e., it should provide
   a way that we can eliminate docbook and stop using it completely.
   
   Hm, I don't mind docbook at all, as long as all the real content is
   embedded into source files as kerneldoc and the docbook template just
   pulls in all the right bits and pieces. Even gtkdoc allos you to do that
   and pull in the different libararies (== header files with declarations
   for C) in the order you 

Re: [Intel-gfx] [PULL] drm-intel-next

2014-08-04 Thread Dave Airlie
On 4 August 2014 17:10, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 Hi Dave,

 Final feature pull for 3.17.

 drm-intel-next-2014-07-25:
 - 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 :(

 QA is a bit unhappy about the DP MST stuff since it broke hpd testing a
 bit, but otherwise looks sane. I've backmerged drm-next to resolve
 conflicts with the mst stuff, which means the new tag itself doesn't
 contain the overview as usual.

They are unhappy to do their job?

I'm a bit confused. :-P

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


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

2014-08-04 Thread Daniel Vetter
On Thu, Jul 31, 2014 at 04:37:14PM +, Mcaulay, Alistair wrote:
 Hi Daniel,
 
 Something more like this then?  (and revert the change to intel_ring_begin(), 
 putting it back to how it was )

Yeah, roughly. Except that I would place the reload_in_reset wrapping in
the i915_reset function. It is paramount that we never leak this outside
of the dev-struct_mutex protection so that other threads can't ever
observe this to be set. So putting it right next to the mutex locking is
better.

Also I think you've wrapped the wrong function - the re-init is done in
i915_gem_init_hw, this here just resets the software state (mostly) and is
done before the actual gpu hw reset is done. gem_init_hw is only run if
the reset succeeds.
-Daniel

 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 991b663..b811ff2 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1217,6 +1217,9 @@ struct i915_gpu_error {
  
   /* For missed irq/seqno simulation. */
   unsigned int test_irq_rings;
 +
 + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset  
  */
 + bool reload_in_progress;
  };
  
  enum modeset_restore {
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index b38e086..a25d3b5 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
   if (i915_terminally_wedged(error))
   return -EIO;
  
 - return -EAGAIN;
 + /* Check if GPU Reset is in progress */
 + if (!error-reload_in_reset)
 + return -EAGAIN;
   }
  
   return 0;
 @@ -2579,6 +2581,8 @@ void i915_gem_reset(struct drm_device *dev)
   struct intel_engine_cs *ring;
   int i;
  
 + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset 
 */
 + dev_priv-gpu_error.reload_in_reset = true;
   /*
* Before we free the objects from the requests, we need to inspect
* them for finding the guilty party. As the requests only borrow
 @@ -2591,6 +2595,8 @@ void i915_gem_reset(struct drm_device *dev)
   i915_gem_reset_ring_cleanup(dev_priv, ring);
  
   i915_gem_restore_fences(dev);
 +
 + dev_priv-gpu_error.reload_in_reset = false;
  }
 
 
 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
 Sent: Wednesday, July 30, 2014 10:01 PM
 To: Mcaulay, Alistair
 Cc: Daniel Vetter; Chris Wilson; Ben Widawsky; intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match 
 driver load  thaw
 
 On Wed, Jul 30, 2014 at 04:59:33PM +, Mcaulay, Alistair wrote:
  Hi Daniel,
  
  could you please be clearer on the change you mean.  I think you mean 
  something functionally equivalent to the code below, but done in a less 
  hacky way.
  (This slight change has made no change to test results) Or is the idea 
  to return at a different point to this?
  I couldn't find  dev_priv-mm.reload_in_reset or similar in the 
  code. The only thing I can find is error-reset_counter, which is used 
  in check_wedge(). Bottom bit set means RESET_IN_PROGRESS, top bit 
  means WEDGED
 
 Well I've meant that you have to add a new dev_prive-mm.realod_in_reset.
 And the below won't work since in all other places but when doing a gpu reset 
 we want the -EAGAIN to reach callers. Actually it's really important that if 
 we have an -EGAIN we don't eat it.
 
 And I guess the check for mm.reload_in_reset should actually be in 
 gem_check_wedged.
 -Daniel
 
  
  
   --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
   +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
   @@ -1832,7 +1832,9 @@ int intel_ring_begin(struct intel_engine_cs  
  *ring,

  ret = i915_gem_check_wedge(dev_priv-gpu_error,
 dev_priv-mm.interruptible);
   -  if (ret)
   +
   +  /* -EAGAIN means a reset is in progress, it is Ok to return */
   +  if (ret == -EAGAIN)
   +  return 0;
   +  if (ret)
   +  return ret;

  ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t));
  
  Alistair.
  
  -Original Message-
  From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On 
  Behalf Of Daniel Vetter
  Sent: Tuesday, July 29, 2014 11:33 AM
  To: Chris Wilson; Daniel Vetter; Ben Widawsky; 
  intel-gfx@lists.freedesktop.org
  Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence 
  to match driver load  thaw
  
  On Tue, Jul 29, 2014 at 08:36:33AM +0100, Chris Wilson wrote:
   On Mon, Jul 28, 2014 at 11:26:38AM +0200, Daniel Vetter wrote:
Oh, I guess that's the tricky bit why the old approach never 
worked
- because reset_in_progress is set we failed the context/ppgtt 
loading through the rings and screwed up.

Problem with your 

Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote:
 From: Sagar Kamble sagar.a.kam...@intel.com
 
 Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of 
 gunit registers need to be followed in
 PM suspend and resume path similar to runtime suspend and resume.
 
 v2:
 1. Keeping GT access, wake, gunit save/restore related helpers static.
 2. Moved GT access check, Wake Control, Gunit state save to end of 
 i915_drm_freeze.
 3. Reusing the sequence in runtime_suspend/resume path at macro level.
 
 Cc: Imre Deak imre.deak at intel.com
 Cc: Paulo Zanoni paulo.r.zanoni at intel.com
 Cc: Daniel Vetter daniel.vetter at ffwll.ch
 Cc: Jani Nikula jani.nikula at linux.intel.com
 Cc: Goel, Akash akash.g...@intel.com
 Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2
 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.c | 39 +--
  drivers/gpu/drm/i915/i915_drv.h |  1 +
  2 files changed, 34 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 6c4b25c..385dc74 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -490,11 +490,16 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
   return true;
  }
  
 +static int vlv_runtime_suspend(struct drm_i915_private *dev_priv);
 +static int vlv_runtime_resume(struct drm_i915_private *dev_priv,
 + bool resume_from_s0ix);
 +
  static int i915_drm_freeze(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_crtc *crtc;
   pci_power_t opregion_target_state;
 + int ret = 0;
  
   /* ignore lid events during suspend */
   mutex_lock(dev_priv-modeset_restore_lock);
 @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   intel_display_set_init_power(dev_priv, false);
  
 - return 0;
 + /* Save Gunit State and clear wake - Need to make sure
 +  * changes in vlv_runtime_suspend path don't impact this path */
 + if (IS_VALLEYVIEW(dev))
 + ret = vlv_runtime_suspend(dev_priv);

Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to
core resume/thaw code. This should be shovelled into the runtime pm
handling code, which should be reused in the suspend/resume code.

 +
 + return ret;
  }
  
  int i915_suspend(struct drm_device *dev, pm_message_t state)
 @@ -610,6 +620,12 @@ void intel_console_resume(struct work_struct *work)
  static int i915_drm_thaw_early(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 + int ret = 0;
 +
 + /* Restore Gunit State and allow wake - Need to make sure
 +  * changes in vlv_runtime_resume path don't impact this path */
 + if (IS_VALLEYVIEW(dev))
 + ret = vlv_runtime_resume(dev_priv, true);
  
   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
   hsw_disable_pc8(dev_priv);
 @@ -618,7 +634,7 @@ static int i915_drm_thaw_early(struct drm_device *dev)
   intel_uncore_sanitize(dev);
   intel_power_domains_init_hw(dev_priv);
  
 - return 0;
 + return ret;
  }
  
  static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 @@ -1098,6 +1114,7 @@ static void vlv_save_gunit_s0ix_state(struct 
 drm_i915_private *dev_priv)
   s-gu_ctl0  = I915_READ(VLV_GU_CTL0);
   s-gu_ctl1  = I915_READ(VLV_GU_CTL1);
   s-clock_gate_dis2  = I915_READ(VLV_GUNIT_CLOCK_GATE2);
 + s-dpio_cfg_data= I915_READ(DPIO_CTL);
  
   /*
* Not saving any of:
 @@ -1192,6 +1209,7 @@ static void vlv_restore_gunit_s0ix_state(struct 
 drm_i915_private *dev_priv)
   I915_WRITE(VLV_GU_CTL0, s-gu_ctl0);
   I915_WRITE(VLV_GU_CTL1, s-gu_ctl1);
   I915_WRITE(VLV_GUNIT_CLOCK_GATE2,   s-clock_gate_dis2);
 + I915_WRITE(DPIO_CTL,s-dpio_cfg_data);
  }
  
  int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
 @@ -1291,6 +1309,8 @@ static void vlv_check_no_gt_access(struct 
 drm_i915_private *dev_priv)
   I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
  }
  
 +/* This function is used in system suspend path as well to utilize
 + * Gfx clock, Wake control, Gunit state save related functionaility */
  static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
  {
   u32 mask;
 @@ -1331,7 +1351,12 @@ err1:
   return err;
  }
  
 -static int vlv_runtime_resume(struct drm_i915_private *dev_priv)
 +/* This function is used in system resume path as well to utilize
 + * Gfx clock, Wake control, Gunit state restore related functionaility.
 + * GEM and other initialization will differ which will be controlled by
 + * resume_from_s0ix variable */
 +static int vlv_runtime_resume(struct drm_i915_private *dev_priv,
 +  

Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: fix VDD state tracking after system resume

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 02:56:54PM +0300, Ville Syrjälä wrote:
 On Thu, Jul 31, 2014 at 02:03:36PM +0300, Imre Deak wrote:
  Just like during booting the BIOS can leave the VDD bit enabled after
  system resume. So apply the same state sanitization there too. This
  fixes a problem where after resume the port power domain refcount gets
  unbalanced.
  
  v2:
  - unchanged
  v3:
  - call edp sanitizing from the encoder reset handler (Daniel)
 
 It happens a bit earlier than with the earlier attempt, but if
 it works it works.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

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


Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote:
 On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote:
  On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote:
   On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote:
   Daniel, the only way intel_dp-is_mst can get reset is inside this path.
  
   Ok, so that one should be safe. Then I guess we can just push the
   locking down into the respective non-mst leafs (since atm we do
   link-retraining without any locking, which isn't good). And it needs
   to be dev-mode_config.mutex, not connection mutex.
 
 Why that? We can't be doing a modeset w/o connection_mutex so that
 seems like it should be enough. Well, there's also dpms which leaves
 the crtc-encoder-connector links intact but that too takes
 connection_mutex currently.
 
  
  I'd like to know why we do link training at this point though as well,
  adding locking is required of course, I was just going to wrap the
  short irq call to the link status check with the lock, but I think it
  should be possible to push it down further,
 
 I don't really know why the sink generates the hpd when we turn off the
 port, but that doesn't really matter I think. We need to be prepared for
 hpds at any time.
 
 intel_dp_check_link_status() just checks if there's a crtc, which there
 is (either the old one or the new one, depending on how far along the
 modeset path we are I guess), and then it just checks
 drm_dp_channel_eq_ok() which says false since the link was turned off,
 and then it proceeds to retrain the link.
 
 Maybe it should also check crtc-active? Though that itself won't 
 eliminate the problem unless the locking gets fixed somehow.

We check encoder-connectors_active, which is equivalent.
-Daniel

 
  
  I'm not sure how vague the spec is on what should happen on HPDs, but
  if we drop the port clock we obviously will lose the link, but we
  should also know not to be retraining it at that point anyways.
  
  Dave.
 
 -- 
 Ville Syrjälä
 Intel OTC

-- 
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: FBC flush nuke for BDW

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 05:07:30PM +0300, Ville Syrjälä wrote:
 On Thu, Jul 31, 2014 at 12:07:44PM -0700, Rodrigo Vivi wrote:
  According to spec FBC on BDW and HSW are identical without any gaps.
  So let's copy the nuke and let FBC really start compressing stuff.
  
  Without this patch we can verify with false color that nothing is being
  compressed. With the nuke in place and false color it is possible
  to see false color debugs.
  
  v2: Fix rebase conflict.
  
  Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
  ---
   drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++--
   1 file changed, 10 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
  b/drivers/gpu/drm/i915/intel_ringbuffer.c
  index 2908896..4ba3db1 100644
  --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
  +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
  @@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
   {
  u32 flags = 0;
  u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES;
  +   int ret;
   
  flags |= PIPE_CONTROL_CS_STALL;
   
  @@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
  flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
  }
   
  -   return gen8_emit_pipe_control(ring, flags, scratch_addr);
  +   ret = gen8_emit_pipe_control(ring, flags, scratch_addr);
  +   if (ret)
  +   return ret;
  +
  +   if (!invalidate_domains  flush_domains)
  +   return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
  +
  +   return 0;
   }
   
   static void ring_write_tail(struct intel_engine_cs *ring,
  @@ -2065,7 +2073,7 @@ static int gen6_ring_flush(struct intel_engine_cs 
  *ring,
  }
  intel_ring_advance(ring);
   
  -   if (IS_GEN7(dev)  !invalidate  flush)
  +   if (INTEL_INFO(dev)-gen = 7  !invalidate  flush)
  return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
 
 Apparently BDW has problems with LRIs to certain register offsets on
 !RCS and this here would hit that. The suggested workaround is to emit
 such LRIs only from RCS. Doing that would also involve tossing in a
 semaphore, so it feels like something that ought to be handled
 somewhere a bit higher up.

*cough* sw frontbuffer tracking *cough*

At least with fbc we have this nice igt test already.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Introduce FBC False Color for debug purposes.

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 02:04:45AM -0700, Rodrigo Vivi wrote:
 With this bit enabled, HW changes the color when compressing frames for
 debug purposes.
 
 ALthough the simple way to enable a single bit is over intel_reg_write,
 this value is overwriten on next update_fbc so depending on the workload
 it is not possible to set this bit with intel-gpu-tools. So this patch
 introduces a persistent way to enable false color over debugfs.
 
 v2: Use DEFINE_SIMPLE_ATTRIBUTE as Daniel suggested
 v3: (Ville) only do false color for IVB+ since according to spec bit is
 MBZ before IVB.
 v4: We don't have FBC on valleyview nor on cherryview (Ben)
 v5: s/!HAS_PCH_SPLIT/!HAS_FBC (Ville)
 
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 Reviewed-by: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com

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


Re: [Intel-gfx] [PATCH 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt

2014-08-04 Thread Daniel Vetter
On Thu, Jul 31, 2014 at 07:15:52PM +0300, Ville Syrjälä wrote:
 On Wed, Jul 30, 2014 at 09:42:02PM +0200, Daniel Vetter wrote:
  Stuffing this into the context setup code doesn't make a lot of sense.
  Also reusing the real ppgtt setup code makes even less sense since the
  aliasing ppgtt isn't a real address space. Leaving all that stuff
  unitialized will make sure that we catch any abusers promptly.
  
  This is also a prep work to clean up the context-ppgtt link.
  
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/i915_gem_context.c | 13 +
   drivers/gpu/drm/i915/i915_gem_gtt.c | 31 
  +--
   2 files changed, 26 insertions(+), 18 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
  b/drivers/gpu/drm/i915/i915_gem_context.c
  index 3b8367aa8404..7a455fcee3a7 100644
  --- a/drivers/gpu/drm/i915/i915_gem_context.c
  +++ b/drivers/gpu/drm/i915/i915_gem_context.c
  @@ -276,17 +276,6 @@ i915_gem_create_context(struct drm_device *dev,
  goto err_unpin;
  } else
  ctx-vm = ppgtt-base;
  -
  -   /* This case is reserved for the global default context and
  -* should only happen once. */
  -   if (is_global_default_ctx) {
  -   if (WARN_ON(dev_priv-mm.aliasing_ppgtt)) {
  -   ret = -EEXIST;
  -   goto err_unpin;
  -   }
  -
  -   dev_priv-mm.aliasing_ppgtt = ppgtt;
  -   }
  } else if (USES_PPGTT(dev)) {
  /* For platforms which only have aliasing PPGTT, we fake the
   * address space and refcounting. */
  @@ -361,7 +350,7 @@ int i915_gem_context_init(struct drm_device *dev)
  }
  }
   
  -   ctx = i915_gem_create_context(dev, NULL, USES_PPGTT(dev));
  +   ctx = i915_gem_create_context(dev, NULL, USES_FULL_PPGTT(dev));
  if (IS_ERR(ctx)) {
  DRM_ERROR(Failed to create default global context (error 
  %ld)\n,
PTR_ERR(ctx));
  diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
  b/drivers/gpu/drm/i915/i915_gem_gtt.c
  index 4fa7807ed4d5..cb9bb13ff20a 100644
  --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
  +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
  @@ -1191,23 +1191,27 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt 
  *ppgtt)
  return 0;
   }
   
  -int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
  +int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  -   int ret = 0;
   
  ppgtt-base.dev = dev;
  ppgtt-base.scratch = dev_priv-gtt.base.scratch;
   
  if (INTEL_INFO(dev)-gen  8)
  -   ret = gen6_ppgtt_init(ppgtt);
  +   return gen6_ppgtt_init(ppgtt);
  else if (IS_GEN8(dev))
  -   ret = gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total);
  +   return gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total);
  else
  BUG();
  +}
  +int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
  +{
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   int ret = 0;
   
  +   ret = __hw_ppgtt_init(dev, ppgtt);
  if (!ret) {
  -   struct drm_i915_private *dev_priv = dev-dev_private;
  kref_init(ppgtt-ref);
  drm_mm_init(ppgtt-base.mm, ppgtt-base.start,
  ppgtt-base.total);
  @@ -1728,6 +1732,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev,
  struct drm_mm_node *entry;
  struct drm_i915_gem_object *obj;
  unsigned long hole_start, hole_end;
  +   int ret;
   
  BUG_ON(mappable_end  end);
   
  @@ -1739,7 +1744,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev,
  /* Mark any preallocated objects as occupied */
  list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) {
  struct i915_vma *vma = i915_gem_obj_to_vma(obj, ggtt_vm);
  -   int ret;
  +
  DRM_DEBUG_KMS(reserving preallocated space: %lx + %zx\n,
i915_gem_obj_ggtt_offset(obj), obj-base.size);
   
  @@ -1766,6 +1771,20 @@ int i915_gem_setup_global_gtt(struct drm_device *dev,
  /* And finally clear the reserved guard page */
  ggtt_vm-clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true);
   
  +   if (HAS_ALIASING_PPGTT(dev)  USES_FULL_PPGTT(dev)) {
 
 Should that be !USES_FULL_PPGTT ?

Yup. And since I've also fumbled the check for __hw_ppgtt_init I really
wonder what exactly I've tested on my gen6 here ...

Time to hide and lick wounds I think ;-)
-Daniel

 
  +   struct i915_hw_ppgtt *ppgtt;
  +
  +   ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
  +   if (!ppgtt)
  +   return -ENOMEM;
  +
  +   ret = __hw_ppgtt_init(dev, ppgtt);
  +   if (!ret)
  +   return ret;
  +
  +  

Re: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV

2014-08-04 Thread Daniel Vetter
On Thu, Jul 31, 2014 at 11:55:58AM +, Mcaulay, Alistair wrote:
 Hi Jeff,
 
 Some more comments in the code.

Can you please fix your mailer to properly quote? Just doing in-line
comments makes it really hard to find them.
-Daniel

 
 Thanks,
 Alistair.
 
 -Original Message-
 From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
 jeff.mc...@intel.com
 Sent: Thursday, July 31, 2014 3:00 AM
 To: intel-gfx@lists.freedesktop.org
 Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
 
 From: Jeff McGee jeff.mc...@intel.com
 
 Cherryview can have different SSEU configurations within a given PCI ID, so 
 we collect the info from the fuse register.
 
 I don't currently have access to a CHV, much less one with an interesting 
 fuse config. So I have compile-checked this only!
 
 Signed-off-by: Jeff McGee jeff.mc...@intel.com
 ---
  drivers/gpu/drm/i915/Makefile |  3 +-
  drivers/gpu/drm/i915/i915_dma.c   |  2 ++
  drivers/gpu/drm/i915/i915_reg.h   | 13 
  drivers/gpu/drm/i915/intel_sseu.c | 64 
 +++
  drivers/gpu/drm/i915/intel_sseu.h |  2 ++
  5 files changed, 83 insertions(+), 1 deletion(-)  create mode 100644 
 drivers/gpu/drm/i915/intel_sseu.c
 
 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile 
 index 91bd167..9a0f411 100644
 --- a/drivers/gpu/drm/i915/Makefile
 +++ b/drivers/gpu/drm/i915/Makefile
 @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
 i915_irq.o \
 i915_trace_points.o \
 intel_ringbuffer.o \
 -   intel_uncore.o
 +   intel_uncore.o \
 +   intel_sseu.o
  
  # autogenerated null render state
  i915-y += intel_renderstate_gen6.o \
 diff --git a/drivers/gpu/drm/i915/i915_dma.c 
 b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   goto out_gem_unload;
   }
  
 + intel_sseu_init(dev);
 +
   intel_power_domains_init(dev_priv);
  
   if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git 
 a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 
 28e21ed..24a2d56 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -5624,6 +5624,19 @@ enum punit_power_well {
  #define GEN7_MISCCPCTL   (0x9424)
  #define   GEN7_DOP_CLOCK_GATE_ENABLE (10)
  
 +/* Fuse readout registers for GT */
 +#define CHV_FUSE_GT  0x182168
 +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0   10
 +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1   11
 
 These should be:
 #define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0(1  10)
 #define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1(1  11)
 
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK   (0xf16)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT  16
 
 Why not use the shift define here?
 #define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK
 (0xfCHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT)
 and for the others.
 
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK   (0xf20)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT  20
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK   (0xf24)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT  24
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK   (0xf28)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT  28
 +
  /* IVYBRIDGE DPF */
  #define GEN7_L3CDERRST1  0xB008 /* L3CD Error Status 1 */
  #define HSW_L3CDERRST11  0xB208 /* L3CD Error Status 
 register 1 slice 1 */
 diff --git a/drivers/gpu/drm/i915/intel_sseu.c 
 b/drivers/gpu/drm/i915/intel_sseu.c
 new file mode 100644
 index 000..6ba4830
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/intel_sseu.c
 @@ -0,0 +1,64 @@
 +/*
 + * Copyright © 2014 Intel Corporation
 + *
 + * Permission is hereby granted, free of charge, to any person 
 +obtaining a
 + * copy of this software and associated documentation files (the 
 +Software),
 + * to deal in the Software without restriction, including without 
 +limitation
 + * the rights to use, copy, modify, merge, publish, distribute, 
 +sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom 
 +the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the 
 +next
 + * paragraph) shall be included in all copies or substantial portions 
 +of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, 
 +EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 +MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
 +SHALL
 + 

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM

2014-08-04 Thread Daniel Vetter
On Wed, Jul 30, 2014 at 08:59:46PM -0500, jeff.mc...@intel.com wrote:
 From: Jeff McGee jeff.mc...@intel.com
 
 Define a struct to capture information on the device's Slice/Subslice/EU
 (SSEU) configuration. Add this struct to the main device info struct.
 Define a packed bitfield form for the SSEU info and share it with
 userspace via a new GETPARAM option.
 
 Starting with Cherryview, devices may have a varying number of EU for
 a given ID due to creative fusing. The surest way to determine the
 configuration is by reading fuses which is best done in the kernel and
 communicated to userspace. The immediate need from userspace is to
 determine the number of threads of compute work that can be safely
 submitted.
 
 The definition of SSEU as a new drm/i915 component, with its own header
 file and soon-to-be source file, is in anticipation of lots of upcoming
 code for its management, particularly the power gating functionality.
 
 Signed-off-by: Jeff McGee jeff.mc...@intel.com
 ---
  drivers/gpu/drm/i915/i915_dma.c   |  3 +++
  drivers/gpu/drm/i915/i915_drv.h   |  3 +++
  drivers/gpu/drm/i915/intel_sseu.h | 40 
 +++
  include/uapi/drm/i915_drm.h   | 18 ++
  4 files changed, 64 insertions(+)
  create mode 100644 drivers/gpu/drm/i915/intel_sseu.h
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 2e7f03a..f581848 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void 
 *data,
   case I915_PARAM_CMD_PARSER_VERSION:
   value = i915_cmd_parser_get_version();
   break;
 + case I915_PARAM_SSEU_INFO:
 + value = INTEL_INFO(dev)-sseu.gp_sseu_info;
 + break;
   default:
   DRM_DEBUG(Unknown parameter %d\n, param-param);
   return -EINVAL;
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 18c9ad8..01adafd 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -45,6 +45,7 @@
  #include linux/intel-iommu.h
  #include linux/kref.h
  #include linux/pm_qos.h
 +#include intel_sseu.h
  
  /* General customization:
   */
 @@ -562,6 +563,8 @@ struct intel_device_info {
   int trans_offsets[I915_MAX_TRANSCODERS];
   int palette_offsets[I915_MAX_PIPES];
   int cursor_offsets[I915_MAX_PIPES];
 + /* Slice/Subslice/EU info */
 + struct intel_sseu_info sseu;
  };
  
  #undef DEFINE_FLAG
 diff --git a/drivers/gpu/drm/i915/intel_sseu.h 
 b/drivers/gpu/drm/i915/intel_sseu.h
 new file mode 100644
 index 000..7db7175
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/intel_sseu.h
 @@ -0,0 +1,40 @@
 +/*
 + * Copyright © 2014 Intel Corporation
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the next
 + * paragraph) shall be included in all copies or substantial portions of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
 DEALINGS
 + * IN THE SOFTWARE.
 + *
 + */
 +#ifndef _INTEL_SSEU_H_
 +#define _INTEL_SSEU_H_
 +
 +struct intel_sseu_info {
 + /* Total slice count */
 + unsigned int slice_cnt;
 + /* Total subslice count */
 + unsigned int subslice_cnt;
 + /* Total execution unit count */
 + unsigned int eu_cnt;
 + /* Thread count per EU */
 + unsigned int threads_per_eu;
 + /* Bit field representation for I915_PARAM_SSEU_INFO */
 + u32 gp_sseu_info;
 +};
 +
 +#endif
 diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
 index ff57f07..b99c1a2 100644
 --- a/include/uapi/drm/i915_drm.h
 +++ b/include/uapi/drm/i915_drm.h
 @@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea {
  #define I915_BOX_TEXTURE_LOAD  0x8
  #define I915_BOX_LOST_CONTEXT  0x10
  
 +/*
 + * Slice/Subslice/EU Info
 + * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO
 + * - SLICE_CNT: total slice count
 + * - SUBSLICE_CNT: total subslice count
 + * - EU_CNT: total execution unit count
 + * - THREADS_PER_EU: thread count per EU

Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()

2014-08-04 Thread Dave Airlie
On 4 August 2014 18:10, Daniel Vetter dan...@ffwll.ch wrote:
 On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote:
 On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote:
  On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote:
   On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote:
   Daniel, the only way intel_dp-is_mst can get reset is inside this path.
  
   Ok, so that one should be safe. Then I guess we can just push the
   locking down into the respective non-mst leafs (since atm we do
   link-retraining without any locking, which isn't good). And it needs
   to be dev-mode_config.mutex, not connection mutex.

 Why that? We can't be doing a modeset w/o connection_mutex so that
 seems like it should be enough. Well, there's also dpms which leaves
 the crtc-encoder-connector links intact but that too takes
 connection_mutex currently.

 
  I'd like to know why we do link training at this point though as well,
  adding locking is required of course, I was just going to wrap the
  short irq call to the link status check with the lock, but I think it
  should be possible to push it down further,

 I don't really know why the sink generates the hpd when we turn off the
 port, but that doesn't really matter I think. We need to be prepared for
 hpds at any time.

 intel_dp_check_link_status() just checks if there's a crtc, which there
 is (either the old one or the new one, depending on how far along the
 modeset path we are I guess), and then it just checks
 drm_dp_channel_eq_ok() which says false since the link was turned off,
 and then it proceeds to retrain the link.

 Maybe it should also check crtc-active? Though that itself won't
 eliminate the problem unless the locking gets fixed somehow.

 We check encoder-connectors_active, which is equivalent.
 -Daniel

We still check that. if you mean intel_encoders-connectors_active.

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV

2014-08-04 Thread Daniel Vetter
On Wed, Jul 30, 2014 at 08:59:47PM -0500, jeff.mc...@intel.com wrote:
 From: Jeff McGee jeff.mc...@intel.com
 
 Cherryview can have different SSEU configurations within a given PCI
 ID, so we collect the info from the fuse register.
 
 I don't currently have access to a CHV, much less one with an interesting
 fuse config. So I have compile-checked this only!
 
 Signed-off-by: Jeff McGee jeff.mc...@intel.com
 ---
  drivers/gpu/drm/i915/Makefile |  3 +-
  drivers/gpu/drm/i915/i915_dma.c   |  2 ++
  drivers/gpu/drm/i915/i915_reg.h   | 13 
  drivers/gpu/drm/i915/intel_sseu.c | 64 
 +++
  drivers/gpu/drm/i915/intel_sseu.h |  2 ++
  5 files changed, 83 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/i915/intel_sseu.c
 
 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
 index 91bd167..9a0f411 100644
 --- a/drivers/gpu/drm/i915/Makefile
 +++ b/drivers/gpu/drm/i915/Makefile
 @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
 i915_irq.o \
 i915_trace_points.o \
 intel_ringbuffer.o \
 -   intel_uncore.o
 +   intel_uncore.o \
 +   intel_sseu.o
  
  # autogenerated null render state
  i915-y += intel_renderstate_gen6.o \
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index f581848..384ef65 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   goto out_gem_unload;
   }
  
 + intel_sseu_init(dev);
 +
   intel_power_domains_init(dev_priv);
  
   if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 28e21ed..24a2d56 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -5624,6 +5624,19 @@ enum punit_power_well {
  #define GEN7_MISCCPCTL   (0x9424)
  #define   GEN7_DOP_CLOCK_GATE_ENABLE (10)
  
 +/* Fuse readout registers for GT */
 +#define CHV_FUSE_GT  0x182168
 +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS0   10
 +#define   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1   11
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK   (0xf16)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT  16
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK   (0xf20)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT  20
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK   (0xf24)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT  24
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK   (0xf28)
 +#define   CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT  28
 +
  /* IVYBRIDGE DPF */
  #define GEN7_L3CDERRST1  0xB008 /* L3CD Error Status 1 */
  #define HSW_L3CDERRST11  0xB208 /* L3CD Error Status 
 register 1 slice 1 */
 diff --git a/drivers/gpu/drm/i915/intel_sseu.c 
 b/drivers/gpu/drm/i915/intel_sseu.c
 new file mode 100644
 index 000..6ba4830
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/intel_sseu.c
 @@ -0,0 +1,64 @@
 +/*
 + * Copyright © 2014 Intel Corporation
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the next
 + * paragraph) shall be included in all copies or substantial portions of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
 DEALINGS
 + * IN THE SOFTWARE.
 + *
 + */
 +#include linux/bitops.h
 +#include i915_drv.h
 +#include intel_sseu.h
 +
 +void intel_sseu_init(struct drm_device *dev)
 +{
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct intel_sseu_info sseu_info;
 + u32 gp = 0;
 +
 + /* Collect SSEU info per device */
 + if (IS_CHERRYVIEW(dev)) {
 + u32 fuse, mask_ss, mask_eu;
 +
 + fuse = I915_READ(CHV_FUSE_GT);
 + mask_ss = fuse  (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
 +   CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
 +

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Remove now useless comments about the translation values

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 12:29:22PM -0300, Paulo Zanoni wrote:
 2014-08-01 7:07 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
  We used to carry a default HDMI value in entry 9, but this entry got
  removed for both HSW and BDW.
 
 
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

Pulled in entire series, thanks. I'll shuffle the first two for 3.17 when
I split my queue. We can cc: stable if they are known to make an actual
user happier ;-)
-Daniel
 
  Signed-off-by: Damien Lespiau damien.lesp...@intel.com
  ---
   drivers/gpu/drm/i915/intel_ddi.c | 10 +-
   1 file changed, 5 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
  b/drivers/gpu/drm/i915/intel_ddi.c
  index 80526ab..ab5f65f 100644
  --- a/drivers/gpu/drm/i915/intel_ddi.c
  +++ b/drivers/gpu/drm/i915/intel_ddi.c
  @@ -33,7 +33,7 @@
* automatically adapt to HDMI connections as well
*/
   static const u32 hsw_ddi_translations_dp[] = {
  -   0x00FF, 0x0006000E, /* DP parameters */
  +   0x00FF, 0x0006000E,
  0x00D75FFF, 0x0005000A,
  0x00C30FFF, 0x00040006,
  0x80AAAFFF, 0x000B,
  @@ -45,7 +45,7 @@ static const u32 hsw_ddi_translations_dp[] = {
   };
 
   static const u32 hsw_ddi_translations_fdi[] = {
  -   0x00FF, 0x0007000E, /* FDI parameters */
  +   0x00FF, 0x0007000E,
  0x00D75FFF, 0x000F000A,
  0x00C30FFF, 0x00060006,
  0x00AAAFFF, 0x001E,
  @@ -73,7 +73,7 @@ static const u32 hsw_ddi_translations_hdmi[] = {
   };
 
   static const u32 bdw_ddi_translations_edp[] = {
  -   0x00FF, 0x0012, /* eDP parameters */
  +   0x00FF, 0x0012,
  0x00EBAFFF, 0x00020011,
  0x00C71FFF, 0x0006000F,
  0x00AAAFFF, 0x000E000A,
  @@ -85,7 +85,7 @@ static const u32 bdw_ddi_translations_edp[] = {
   };
 
   static const u32 bdw_ddi_translations_dp[] = {
  -   0x00FF, 0x0007000E, /* DP parameters */
  +   0x00FF, 0x0007000E,
  0x00D75FFF, 0x000E000A,
  0x00BE, 0x00140006,
  0x80B2CFFF, 0x001B0002,
  @@ -97,7 +97,7 @@ static const u32 bdw_ddi_translations_dp[] = {
   };
 
   static const u32 bdw_ddi_translations_fdi[] = {
  -   0x00FF, 0x0001000E, /* FDI parameters */
  +   0x00FF, 0x0001000E,
  0x00D75FFF, 0x0004000A,
  0x00C30FFF, 0x00070006,
  0x00AAAFFF, 0x000C,
  --
  1.8.3.1
 
  ___
  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

-- 
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] tests: Add drv_workarounds

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 08:46:25PM +0300, Mika Kuoppala wrote:
 To check that static workarounds are set and stay after init,
 hang and suspend/restore. Checks are currently provided for ivb
 and bdw only.
 
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
 ---
  lib/intel_chipset.h |4 +
  lib/intel_workaround.h  |  142 +++
  tests/Makefile.sources  |1 +
  tests/drv_workarounds.c |  244 
 +++
  4 files changed, 391 insertions(+)
  create mode 100644 lib/intel_workaround.h
  create mode 100644 tests/drv_workarounds.c
 
 diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
 index 404c632..5a03f2b 100644
 --- a/lib/intel_chipset.h
 +++ b/lib/intel_chipset.h
 @@ -263,6 +263,10 @@ void intel_check_pch(void);
(devid) == PCI_CHIP_IVYBRIDGE_S || \
(devid) == PCI_CHIP_IVYBRIDGE_S_GT2)
  
 +#define IS_IVB_GT1(devid)((devid) == PCI_CHIP_IVYBRIDGE_GT1 || \
 +  (devid) == PCI_CHIP_IVYBRIDGE_M_GT1 || \
 +  (devid) == PCI_CHIP_IVYBRIDGE_S)
 +
  #define IS_VALLEYVIEW(devid) ((devid) == PCI_CHIP_VALLEYVIEW_PO || \
(devid) == PCI_CHIP_VALLEYVIEW_1 || \
(devid) == PCI_CHIP_VALLEYVIEW_2 || \
 diff --git a/lib/intel_workaround.h b/lib/intel_workaround.h
 new file mode 100644
 index 000..87e3a44
 --- /dev/null
 +++ b/lib/intel_workaround.h
 @@ -0,0 +1,142 @@
 +/*
 + * Copyright © 2013,2014 Intel Corporation
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the next
 + * paragraph) shall be included in all copies or substantial portions of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
 DEALINGS
 + * IN THE SOFTWARE.
 + *
 + * Authors:
 + *Mika Kuoppala mika.kuopp...@intel.com
 + *
 + */
 +
 +#ifndef INTEL_WORKAROUNDS
 +#define INTEL_WORKAROUNDS
 +
 +#include intel_chipset.h
 +#include intel_io.h
 +#include igt_core.h
 +
 +static int __wa_devid = 0;
 +
 +struct wa {
 + const char * const name;
 + int (*f)(const int devid);
 +};
 +
 +static void wa_init(const int devid)
 +{
 + __wa_devid = devid;
 + intel_register_access_init(intel_get_pci_device(), 0);
 +}
 +
 +static void wa_fini(void)
 +{
 + __wa_devid = 0;
 + intel_register_access_fini();
 +}
 +
 +static int wa_check(const struct wa *wa)
 +{
 + igt_assert(__wa_devid);
 + igt_assert(wa-name);
 + igt_assert(wa-f);
 + return wa-f(__wa_devid);
 +}
 +
 +#define WA(_name) /* _name */ static int _name##_fun(const int devid);   
 \
 + static const struct wa _name =  \
 + { #_name, _name##_fun };\
 + static int _name##_fun(const int devid)
 +
 +#define wa_assert_m(reg, val, mask) {
 \
 + unsigned int x; \
 + igt_assert(mask);   \
 + x = intel_register_read(reg);   \
 + if (((x)  (mask)) != (val)) {  \
 + igt_warn(a:0x%08x r:0x%08x m:0%08x v:0%08x\n, reg, x, 
 mask, val); \
 + return 1;   \
 + }   \
 + }
 +
 +#define wa_assert(reg, val) wa_assert_m(reg, val, 0x)
 +#define wa_assert_bit_set(reg, bit) wa_assert_m(reg, (1  bit), (1  bit))
 +#define wa_assert_bit_clr(reg, bit) wa_assert_m(reg, 0, (1  bit))
 +
 +#define WA_MASK(_name, reg, val, mask)   \
 + WA(_name) { \
 + wa_assert_m(reg, val, mask);\
 + return 0;   \
 + }
 +
 +#define WA_BIT_SET(_name, reg, bit) WA_MASK(_name, reg, (1  bit), (1  
 bit))
 +#define WA_BIT_CLR(_name, reg, bit) WA_MASK(_name, reg, 0, (1  bit))
 +
 +
 +/* 

[Intel-gfx] [PATCH] drm/i915: Don't require dev-struct_mutex in psr_match_conditions

2014-08-04 Thread Daniel Vetter
Since I've reworked psr support to no longer require x-tiling we don't
check any state protected by the Giant GEM Lock. So drop that check.

Also boo for lockdep_assert_held for not yelling when lockdep is
disabled.

Cc: Paulo Zanoni przan...@gmail.com
Reported-by: Paulo Zanoni przan...@gmail.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_dp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3e6100ea7295..6dbe0f84d455 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1773,7 +1773,6 @@ static bool intel_edp_psr_match_conditions(struct 
intel_dp *intel_dp)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
lockdep_assert_held(dev_priv-psr.lock);
-   lockdep_assert_held(dev-struct_mutex);
WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex));
WARN_ON(!drm_modeset_is_locked(crtc-mutex));
 
-- 
2.0.1

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


Re: [Intel-gfx] [PATCH] drm/i915: remove duplicate register defines

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 04:19:54PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 cat i915_reg.h | sort | uniq -d | grep define
 
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com

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

 ---
  drivers/gpu/drm/i915/i915_reg.h | 2 --
  1 file changed, 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 28e21ed..c45b679 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -3755,7 +3755,6 @@ enum punit_power_well {
  #define   PIPE_VSYNC_INTERRUPT_STATUS(1UL9)
  #define   PIPE_DISPLAY_LINE_COMPARE_STATUS   (1UL8)
  #define   PIPE_DPST_EVENT_STATUS (1UL7)
 -#define   PIPE_LEGACY_BLC_EVENT_STATUS   (1UL6)
  #define   PIPE_A_PSR_STATUS_VLV  (1UL6)
  #define   PIPE_LEGACY_BLC_EVENT_STATUS   (1UL6)
  #define   PIPE_ODD_FIELD_INTERRUPT_STATUS(1UL5)
 @@ -5430,7 +5429,6 @@ enum punit_power_well {
  #define   VLV_GTLC_ALLOWWAKEERR  (1  1)
  #define   VLV_GTLC_PW_MEDIA_STATUS_MASK  (1  5)
  #define   VLV_GTLC_PW_RENDER_STATUS_MASK (1  7)
 -#define VLV_GTLC_SURVIVABILITY_REG  0x130098
  #define  FORCEWAKE_MT0xa188 /* 
 multi-threaded */
  #define   FORCEWAKE_KERNEL   0x1
  #define   FORCEWAKE_USER 0x2
 -- 
 2.0.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] [PATCH] intel-gpu-tools: Change type of variable from unsigned to uint64_t in gem_stress

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 12:10:44PM +0700, Pavel Popov wrote:
 To run gem_stress with the correct number of buffers even if aperture size = 
 4GB.
 
 Signed-off-by: Pavel Popov pavel.e.po...@intel.com
 ---
  tests/gem_stress.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/tests/gem_stress.c b/tests/gem_stress.c
 index 3bbe487..313a82a 100644
 --- a/tests/gem_stress.c
 +++ b/tests/gem_stress.c
 @@ -737,7 +737,7 @@ static int parse_options(int opt, int opt_index)
  static void init(void)
  {
   int i;
 - unsigned tmp;
 + uint64_t tmp;

We have tons of places which use gem_aperture_size. Have you reviewed them
all?
-Daniel

  
   if (options.num_buffers == 0) {
   tmp = gem_aperture_size(drm_fd);
 -- 
 1.9.1
 
 
 
 Closed Joint Stock Company Intel A/O
 Registered legal address: Krylatsky Hills Business Park, 
 17 Krylatskaya Str., Bldg 4, Moscow 121614, 
 Russian Federation
 
 This e-mail and any attachments may contain confidential material for
 the sole use of the intended recipient(s). Any review or distribution
 by others is strictly prohibited. If you are not the intended
 recipient, please contact the sender and delete all copies.
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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: Add rotation_property to mode_config and creating it

2014-08-04 Thread Jindal, Sonika



On 7/31/2014 9:39 AM, Jindal, Sonika wrote:



On 7/29/2014 4:00 PM, Daniel Vetter wrote:

On Tue, Jul 29, 2014 at 12:40:29PM +0300, Ville Syrjälä wrote:

On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote:

On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote:

On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote:

From: Sonika Jindal sonika.jin...@intel.com

v2: Adding creation of rotation_property here.

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
   drivers/gpu/drm/drm_crtc.c |3 ++-
   include/drm/drm_crtc.h |1 +
   2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 787631e..49c0747 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1299,7 +1299,8 @@ static int 
drm_mode_create_standard_plane_properties(struct drm_device *dev)
type, drm_plane_type_enum_list,
ARRAY_SIZE(drm_plane_type_enum_list));
dev-mode_config.plane_type_property = type;
-
+   dev-mode_config.rotation_property = 
drm_mode_create_rotation_property(dev,
+   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180));


This might not make sense for other (!i915) hardware. And that's the
reason why I had the driver create the property in the first place.

I think Daniel was thinking that we might want to expose all the bits
regardless of what the hardware supports, but I don't like that idea.
There are other properties (eg. alpha blending, csc stuff, etc.) that
have the same problem of hardware supporting only a (potentially small)
subset of the possible values. I'd rather we didn't make life harder
for userspace when the kernel can already report that certain values
will never work.


Well I'd like the property to be in some generic place so that fbcon can
unroate and that with the atomic stuff we can have rotation support in the
core structures. Which should help with argument checking.

But for rotation I don't think we should set it up in generic code, but in
i915. So the place where we keep it would be generic, the values would be
the sames, but the allowed set would differ depending upon platform or
driver.


That would still fail if all the planes on the same device don't support
the same rotation flags. Eg. on i915 we would have this problem if we
exposed the old video overlay as a drm plane. And it wouldn't be the
first piece of hardware where I've seen this kind of thing.


Problem is still that I'd like to have a somewhat generic internal
representation available. We could punt this to atomic though by adding a
rotation field to the drm_plane_state structure. Which is more-or-less my
plan, but wouldn't work with Rob's approach.

Or we keep the property link only in drm_plane (and give drivers the
freedom to set up the underlying enum however they want to), but I'm not
sure whether our interfaces can cope with that.
-Daniel


Daniel, Ville

So what is the suggestion for this property? Should I be moving it to
somewhere else?

-Sonika

Hi Daniel/Ville,

Please let me know if I need to move this property somewhere else.

Thanks,
Sonika

___
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] drm: Add rotation_property to mode_config and creating it

2014-08-04 Thread Ville Syrjälä
On Mon, Aug 04, 2014 at 03:59:23PM +0530, Jindal, Sonika wrote:
 
 
 On 7/31/2014 9:39 AM, Jindal, Sonika wrote:
 
 
  On 7/29/2014 4:00 PM, Daniel Vetter wrote:
  On Tue, Jul 29, 2014 at 12:40:29PM +0300, Ville Syrjälä wrote:
  On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote:
  On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote:
  On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote:
  From: Sonika Jindal sonika.jin...@intel.com
 
  v2: Adding creation of rotation_property here.
 
  Signed-off-by: Sonika Jindal sonika.jin...@intel.com
  ---
 drivers/gpu/drm/drm_crtc.c |3 ++-
 include/drm/drm_crtc.h |1 +
 2 files changed, 3 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
  index 787631e..49c0747 100644
  --- a/drivers/gpu/drm/drm_crtc.c
  +++ b/drivers/gpu/drm/drm_crtc.c
  @@ -1299,7 +1299,8 @@ static int 
  drm_mode_create_standard_plane_properties(struct drm_device *dev)
 type, 
  drm_plane_type_enum_list,
 
  ARRAY_SIZE(drm_plane_type_enum_list));
 dev-mode_config.plane_type_property = type;
  -
  +  dev-mode_config.rotation_property = 
  drm_mode_create_rotation_property(dev,
  +  BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180));
 
  This might not make sense for other (!i915) hardware. And that's the
  reason why I had the driver create the property in the first place.
 
  I think Daniel was thinking that we might want to expose all the bits
  regardless of what the hardware supports, but I don't like that idea.
  There are other properties (eg. alpha blending, csc stuff, etc.) that
  have the same problem of hardware supporting only a (potentially small)
  subset of the possible values. I'd rather we didn't make life harder
  for userspace when the kernel can already report that certain values
  will never work.
 
  Well I'd like the property to be in some generic place so that fbcon can
  unroate and that with the atomic stuff we can have rotation support in 
  the
  core structures. Which should help with argument checking.
 
  But for rotation I don't think we should set it up in generic code, but 
  in
  i915. So the place where we keep it would be generic, the values would be
  the sames, but the allowed set would differ depending upon platform or
  driver.
 
  That would still fail if all the planes on the same device don't support
  the same rotation flags. Eg. on i915 we would have this problem if we
  exposed the old video overlay as a drm plane. And it wouldn't be the
  first piece of hardware where I've seen this kind of thing.
 
  Problem is still that I'd like to have a somewhat generic internal
  representation available. We could punt this to atomic though by adding a
  rotation field to the drm_plane_state structure. Which is more-or-less my
  plan, but wouldn't work with Rob's approach.
 
  Or we keep the property link only in drm_plane (and give drivers the
  freedom to set up the underlying enum however they want to), but I'm not
  sure whether our interfaces can cope with that.
  -Daniel
 
  Daniel, Ville
 
  So what is the suggestion for this property? Should I be moving it to
  somewhere else?
 
  -Sonika
 Hi Daniel/Ville,
 
 Please let me know if I need to move this property somewhere else.

Well we at least need to move the crationg of the property to the
driver. I guess we could leave the property itself in place in
mode_config so that fbdev can get at it easily. If we come across
a real need for separate per-plane rotation properties we can always
move it to drm_plane later.

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


Re: [Intel-gfx] [PATCH 6/6] drm: Resetting rotation property

2014-08-04 Thread Ville Syrjälä
On Tue, Jul 15, 2014 at 02:10:29PM +0530, sonika.jin...@intel.com wrote:
 From: Sonika Jindal sonika.jin...@intel.com
 
 Reset rotation property to 0 wherever applicable
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 ---
  drivers/gpu/drm/drm_fb_helper.c |   10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
 index 3144db9..961afcb 100644
 --- a/drivers/gpu/drm/drm_fb_helper.c
 +++ b/drivers/gpu/drm/drm_fb_helper.c
 @@ -345,9 +345,17 @@ static bool restore_fbdev_mode(struct drm_fb_helper 
 *fb_helper)
  
   drm_warn_on_modeset_not_all_locked(dev);
  
 - list_for_each_entry(plane, dev-mode_config.plane_list, head)
 + list_for_each_entry(plane, dev-mode_config.plane_list, head) {
 +
 + if (dev-mode_config.rotation_property) {
 + drm_object_property_set_value(plane-base,
 + dev-mode_config.rotation_property,
 + BIT(DRM_ROTATE_0));
 + }
 +
   if (plane-type != DRM_PLANE_TYPE_PRIMARY)
   drm_plane_force_disable(plane);

I would reset the property after disabling the plane.

 + }
  
   for (i = 0; i  fb_helper-crtc_count; i++) {
   struct drm_mode_set *mode_set = 
 fb_helper-crtc_info[i].mode_set;
 -- 
 1.7.10.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 6/6] drm: Resetting rotation property

2014-08-04 Thread Jindal, Sonika



On 8/4/2014 5:31 PM, Ville Syrjälä wrote:

On Tue, Jul 15, 2014 at 02:10:29PM +0530, sonika.jin...@intel.com wrote:

From: Sonika Jindal sonika.jin...@intel.com

Reset rotation property to 0 wherever applicable

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
  drivers/gpu/drm/drm_fb_helper.c |   10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3144db9..961afcb 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -345,9 +345,17 @@ static bool restore_fbdev_mode(struct drm_fb_helper 
*fb_helper)

drm_warn_on_modeset_not_all_locked(dev);

-   list_for_each_entry(plane, dev-mode_config.plane_list, head)
+   list_for_each_entry(plane, dev-mode_config.plane_list, head) {
+
+   if (dev-mode_config.rotation_property) {
+   drm_object_property_set_value(plane-base,
+   dev-mode_config.rotation_property,
+   BIT(DRM_ROTATE_0));
+   }
+
if (plane-type != DRM_PLANE_TYPE_PRIMARY)
drm_plane_force_disable(plane);


I would reset the property after disabling the plane.

Ok, I will change that.



+   }

for (i = 0; i  fb_helper-crtc_count; i++) {
struct drm_mode_set *mode_set = 
fb_helper-crtc_info[i].mode_set;
--
1.7.10.4

___
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] drm: Add rotation_property to mode_config and creating it

2014-08-04 Thread Jindal, Sonika



On 8/4/2014 5:24 PM, Ville Syrjälä wrote:

On Mon, Aug 04, 2014 at 03:59:23PM +0530, Jindal, Sonika wrote:



On 7/31/2014 9:39 AM, Jindal, Sonika wrote:



On 7/29/2014 4:00 PM, Daniel Vetter wrote:

On Tue, Jul 29, 2014 at 12:40:29PM +0300, Ville Syrjälä wrote:

On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote:

On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote:

On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote:

From: Sonika Jindal sonika.jin...@intel.com

v2: Adding creation of rotation_property here.

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/drm_crtc.c |3 ++-
include/drm/drm_crtc.h |1 +
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 787631e..49c0747 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1299,7 +1299,8 @@ static int 
drm_mode_create_standard_plane_properties(struct drm_device *dev)
type, drm_plane_type_enum_list,
ARRAY_SIZE(drm_plane_type_enum_list));
dev-mode_config.plane_type_property = type;
-
+   dev-mode_config.rotation_property = 
drm_mode_create_rotation_property(dev,
+   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180));


This might not make sense for other (!i915) hardware. And that's the
reason why I had the driver create the property in the first place.

I think Daniel was thinking that we might want to expose all the bits
regardless of what the hardware supports, but I don't like that idea.
There are other properties (eg. alpha blending, csc stuff, etc.) that
have the same problem of hardware supporting only a (potentially small)
subset of the possible values. I'd rather we didn't make life harder
for userspace when the kernel can already report that certain values
will never work.


Well I'd like the property to be in some generic place so that fbcon can
unroate and that with the atomic stuff we can have rotation support in the
core structures. Which should help with argument checking.

But for rotation I don't think we should set it up in generic code, but in
i915. So the place where we keep it would be generic, the values would be
the sames, but the allowed set would differ depending upon platform or
driver.


That would still fail if all the planes on the same device don't support
the same rotation flags. Eg. on i915 we would have this problem if we
exposed the old video overlay as a drm plane. And it wouldn't be the
first piece of hardware where I've seen this kind of thing.


Problem is still that I'd like to have a somewhat generic internal
representation available. We could punt this to atomic though by adding a
rotation field to the drm_plane_state structure. Which is more-or-less my
plan, but wouldn't work with Rob's approach.

Or we keep the property link only in drm_plane (and give drivers the
freedom to set up the underlying enum however they want to), but I'm not
sure whether our interfaces can cope with that.
-Daniel


Daniel, Ville

So what is the suggestion for this property? Should I be moving it to
somewhere else?

-Sonika

Hi Daniel/Ville,

Please let me know if I need to move this property somewhere else.


Well we at least need to move the crationg of the property to the
driver. I guess we could leave the property itself in place in
mode_config so that fbdev can get at it easily. If we come across
a real need for separate per-plane rotation properties we can always
move it to drm_plane later.


Ok, I will move it back to plane_create function.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()

2014-08-04 Thread Ville Syrjälä
On Mon, Aug 04, 2014 at 10:10:50AM +0200, Daniel Vetter wrote:
 On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote:
  On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote:
   On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote:
On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote:
Daniel, the only way intel_dp-is_mst can get reset is inside this 
path.
   
Ok, so that one should be safe. Then I guess we can just push the
locking down into the respective non-mst leafs (since atm we do
link-retraining without any locking, which isn't good). And it needs
to be dev-mode_config.mutex, not connection mutex.
  
  Why that? We can't be doing a modeset w/o connection_mutex so that
  seems like it should be enough. Well, there's also dpms which leaves
  the crtc-encoder-connector links intact but that too takes
  connection_mutex currently.
  
   
   I'd like to know why we do link training at this point though as well,
   adding locking is required of course, I was just going to wrap the
   short irq call to the link status check with the lock, but I think it
   should be possible to push it down further,
  
  I don't really know why the sink generates the hpd when we turn off the
  port, but that doesn't really matter I think. We need to be prepared for
  hpds at any time.
  
  intel_dp_check_link_status() just checks if there's a crtc, which there
  is (either the old one or the new one, depending on how far along the
  modeset path we are I guess), and then it just checks
  drm_dp_channel_eq_ok() which says false since the link was turned off,
  and then it proceeds to retrain the link.
  
  Maybe it should also check crtc-active? Though that itself won't 
  eliminate the problem unless the locking gets fixed somehow.
 
 We check encoder-connectors_active, which is equivalent.

Only after intel_sanitize_encoder(). Before that we can have
!crtc-active  encoder-connectors_active.

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


Re: [Intel-gfx] [PATCH 1/1] Documentation: drm: describing drm properties exposed by various drivers

2014-08-04 Thread Laurent Pinchart
On Monday 04 August 2014 09:30:04 Daniel Vetter wrote:
 On Fri, Aug 01, 2014 at 02:58:21PM +0200, Laurent Pinchart wrote:
  On Thursday 31 July 2014 15:16:21 Randy Dunlap wrote:
  On 05/12/14 11:04, Randy Dunlap wrote:
  On 05/12/2014 08:54 AM, Daniel Vetter wrote:
  On Mon, May 12, 2014 at 08:23:45AM -0700, Randy Dunlap wrote:
  On 05/12/2014 01:58 AM, Daniel Vetter wrote:
  On Mon, May 12, 2014 at 06:24:57PM +1000, Dave Airlie wrote:
  If we decide to go for property documentation inside the source
  code then I believe we'll have to create our own format, as
  creating a properties table from kerneldoc information extracted
  from comments is probably not possible.
  
  Can comeone pick up the ball here and figure out what needs to be
  done?
  
  The reason why I want a central place for the documentation is to
  force people to collaborate outside their own sandbox when adding
  properties. Whether that's docbook or some text file I don't care
  so much at this point. The fact that it's a central place should
  mandate that the patches changing it will go through dri-devel
  and so everyone should se them, and when adding new properties it
  would make the patch author more likely to look around a bit
  before adding another slighty incompatible version of the same
  property. If someone has a better suggestion how to encforce this
  I'm all ears.
  
  Of course this idea can still fail if our esteemed maintainer
  merges stuff without checking for violations of this policy.
  Dave, any thoughts on the subject?
  
  Yeah I'm happy to block merging stuff, if we can spot new 
  properties when stuff is posted on dri-devel, so much the better,
  
  most drivers still send everything via dri-devel anyways, its only
  really Intel I have to worry about so far,
  
  I'll enforce that all prop stuff gets cc: dri-devel and that it has
  updates for the prop docs.
  
  But we should definitely add it to the new driver review checklist
  as well.
  
  I'm also on the side of this patch is ugly and makes my eyes burn,
  please please get a plan to use something else ASAP, I'm willing
  to merge this but I'm tempted to give it a lifetime of a kernel or
  two before I burn it.
  
  Ok, I'll try to move make kerneldoc suck less up the task list
  and maybe find someone to do it for me internally ;-)
  
  I certainly have no objections to making kerneldoc suck less.
  There was already an attempt to use asciidoc (like git uses) for
  kernel-doc (a few years ago, by Sam Ravnborg).  I support(ed) that
  effort.
  
  Hm, do you have pointers to those? My google-fu seems lacking ...
  
  I googled for /kernel doc asciidoc ravnborg/ and found several hits
  for them.
  
  Ok, let's move this to the top and start discussions. The past few
  months I've written piles of kerneldoc comments for the DRM DocBook
  (all pulled in as kerneldoc, docbook .tmpl has just the chapter
  structure). DOC: sections are really useful to pull all the actual
  documentation out of the docbook xml into kerneldoc.
  
  But I've also done piles of docs for intel-gpu-tools, which is using
  gtkdoc. And there are some clear deficiencies:
  
  - No markdown for inline coments. Lack of lists and tables is hurting
especially badly. If we add this (and I don't care one iota whether
it's
  
  Yes, I've tried to add lists to kernel-doc notation but have failed so
  far.
  
markdown or asciidoc or something else as long as it's readable
plain text in comments) we should be able to move all the existing
docbook xml paragraphs/lists/tables into kerneldoc comments.
  
  - Automatic cross-referencing of functions. If you place e.g.
function() or #struct anywhere in a documentation comment gtk-doc
automatically inserts a hyperlink to the relevant documentation
page across the entire project. Really powerful and makes overview
sections much more useful entry points for beginners since they can
easily jump backforth betweeen the high-level overview and low-
level per-function documentation.
  
  That's a nice-to-have IMO, but a really nice one.
  
  - In a really perfect world it would help if kerneldoc could collect
structure member kerneldoc from per-member comments. Especially for
large structures with lots of comments this would bring the
kerneldoc and struct member much closer together.
  
  So that's my wishlist.
  
  OTOH, I would only want to add another way to do kernel-doc if it
  can be a full replacement for all of our docbook usage, i.e., it
  should provide a way that we can eliminate docbook and stop using it
  completely.
  
  Hm, I don't mind docbook at all, as long as all the real content is
  embedded into source files as kerneldoc and the docbook template just
  pulls in all the right bits and pieces. Even gtkdoc allos you to do
  that and pull in the different libararies (== header files with
  declarations for C) in the order you want. So imo the docbook
  toolchain is good enough 

[Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs

2014-08-04 Thread Damien Lespiau
We don't actually need to write the HDMI entry on DDIs that have no
chance to be used as HDMI ports.

While this patch shouldn't change the current behaviour, it makes
further enabling work easier as we'll have an eDP table filling the full
10 entries.

Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/intel_ddi.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ca1f9a8..95933b2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -163,6 +163,7 @@ static void intel_prepare_ddi_buffers(struct drm_device 
*dev, enum port port)
const u32 *ddi_translations_edp;
const u32 *ddi_translations_hdmi;
const u32 *ddi_translations;
+   bool need_hdmi = false;
 
if (IS_BROADWELL(dev)) {
ddi_translations_fdi = bdw_ddi_translations_fdi;
@@ -195,12 +196,15 @@ static void intel_prepare_ddi_buffers(struct drm_device 
*dev, enum port port)
case PORT_B:
case PORT_C:
ddi_translations = ddi_translations_dp;
+   need_hdmi = true;
break;
case PORT_D:
if (intel_dp_is_edp(dev, PORT_D))
ddi_translations = ddi_translations_edp;
-   else
+   else {
ddi_translations = ddi_translations_dp;
+   need_hdmi = true;
+   }
break;
case PORT_E:
ddi_translations = ddi_translations_fdi;
@@ -215,6 +219,9 @@ static void intel_prepare_ddi_buffers(struct drm_device 
*dev, enum port port)
reg += 4;
}
 
+   if (!need_hdmi)
+   return;
+
/* Choose a good default if VBT is badly populated */
if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
hdmi_level = n_hdmi_entries)
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH] drm/i915: Don't require dev-struct_mutex in psr_match_conditions

2014-08-04 Thread Paulo Zanoni
2014-08-04 5:46 GMT-03:00 Daniel Vetter daniel.vet...@ffwll.ch:
 Since I've reworked psr support to no longer require x-tiling we don't
 check any state protected by the Giant GEM Lock. So drop that check.

 Also boo for lockdep_assert_held for not yelling when lockdep is
 disabled.

 Cc: Paulo Zanoni przan...@gmail.com
 Reported-by: Paulo Zanoni przan...@gmail.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

I was going to start reviewing it, but then I realized it's already merged.

Do we have any doc explaining all our locks/mutexes and what each one
is supposed to protect?

Anyway, the patch looks fine.


 ---
  drivers/gpu/drm/i915/intel_dp.c | 1 -
  1 file changed, 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 3e6100ea7295..6dbe0f84d455 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -1773,7 +1773,6 @@ static bool intel_edp_psr_match_conditions(struct 
 intel_dp *intel_dp)
 struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

 lockdep_assert_held(dev_priv-psr.lock);
 -   lockdep_assert_held(dev-struct_mutex);
 WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex));
 WARN_ON(!drm_modeset_is_locked(crtc-mutex));

 --
 2.0.1




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


Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs

2014-08-04 Thread Paulo Zanoni
2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
 We don't actually need to write the HDMI entry on DDIs that have no
 chance to be used as HDMI ports.

 While this patch shouldn't change the current behaviour, it makes
 further enabling work easier as we'll have an eDP table filling the full
 10 entries.

 Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com

While your patch looks correct, maybe you could have used
dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we
already do in intel_ddi_init. Or you could set another variable at
intel_ddi_init and use it, or do some other equivalent check that
doesn't require knowledge of what can really go in each port (since we
already have it at intel_ddi_init and we probably shouldn't duplicate
it to avoid future desync).

Anyway, the patch looks correct for now. So if you still think the
current approach is the best, you can add Reviewed-by: Paulo Zanoni
paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_ddi.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index ca1f9a8..95933b2 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -163,6 +163,7 @@ static void intel_prepare_ddi_buffers(struct drm_device 
 *dev, enum port port)
 const u32 *ddi_translations_edp;
 const u32 *ddi_translations_hdmi;
 const u32 *ddi_translations;
 +   bool need_hdmi = false;

 if (IS_BROADWELL(dev)) {
 ddi_translations_fdi = bdw_ddi_translations_fdi;
 @@ -195,12 +196,15 @@ static void intel_prepare_ddi_buffers(struct drm_device 
 *dev, enum port port)
 case PORT_B:
 case PORT_C:
 ddi_translations = ddi_translations_dp;
 +   need_hdmi = true;
 break;
 case PORT_D:
 if (intel_dp_is_edp(dev, PORT_D))
 ddi_translations = ddi_translations_edp;
 -   else
 +   else {
 ddi_translations = ddi_translations_dp;
 +   need_hdmi = true;
 +   }
 break;
 case PORT_E:
 ddi_translations = ddi_translations_fdi;
 @@ -215,6 +219,9 @@ static void intel_prepare_ddi_buffers(struct drm_device 
 *dev, enum port port)
 reg += 4;
 }

 +   if (!need_hdmi)
 +   return;
 +
 /* Choose a good default if VBT is badly populated */
 if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
 hdmi_level = n_hdmi_entries)
 --
 1.8.3.1

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



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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Collect gtier properly on HSW.

2014-08-04 Thread Paulo Zanoni
2014-08-01 13:12 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
 GTIER and DEIER doesn't have same interface on HSW so this or operation
 makes the information provided useless.

 v2: since we have gtier variable already let's split for everybody
 and avoid the strange | op.
 Also avoid overriding the value that was set for vlv. In this case I
 believe that we should reorganize the whole function, but I'll respect
 the comment that ask to not touch the order and let this organization
 work to be done later.
 v3: moving VLV check to the right place.

 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/i915_drv.h   |  1 +
  drivers/gpu/drm/i915/i915_gpu_error.c | 21 +++--
  2 files changed, 12 insertions(+), 10 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index d604f4f..60227b2 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -317,6 +317,7 @@ struct drm_i915_error_state {
 u32 eir;
 u32 pgtbl_er;
 u32 ier;
 +   u32 gtier;
 u32 ccid;
 u32 derrmr;
 u32 forcewake;
 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index 0b3f694..c8f901f 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -359,6 +359,8 @@ int i915_error_state_to_str(struct 
 drm_i915_error_state_buf *m,
 err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device);
 err_printf(m, EIR: 0x%08x\n, error-eir);
 err_printf(m, IER: 0x%08x\n, error-ier);
 +   if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
 +   err_printf(m, GTIER: 0x%08x\n, error-gtier);
 err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er);
 err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake);
 err_printf(m, DERRMR: 0x%08x\n, error-derrmr);
 @@ -1102,7 +1104,8 @@ static void i915_capture_reg_state(struct 
 drm_i915_private *dev_priv,

 /* 1: Registers specific to a single generation */
 if (IS_VALLEYVIEW(dev)) {
 -   error-ier = I915_READ(GTIER) | I915_READ(VLV_IER);
 +   error-gtier = I915_READ(GTIER);
 +   error-ier = I915_READ(VLV_IER);
 error-forcewake = I915_READ(FORCEWAKE_VLV);
 }

 @@ -1135,16 +1138,14 @@ static void i915_capture_reg_state(struct 
 drm_i915_private *dev_priv,
 if (HAS_HW_CONTEXTS(dev))
 error-ccid = I915_READ(CCID);

 -   if (HAS_PCH_SPLIT(dev))
 -   error-ier = I915_READ(DEIER) | I915_READ(GTIER);
 -   else {
 -   if (IS_GEN2(dev))
 -   error-ier = I915_READ16(IER);
 -   else
 -   error-ier = I915_READ(IER);
 +   if (HAS_PCH_SPLIT(dev)) {
 +   error-ier = I915_READ(DEIER);
 +   error-gtier = I915_READ(GTIER);
 +   } else if (IS_GEN2(dev)) {
 +   error-ier = I915_READ16(IER);
 +   } else if (!IS_VALLEYVIEW(dev)) {
 +   error-ier = I915_READ(IER);
 }
 -
 -   /* 4: Everything else */
 error-eir = I915_READ(EIR);
 error-pgtbl_er = I915_READ(PGTBL_ER);

 --
 1.9.1

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



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


Re: [Intel-gfx] [PATCH 2/3] drm/i915: Generalize drain latency computation

2014-08-04 Thread Ville Syrjälä
On Wed, Jul 16, 2014 at 06:24:04PM +0530, Gajanan Bhat wrote:
 Modify drain latency computation to use it for any plane. Same function can be
 used for primary, cursor and sprite planes.
 
 Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
 ---
  drivers/gpu/drm/i915/i915_reg.h |1 +
  drivers/gpu/drm/i915/intel_pm.c |   82 
 ++-
  2 files changed, 47 insertions(+), 36 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index d2a220b..a1260a2 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -3877,6 +3877,7 @@ enum punit_power_well {
  #define DDL_PLANE_PRECISION_64   (17)
  #define DDL_PLANE_PRECISION_32   (07)
  #define DDL_PLANE_SHIFT  0
 +#define DRAIN_LATENCY_MAX0x7f
  
  /* FIFO watermark sizes etc */
  #define G4X_FIFO_LINE_SIZE   64
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 90df1e8..f3a3e90 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -1268,33 +1268,21 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 display, cursor);
  }
  
 -static bool vlv_compute_drain_latency(struct drm_device *dev,
 -  int plane,
 -  int *plane_prec_mult,
 -  int *plane_dl,
 -  int *cursor_prec_mult,
 -  int *cursor_dl)
 +static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
 +   int pixel_size,
 +   int *prec_mult,
 +   int *drain_latency)
  {
 - struct drm_crtc *crtc;
 - int clock, pixel_size;
   int entries;
 + int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock;
  
 - crtc = intel_get_crtc_for_plane(dev, plane);
 - if (!intel_crtc_active(crtc))
 + if (clock == 0 || pixel_size == 0)
   return false;
  
 - clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock;
 - pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */
 -
 - entries = (clock / 1000) * pixel_size;
 - *plane_prec_mult = (entries  128) ?
 - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
 - *plane_dl = (64 * (*plane_prec_mult) * 4) / entries;
 -
 - entries = (clock / 1000) * 4;   /* BPP is always 4 for cursor */
 - *cursor_prec_mult = (entries  128) ?
 - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
 - *cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries;
 + entries = DIV_ROUND_UP(clock, 1000) * pixel_size;

Yeah rounding the clock up should be safe, rounding down not so much.
But this change should really be a separate patch.

 + *prec_mult = (entries  128) ? DRAIN_LATENCY_PRECISION_64 :
 +DRAIN_LATENCY_PRECISION_32;
 + *drain_latency = (64 * (*prec_mult) * 4) / entries;

Another thing you could fix is making sure the result is =0x7f. It's
quite possible we'd exceed that (all that's needed is a 16bpp fb and
32MHz pixel clock, which isn't all that far fetched). This could
be part of the rounding fix patch since it's somewhat related.

  
   return true;
  }
 @@ -1309,24 +1297,46 @@ static bool vlv_compute_drain_latency(struct 
 drm_device *dev,
  
  static void vlv_update_drain_latency(struct drm_crtc *crtc)
  {
 - struct drm_device *dev = crtc-dev;
 - struct drm_i915_private *dev_priv = dev-dev_private;
 + struct drm_i915_private *dev_priv = crtc-dev-dev_private;
 + int pixel_size;
 + int drain_latency;
   enum pipe pipe = to_intel_crtc(crtc)-pipe;
 - int plane_prec, plane_dl;
 - int cursor_prec, cursor_dl;
 - int plane_prec_mult, cursor_prec_mult;
 + int plane_prec, prec_mult, plane_dl;
  
 - if (vlv_compute_drain_latency(dev, pipe, plane_prec_mult, plane_dl,
 -   cursor_prec_mult, cursor_dl)) {
 - cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
 - DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32;
 - plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
 - DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32;
 + plane_dl = I915_READ(VLV_DDL(pipe))  ~DDL_PLANE_PRECISION_64 
 +~DRAIN_LATENCY_MAX  ~DDL_CURSOR_PRECISION_64 
 +~(DRAIN_LATENCY_MAX  DDL_CURSOR_SHIFT);
  
 - I915_WRITE(VLV_DDL(pipe), cursor_prec |
 -(cursor_dl  DDL_CURSOR_SHIFT) |
 -plane_prec | (plane_dl  DDL_PLANE_SHIFT));
 + if (!intel_crtc_active(crtc)) {
 + I915_WRITE(VLV_DDL(pipe), plane_dl);
 + return;
   }
 +
 + /* Primary plane Drain Latency */
 + 

Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs

2014-08-04 Thread Damien Lespiau
On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote:
 2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
  We don't actually need to write the HDMI entry on DDIs that have no
  chance to be used as HDMI ports.
 
  While this patch shouldn't change the current behaviour, it makes
  further enabling work easier as we'll have an eDP table filling the full
  10 entries.
 
  Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com
  Signed-off-by: Damien Lespiau damien.lesp...@intel.com
 
 While your patch looks correct, maybe you could have used
 dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we
 already do in intel_ddi_init. Or you could set another variable at
 intel_ddi_init and use it, or do some other equivalent check that
 doesn't require knowledge of what can really go in each port (since we
 already have it at intel_ddi_init and we probably shouldn't duplicate
 it to avoid future desync).
 
 Anyway, the patch looks correct for now. So if you still think the
 current approach is the best, you can add Reviewed-by: Paulo Zanoni
 paulo.r.zan...@intel.com

I actually like that better, Daniel, hold on your horses :)

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

2014-08-04 Thread Paulo Zanoni
2014-08-01 13:13 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
 BDW has many other Display Engine interrupts and GT interrupts registers.
 Collecting it properly on gpu_error_state.

 On debugfs all was properly listed already but besides we were also listing 
 old
 DEIER and GTIER that doesn't exist on BDW anymore. This was causing
 unclaimed register messages:

 https://bugs.freedesktop.org/show_bug.cgi?id=81701

 v2: Fix small issues of first version and don't read DEIER regs when pipe's
 power well is disabled
 v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe 
 interection

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com


 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/i915_debugfs.c   |  4 
  drivers/gpu/drm/i915/i915_drv.h   |  4 +++-
  drivers/gpu/drm/i915/i915_gpu_error.c | 32 
  3 files changed, 35 insertions(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 9e737b7..b3493d3 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void 
 *data)
 }

 for_each_pipe(pipe) {
 +   if (!intel_display_power_enabled(dev_priv,
 +
 POWER_DOMAIN_PIPE(pipe)))
 +   continue;
 +
 seq_printf(m, Pipe %c IMR:\t%08x\n,
pipe_name(pipe),
I915_READ(GEN8_DE_PIPE_IMR(pipe)));
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 60227b2..d1ae952 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -317,7 +317,9 @@ struct drm_i915_error_state {
 u32 eir;
 u32 pgtbl_er;
 u32 ier;
 -   u32 gtier;
 +   u32 gtier[4];
 +   u32 deier[3];
 +   u32 de_misc_ier;
 u32 ccid;
 u32 derrmr;
 u32 forcewake;
 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index c8f901f..402b621 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -328,6 +328,7 @@ int i915_error_state_to_str(struct 
 drm_i915_error_state_buf *m,
 struct drm_i915_private *dev_priv = dev-dev_private;
 struct drm_i915_error_state *error = error_priv-error;
 struct drm_i915_error_object *obj;
 +   enum pipe pipe;
 int i, j, offset, elt;
 int max_hangcheck_score;

 @@ -359,8 +360,20 @@ int i915_error_state_to_str(struct 
 drm_i915_error_state_buf *m,
 err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device);
 err_printf(m, EIR: 0x%08x\n, error-eir);
 err_printf(m, IER: 0x%08x\n, error-ier);
 +   if (IS_BROADWELL(dev)) {
 +   for_each_pipe(pipe)
 +   err_printf(m, DEIER pipe %c: 0x%08x\n,
 +  pipe_name(pipe),
 +  error-deier[pipe]);
 +   for (i = 0; i  4; i++)
 +   err_printf(m, GTIER gt %d: 0x%08x\n, i,
 +  error-gtier[i]);
 +   err_printf(m, DE_MISC_IER: 0x%08x\n, error-de_misc_ier);
 +   } else {
 +   err_printf(m, IER: 0x%08x\n, error-ier);
 +   }
 if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
 -   err_printf(m, GTIER: 0x%08x\n, error-gtier);
 +   err_printf(m, GTIER: 0x%08x\n, error-gtier[0]);
 err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er);
 err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake);
 err_printf(m, DERRMR: 0x%08x\n, error-derrmr);
 @@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct 
 drm_i915_private *dev_priv,
struct drm_i915_error_state *error)
  {
 struct drm_device *dev = dev_priv-dev;
 +   enum pipe pipe;
 +   int i;

 /* General organization
  * 1. Registers specific to a single generation
 @@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct 
 drm_i915_private *dev_priv,

 /* 1: Registers specific to a single generation */
 if (IS_VALLEYVIEW(dev)) {
 -   error-gtier = I915_READ(GTIER);
 +   error-gtier[0] = I915_READ(GTIER);
 error-ier = I915_READ(VLV_IER);
 error-forcewake = I915_READ(FORCEWAKE_VLV);
 }
 @@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct 
 drm_i915_private *dev_priv,
 if (HAS_HW_CONTEXTS(dev))
 error-ccid = I915_READ(CCID);

 -   if (HAS_PCH_SPLIT(dev)) {
 +   if (IS_BROADWELL(dev)) {
 +   for_each_pipe(pipe)
 +   if 

[Intel-gfx] [PATCH 1/2] drm/i915: Tune done rc6 enabling output

2014-08-04 Thread Daniel Vetter
Power users spot this and then get adventurous and try to adjust
module driver options. Nothing good ever came out of that, so
hide it better.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_pm.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 338a80b6773e..4f879494e0c5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device 
*dev, u32 mode)
else
mode = 0;
}
-   DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
-(mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
-(mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
-(mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
+   DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
+ (mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
+ (mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
+ (mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
 }
 
 static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
@@ -3452,8 +3452,8 @@ static int sanitize_rc6_option(const struct drm_device 
*dev, int enable_rc6)
mask = INTEL_RC6_ENABLE;
 
if ((enable_rc6  mask) != enable_rc6)
-   DRM_INFO(Adjusting RC6 mask to %d (requested %d, valid 
%d)\n,
-enable_rc6  mask, enable_rc6, mask);
+   DRM_DEBUG_KMS(Adjusting RC6 mask to %d (requested %d, 
valid %d)\n,
+ enable_rc6  mask, enable_rc6, mask);
 
return enable_rc6  mask;
}
-- 
2.0.1

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


[Intel-gfx] [PATCH 2/2] drm/i915: Tune down MCH_SSKPD values warning

2014-08-04 Thread Daniel Vetter
Users often can't do anything about this since their vendors stopped
providing BIOS updates. Also we seem to be able to hack around it
with increased latency values, and thus far the only reports have
been for screens with really high resolutions. So tune it down to a
level where only developers can see it.

Also drop some of the end-user fluff.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4f879494e0c5..684dc5f60544 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5235,11 +5235,9 @@ static void gen6_check_mch_setup(struct drm_device *dev)
uint32_t tmp;
 
tmp = I915_READ(MCH_SSKPD);
-   if ((tmp  MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) {
-   DRM_INFO(Wrong MCH_SSKPD value: 0x%08x\n, tmp);
-   DRM_INFO(This can cause pipe underruns and display issues.\n);
-   DRM_INFO(Please upgrade your BIOS to fix this.\n);
-   }
+   if ((tmp  MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL)
+   DRM_DEBUG_KMS(Wrong MCH_SSKPD value: 0x%08x This can cause 
underruns.\n,
+ tmp);
 }
 
 static void gen6_init_clock_gating(struct drm_device *dev)
-- 
2.0.1

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

2014-08-04 Thread Daniel Vetter
On Mon, Aug 04, 2014 at 11:44:10AM -0300, Paulo Zanoni wrote:
 2014-08-01 13:13 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
  BDW has many other Display Engine interrupts and GT interrupts registers.
  Collecting it properly on gpu_error_state.
 
  On debugfs all was properly listed already but besides we were also listing 
  old
  DEIER and GTIER that doesn't exist on BDW anymore. This was causing
  unclaimed register messages:
 
  https://bugs.freedesktop.org/show_bug.cgi?id=81701
 
  v2: Fix small issues of first version and don't read DEIER regs when pipe's
  power well is disabled
  v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe 
  interection
 
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

Both patches merged to dinq, thanks.
-Daniel

 
 
  Cc: Paulo Zanoni paulo.r.zan...@intel.com
  Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
  ---
   drivers/gpu/drm/i915/i915_debugfs.c   |  4 
   drivers/gpu/drm/i915/i915_drv.h   |  4 +++-
   drivers/gpu/drm/i915/i915_gpu_error.c | 32 
   3 files changed, 35 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
  b/drivers/gpu/drm/i915/i915_debugfs.c
  index 9e737b7..b3493d3 100644
  --- a/drivers/gpu/drm/i915/i915_debugfs.c
  +++ b/drivers/gpu/drm/i915/i915_debugfs.c
  @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, 
  void *data)
  }
 
  for_each_pipe(pipe) {
  +   if (!intel_display_power_enabled(dev_priv,
  +
  POWER_DOMAIN_PIPE(pipe)))
  +   continue;
  +
  seq_printf(m, Pipe %c IMR:\t%08x\n,
 pipe_name(pipe),
 I915_READ(GEN8_DE_PIPE_IMR(pipe)));
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index 60227b2..d1ae952 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -317,7 +317,9 @@ struct drm_i915_error_state {
  u32 eir;
  u32 pgtbl_er;
  u32 ier;
  -   u32 gtier;
  +   u32 gtier[4];
  +   u32 deier[3];
  +   u32 de_misc_ier;
  u32 ccid;
  u32 derrmr;
  u32 forcewake;
  diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
  b/drivers/gpu/drm/i915/i915_gpu_error.c
  index c8f901f..402b621 100644
  --- a/drivers/gpu/drm/i915/i915_gpu_error.c
  +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
  @@ -328,6 +328,7 @@ int i915_error_state_to_str(struct 
  drm_i915_error_state_buf *m,
  struct drm_i915_private *dev_priv = dev-dev_private;
  struct drm_i915_error_state *error = error_priv-error;
  struct drm_i915_error_object *obj;
  +   enum pipe pipe;
  int i, j, offset, elt;
  int max_hangcheck_score;
 
  @@ -359,8 +360,20 @@ int i915_error_state_to_str(struct 
  drm_i915_error_state_buf *m,
  err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device);
  err_printf(m, EIR: 0x%08x\n, error-eir);
  err_printf(m, IER: 0x%08x\n, error-ier);
  +   if (IS_BROADWELL(dev)) {
  +   for_each_pipe(pipe)
  +   err_printf(m, DEIER pipe %c: 0x%08x\n,
  +  pipe_name(pipe),
  +  error-deier[pipe]);
  +   for (i = 0; i  4; i++)
  +   err_printf(m, GTIER gt %d: 0x%08x\n, i,
  +  error-gtier[i]);
  +   err_printf(m, DE_MISC_IER: 0x%08x\n, error-de_misc_ier);
  +   } else {
  +   err_printf(m, IER: 0x%08x\n, error-ier);
  +   }
  if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
  -   err_printf(m, GTIER: 0x%08x\n, error-gtier);
  +   err_printf(m, GTIER: 0x%08x\n, error-gtier[0]);
  err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er);
  err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake);
  err_printf(m, DERRMR: 0x%08x\n, error-derrmr);
  @@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct 
  drm_i915_private *dev_priv,
 struct drm_i915_error_state *error)
   {
  struct drm_device *dev = dev_priv-dev;
  +   enum pipe pipe;
  +   int i;
 
  /* General organization
   * 1. Registers specific to a single generation
  @@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct 
  drm_i915_private *dev_priv,
 
  /* 1: Registers specific to a single generation */
  if (IS_VALLEYVIEW(dev)) {
  -   error-gtier = I915_READ(GTIER);
  +   error-gtier[0] = I915_READ(GTIER);
  error-ier = I915_READ(VLV_IER);
  error-forcewake = I915_READ(FORCEWAKE_VLV);
  }
  @@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct 
  

Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()

2014-08-04 Thread Daniel Vetter
On Mon, Aug 04, 2014 at 03:50:34PM +0300, Ville Syrjälä wrote:
 On Mon, Aug 04, 2014 at 10:10:50AM +0200, Daniel Vetter wrote:
  On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote:
   On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote:
On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote:
 On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com 
 wrote:
 Daniel, the only way intel_dp-is_mst can get reset is inside this 
 path.

 Ok, so that one should be safe. Then I guess we can just push the
 locking down into the respective non-mst leafs (since atm we do
 link-retraining without any locking, which isn't good). And it needs
 to be dev-mode_config.mutex, not connection mutex.
   
   Why that? We can't be doing a modeset w/o connection_mutex so that
   seems like it should be enough. Well, there's also dpms which leaves
   the crtc-encoder-connector links intact but that too takes
   connection_mutex currently.
   

I'd like to know why we do link training at this point though as well,
adding locking is required of course, I was just going to wrap the
short irq call to the link status check with the lock, but I think it
should be possible to push it down further,
   
   I don't really know why the sink generates the hpd when we turn off the
   port, but that doesn't really matter I think. We need to be prepared for
   hpds at any time.
   
   intel_dp_check_link_status() just checks if there's a crtc, which there
   is (either the old one or the new one, depending on how far along the
   modeset path we are I guess), and then it just checks
   drm_dp_channel_eq_ok() which says false since the link was turned off,
   and then it proceeds to retrain the link.
   
   Maybe it should also check crtc-active? Though that itself won't 
   eliminate the problem unless the locking gets fixed somehow.
  
  We check encoder-connectors_active, which is equivalent.
 
 Only after intel_sanitize_encoder(). Before that we can have
 !crtc-active  encoder-connectors_active.

We grab all modeset locks when calling modeset_setup_hw_state, which means
no one will ever notice the inconsistent state between state readout and
sanitizing it. I think we're safe.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Don't require dev-struct_mutex in psr_match_conditions

2014-08-04 Thread Daniel Vetter
On Mon, Aug 04, 2014 at 11:16:10AM -0300, Paulo Zanoni wrote:
 2014-08-04 5:46 GMT-03:00 Daniel Vetter daniel.vet...@ffwll.ch:
  Since I've reworked psr support to no longer require x-tiling we don't
  check any state protected by the Giant GEM Lock. So drop that check.
 
  Also boo for lockdep_assert_held for not yelling when lockdep is
  disabled.
 
  Cc: Paulo Zanoni przan...@gmail.com
  Reported-by: Paulo Zanoni przan...@gmail.com
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 
 I was going to start reviewing it, but then I realized it's already merged.

Oh, I've figured I'll sneak this one by the danvet must have reviewed-by
too rule ;-) But I'll drop such patches asap if anyone spots something
with them ofc.

 Do we have any doc explaining all our locks/mutexes and what each one
 is supposed to protect?

Unfortunately not. It's also constantly changing (e.g. the recent
introduction of the connection_mutex) and rather shockingly often not
quite correct. Atm you need to dig through git history and for drm core
locks through all drm drivers to figure this out :(

 Anyway, the patch looks fine.

I'll count this as an ack and added it, 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] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs

2014-08-04 Thread Daniel Vetter
On Mon, Aug 04, 2014 at 03:44:12PM +0100, Damien Lespiau wrote:
 On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote:
  2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
   We don't actually need to write the HDMI entry on DDIs that have no
   chance to be used as HDMI ports.
  
   While this patch shouldn't change the current behaviour, it makes
   further enabling work easier as we'll have an eDP table filling the full
   10 entries.
  
   Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com
   Signed-off-by: Damien Lespiau damien.lesp...@intel.com
  
  While your patch looks correct, maybe you could have used
  dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we
  already do in intel_ddi_init. Or you could set another variable at
  intel_ddi_init and use it, or do some other equivalent check that
  doesn't require knowledge of what can really go in each port (since we
  already have it at intel_ddi_init and we probably shouldn't duplicate
  it to avoid future desync).
  
  Anyway, the patch looks correct for now. So if you still think the
  current approach is the best, you can add Reviewed-by: Paulo Zanoni
  paulo.r.zan...@intel.com
 
 I actually like that better, Daniel, hold on your horses :)

So I've spotted some other nice cleanups fly around in the internal m-l,
how do those relate to the ones here? Can we please have them for bdw/hsw
right away, I liked the added pretty ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Tune down MCH_SSKPD values warning

2014-08-04 Thread Ville Syrjälä
On Mon, Aug 04, 2014 at 11:18:28AM +0200, Daniel Vetter wrote:
 Users often can't do anything about this since their vendors stopped
 providing BIOS updates. Also we seem to be able to hack around it
 with increased latency values, and thus far the only reports have
 been for screens with really high resolutions. So tune it down to a
 level where only developers can see it.
 
 Also drop some of the end-user fluff.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

Both patches make sense to me so:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 4f879494e0c5..684dc5f60544 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -5235,11 +5235,9 @@ static void gen6_check_mch_setup(struct drm_device 
 *dev)
   uint32_t tmp;
  
   tmp = I915_READ(MCH_SSKPD);
 - if ((tmp  MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) {
 - DRM_INFO(Wrong MCH_SSKPD value: 0x%08x\n, tmp);
 - DRM_INFO(This can cause pipe underruns and display issues.\n);
 - DRM_INFO(Please upgrade your BIOS to fix this.\n);
 - }
 + if ((tmp  MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL)
 + DRM_DEBUG_KMS(Wrong MCH_SSKPD value: 0x%08x This can cause 
 underruns.\n,
 +   tmp);
  }
  
  static void gen6_init_clock_gating(struct drm_device *dev)
 -- 
 2.0.1
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Tune done rc6 enabling output

2014-08-04 Thread Paulo Zanoni
2014-08-04 6:18 GMT-03:00 Daniel Vetter daniel.vet...@ffwll.ch:
 Power users spot this and then get adventurous and try to adjust
 module driver options. Nothing good ever came out of that, so
 hide it better.

Agreed. Back when we were toggling the default behavior of RC6 at
every new -rc release, this was useful.

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com


 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_pm.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 338a80b6773e..4f879494e0c5 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device 
 *dev, u32 mode)
 else
 mode = 0;
 }
 -   DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
 -(mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
 -(mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
 -(mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
 +   DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
 + (mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
 + (mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
 + (mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
  }

  static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
 @@ -3452,8 +3452,8 @@ static int sanitize_rc6_option(const struct drm_device 
 *dev, int enable_rc6)
 mask = INTEL_RC6_ENABLE;

 if ((enable_rc6  mask) != enable_rc6)
 -   DRM_INFO(Adjusting RC6 mask to %d (requested %d, 
 valid %d)\n,
 -enable_rc6  mask, enable_rc6, mask);
 +   DRM_DEBUG_KMS(Adjusting RC6 mask to %d (requested 
 %d, valid %d)\n,
 + enable_rc6  mask, enable_rc6, mask);

 return enable_rc6  mask;
 }
 --
 2.0.1

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



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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Tune down MCH_SSKPD values warning

2014-08-04 Thread Paulo Zanoni
2014-08-04 12:03 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com:
 On Mon, Aug 04, 2014 at 11:18:28AM +0200, Daniel Vetter wrote:
 Users often can't do anything about this since their vendors stopped
 providing BIOS updates. Also we seem to be able to hack around it
 with increased latency values, and thus far the only reports have
 been for screens with really high resolutions. So tune it down to a
 level where only developers can see it.

 Also drop some of the end-user fluff.

 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

 Both patches make sense to me so:
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

I never really understood why we expect a specific value for a
specific field of the SSKPD on SNB. Isn't this value based on a lot of
different machine-dependent variables? On HSW this doesn't make sense
at all (and we don't have this check there, so the code is already
fine).


 ---
  drivers/gpu/drm/i915/intel_pm.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c 
 b/drivers/gpu/drm/i915/intel_pm.c
 index 4f879494e0c5..684dc5f60544 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -5235,11 +5235,9 @@ static void gen6_check_mch_setup(struct drm_device 
 *dev)
   uint32_t tmp;

   tmp = I915_READ(MCH_SSKPD);
 - if ((tmp  MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) {
 - DRM_INFO(Wrong MCH_SSKPD value: 0x%08x\n, tmp);
 - DRM_INFO(This can cause pipe underruns and display 
 issues.\n);
 - DRM_INFO(Please upgrade your BIOS to fix this.\n);
 - }
 + if ((tmp  MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL)
 + DRM_DEBUG_KMS(Wrong MCH_SSKPD value: 0x%08x This can cause 
 underruns.\n,
 +   tmp);
  }

  static void gen6_init_clock_gating(struct drm_device *dev)
 --
 2.0.1

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

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



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


Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs

2014-08-04 Thread Damien Lespiau
On Mon, Aug 04, 2014 at 05:04:31PM +0200, Daniel Vetter wrote:
 On Mon, Aug 04, 2014 at 03:44:12PM +0100, Damien Lespiau wrote:
  On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote:
   2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
We don't actually need to write the HDMI entry on DDIs that have no
chance to be used as HDMI ports.
   
While this patch shouldn't change the current behaviour, it makes
further enabling work easier as we'll have an eDP table filling the full
10 entries.
   
Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
   
   While your patch looks correct, maybe you could have used
   dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we
   already do in intel_ddi_init. Or you could set another variable at
   intel_ddi_init and use it, or do some other equivalent check that
   doesn't require knowledge of what can really go in each port (since we
   already have it at intel_ddi_init and we probably shouldn't duplicate
   it to avoid future desync).
   
   Anyway, the patch looks correct for now. So if you still think the
   current approach is the best, you can add Reviewed-by: Paulo Zanoni
   paulo.r.zan...@intel.com
  
  I actually like that better, Daniel, hold on your horses :)
 
 So I've spotted some other nice cleanups fly around in the internal m-l,
 how do those relate to the ones here? Can we please have them for bdw/hsw
 right away, I liked the added pretty ;-)

We already have all the patches that have been written in tree
(excluding this one, which is really anecdotal for current platforms,
that will just remove a couple of register writes, a patch is coming
this way soon-ish anyway).

The remaining clean-ups Sonika and/or I are signed up for are:

  - rename the DP training vswing/pre-emph defines to not include the
nominal values but include the level (as for instance eDP 1.4
introduces low voltage swings and those values makes no sense then)

  - rename the HSW-specific defines to select the DDI buffer translation
slot to just include the slot index as the specifics
(vswing/pre-emph) depend on the platform and are starting to be more
confusing than helpful.

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


Re: [Intel-gfx] reverse dp link param selection, prefer fast over wide again

2014-08-04 Thread Daniel Vetter
On Mon, Aug 4, 2014 at 5:09 PM,  mario_limoncie...@dell.com wrote:
 (sorry re-sending without Internal use disclaimer – my mail client does that
 if I don’t explicitly tell it not to)



 Thanks Daniel. I wasn't sure if this was better discussed in the open or
 directly with your team.

Well for internal discussions I'll force you to go through the proper
support channels - the community fastpath only counts if it's public
;-)

 Do you have a branch somewhere that we can point folks encountering this
 problem that Dave Airlie has queued up but not yet merged for 3.17? We could
 try to garner feedback if that would help.

drm-intel-nightly from git://anongit.freedesktop.org/drm-intel is the
recommended branch. Probably only for internal testing and not
annoying customers yet since the dp mst support hasn't fully
stabilized yet.
-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 1/2] drm/i915: Tune done rc6 enabling output

2014-08-04 Thread Chris Wilson
On Mon, Aug 04, 2014 at 11:18:27AM +0200, Daniel Vetter wrote:
 Power users spot this and then get adventurous and try to adjust
 module driver options. Nothing good ever came out of that, so
 hide it better.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_pm.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 338a80b6773e..4f879494e0c5 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device 
 *dev, u32 mode)
   else
   mode = 0;
   }
 - DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
 -  (mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
 -  (mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
 -  (mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
 + DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
 +   (mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
 +   (mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
 +   (mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
  }

I disagree. This is a good informative message about the capabilities
enabled for their GPU. If you want to make the statement that the user
shouldn't touch the values, make that adjustment emit a WARNING and
implement the kernel tainting you suggested. Also fixing [drm] to be
more specific would be useful.
-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 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt

2014-08-04 Thread Daniel Vetter
On Fri, Aug 01, 2014 at 09:54:09AM +, Thierry, Michel wrote:
   @@ -1766,6 +1771,20 @@ int i915_gem_setup_global_gtt(struct
  drm_device *dev,
 /* And finally clear the reserved guard page */
 ggtt_vm-clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true);
  
   + if (HAS_ALIASING_PPGTT(dev)  USES_FULL_PPGTT(dev)) {
  
  Should that be !USES_FULL_PPGTT ?
 I think we need this for aliasing  full ppgtt (the global default ctx)...
 so it should be USES_PPGTT.

We want to setup the aliasing ppgtt _only_ when required, so we don't
actually need it for USES_FULL_PPGTT. So Ville's right here.
-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 1/2] drm/i915: Tune done rc6 enabling output

2014-08-04 Thread Daniel Vetter
On Mon, Aug 04, 2014 at 04:34:28PM +0100, Chris Wilson wrote:
 On Mon, Aug 04, 2014 at 11:18:27AM +0200, Daniel Vetter wrote:
  Power users spot this and then get adventurous and try to adjust
  module driver options. Nothing good ever came out of that, so
  hide it better.
  
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/intel_pm.c | 12 ++--
   1 file changed, 6 insertions(+), 6 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 338a80b6773e..4f879494e0c5 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device 
  *dev, u32 mode)
  else
  mode = 0;
  }
  -   DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
  -(mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
  -(mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
  -(mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
  +   DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
  + (mode  GEN6_RC_CTL_RC6_ENABLE) ? on : off,
  + (mode  GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
  + (mode  GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
   }
 
 I disagree. This is a good informative message about the capabilities
 enabled for their GPU. If you want to make the statement that the user
 shouldn't touch the values, make that adjustment emit a WARNING and
 implement the kernel tainting you suggested. Also fixing [drm] to be
 more specific would be useful.

Well I'd agree if we'd generally report this, but we don't. This really is
the only feature where we tell the user in dmesg what we've done with it,
for a lot of other stuff (psr, fbc, rps ...) we just silently set stuff
up. rc6 really is the only thing and I don't see why it's special. Imo
users should use tools like powertop to assess the effectivenes or our pm
code anyway, not look at dmesg.

So this is orthogonal to the module param tainting which unfortunately
missed 3.17 since Jani (who took over) went on vacatino before completing
the revised patches.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Initialize the aliasing ppgtt as part of global gtt

2014-08-04 Thread Daniel Vetter
Stuffing this into the context setup code doesn't make a lot of sense.
Also reusing the real ppgtt setup code makes even less sense since the
aliasing ppgtt isn't a real address space. Leaving all that stuff
unitialized will make sure that we catch any abusers promptly.

This is also a prep work to clean up the context-ppgtt link.

v2: Fix up the logic fail, I've fumbled it so badly to completely
disable ppgtt on gen6. Spotted by Ville and Michel. Also move around
the pde write into the gen6 init function, since otherwise it won't
work at all.

Cc: Thierry, Michel michel.thie...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem_context.c | 13 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 42 +++--
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 3b8367aa8404..7a455fcee3a7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -276,17 +276,6 @@ i915_gem_create_context(struct drm_device *dev,
goto err_unpin;
} else
ctx-vm = ppgtt-base;
-
-   /* This case is reserved for the global default context and
-* should only happen once. */
-   if (is_global_default_ctx) {
-   if (WARN_ON(dev_priv-mm.aliasing_ppgtt)) {
-   ret = -EEXIST;
-   goto err_unpin;
-   }
-
-   dev_priv-mm.aliasing_ppgtt = ppgtt;
-   }
} else if (USES_PPGTT(dev)) {
/* For platforms which only have aliasing PPGTT, we fake the
 * address space and refcounting. */
@@ -361,7 +350,7 @@ int i915_gem_context_init(struct drm_device *dev)
}
}
 
-   ctx = i915_gem_create_context(dev, NULL, USES_PPGTT(dev));
+   ctx = i915_gem_create_context(dev, NULL, USES_FULL_PPGTT(dev));
if (IS_ERR(ctx)) {
DRM_ERROR(Failed to create default global context (error 
%ld)\n,
  PTR_ERR(ctx));
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4fa7807ed4d5..a4c1740d79be 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1188,35 +1188,38 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 ppgtt-node.size  20,
 ppgtt-node.start / PAGE_SIZE);
 
+   gen6_write_pdes(ppgtt);
+   DRM_DEBUG(Adding PPGTT at offset %x\n,
+ ppgtt-pd_offset  10);
+
return 0;
 }
 
-int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
+int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-   int ret = 0;
 
ppgtt-base.dev = dev;
ppgtt-base.scratch = dev_priv-gtt.base.scratch;
 
if (INTEL_INFO(dev)-gen  8)
-   ret = gen6_ppgtt_init(ppgtt);
+   return gen6_ppgtt_init(ppgtt);
else if (IS_GEN8(dev))
-   ret = gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total);
+   return gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total);
else
BUG();
+}
+int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret = 0;
 
-   if (!ret) {
-   struct drm_i915_private *dev_priv = dev-dev_private;
+   ret = __hw_ppgtt_init(dev, ppgtt);
+   if (ret == 0) {
kref_init(ppgtt-ref);
drm_mm_init(ppgtt-base.mm, ppgtt-base.start,
ppgtt-base.total);
i915_init_vm(dev_priv, ppgtt-base);
-   if (INTEL_INFO(dev)-gen  8) {
-   gen6_write_pdes(ppgtt);
-   DRM_DEBUG(Adding PPGTT at offset %x\n,
- ppgtt-pd_offset  10);
-   }
}
 
return ret;
@@ -1728,6 +1731,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev,
struct drm_mm_node *entry;
struct drm_i915_gem_object *obj;
unsigned long hole_start, hole_end;
+   int ret;
 
BUG_ON(mappable_end  end);
 
@@ -1739,7 +1743,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */
list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) {
struct i915_vma *vma = i915_gem_obj_to_vma(obj, ggtt_vm);
-   int ret;
+
DRM_DEBUG_KMS(reserving preallocated space: %lx + %zx\n,
  i915_gem_obj_ggtt_offset(obj), obj-base.size);
 
@@ -1766,6 

[Intel-gfx] [PATCH] drm/i915: Only track real ppgtt for a context

2014-08-04 Thread Daniel Vetter
There's a bit a confusion since we track the global gtt,
the aliasing and real ppgtt in the ctx-vm pointer. And not
all callers really bother to check for the different cases and just
presume that it points to a real ppgtt.

Now looking closely we don't actually need -vm to always point at an
address space - the only place that cares actually has fixup code
already to decide whether to look at the per-proces or the global
address space.

So switch to just tracking the ppgtt directly and ditch all the
extraneous code.

v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx-ppgtt.
Also drop the early exit - without aliasing ppgtt we want to dump all
the ppgtts of the contexts if we have full ppgtt.

Cc: Thierry, Michel michel.thie...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
OTC-Jira: VIZ-3724
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_debugfs.c| 11 ---
 drivers/gpu/drm/i915/i915_drv.h|  3 +--
 drivers/gpu/drm/i915/i915_gem_context.c| 28 +++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 +++--
 drivers/gpu/drm/i915/i915_gpu_error.c  | 10 +++---
 5 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3bf1d20c598b..2bd4beada1bd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data)
 {
struct intel_context *ctx = ptr;
struct seq_file *m = data;
-   struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
+   struct i915_hw_ppgtt *ppgtt = ctx-ppgtt;
+
+   if (!ppgtt) {
+   seq_puts(m,   no ppgtt for context %d\n,
+ctx-user_handle);
+   return 0;
+   }
 
if (i915_gem_context_is_default(ctx))
seq_puts(m,   default context:\n);
@@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m, struct 
drm_device *dev)
seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd_offset);
 
ppgtt-debug_dump(ppgtt, m);
-   } else
-   return;
+   }
 
list_for_each_entry_reverse(file, dev-filelist, lhead) {
struct drm_i915_file_private *file_priv = file-driver_priv;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0762e19f9bc6..3230b08aff13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -616,7 +616,7 @@ struct intel_context {
uint8_t remap_slice;
struct drm_i915_file_private *file_priv;
struct i915_ctx_hang_stats hang_stats;
-   struct i915_address_space *vm;
+   struct i915_hw_ppgtt *ppgtt;
 
struct {
struct drm_i915_gem_object *rcs_state;
@@ -2504,7 +2504,6 @@ i915_gem_object_ggtt_unbind(struct drm_i915_gem_object 
*obj)
 void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj);
 
 /* i915_gem_context.c */
-#define ctx_to_ppgtt(ctx) container_of((ctx)-vm, struct i915_hw_ppgtt, base)
 int __must_check i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_reset(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 7a455fcee3a7..c00e5d027774 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -136,15 +136,8 @@ void i915_gem_context_free(struct kref *ctx_ref)
 {
struct intel_context *ctx = container_of(ctx_ref,
   typeof(*ctx), ref);
-   struct i915_hw_ppgtt *ppgtt = NULL;
 
-   if (ctx-legacy_hw_ctx.rcs_state) {
-   /* We refcount even the aliasing PPGTT to keep the code 
symmetric */
-   if (USES_PPGTT(ctx-legacy_hw_ctx.rcs_state-base.dev))
-   ppgtt = ctx_to_ppgtt(ctx);
-   }
-
-   i915_ppgtt_put(ppgtt);
+   i915_ppgtt_put(ctx-ppgtt);
if (ctx-legacy_hw_ctx.rcs_state)
drm_gem_object_unreference(ctx-legacy_hw_ctx.rcs_state-base);
list_del(ctx-link);
@@ -240,7 +233,6 @@ i915_gem_create_context(struct drm_device *dev,
bool create_vm)
 {
const bool is_global_default_ctx = file_priv == NULL;
-   struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_context *ctx;
int ret = 0;
 
@@ -274,15 +266,10 @@ i915_gem_create_context(struct drm_device *dev,
 PTR_ERR(ppgtt));
ret = PTR_ERR(ppgtt);
goto err_unpin;
-   } else
-   ctx-vm = ppgtt-base;
-   } else if (USES_PPGTT(dev)) {
-   /* For platforms which only have aliasing PPGTT, we fake the
-* address space and 

Re: [Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps

2014-08-04 Thread Eric Anholt
Keith Packard kei...@keithp.com writes:

 Eric Anholt e...@anholt.net writes:

 This change appears to be unrelated, and possibly harmful (if X has
 dropped the last ref to the BO, but it's still the scanout buffer, a new
 allocation would now reuse the BO and scribble on scanout until the next
 modeset happens).

 Yeah, it's unrelated. intel_allocate_framebuffer calls disable_reuse, so
 there's no need to call it from these two other places. I'll split that
 change out into a separate patch with separate comment.

 Unrelated whitespace.

 There are a bunch of whitespace fixups; should I pull those into a
 separate patch or just leave them scattered in the first patch to change
 a file?

One patch at the front is fine with me.


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


Re: [Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-08-04 Thread Eric Anholt
Keith Packard kei...@keithp.com writes:

 Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 I don't see anything indicating that this code path is only used by
 glamor.

 True. It's a fix for DRI3 for either UXA or none. Mesa allocates a
 single page for a 1x1 texture, but this code thinks that should take two
 pages causing a texture-to-pixmap operation to fail.

OK, but isn't the problem with the code you're #if 0ing (which, really?
#if 0 in a patch being submitted for review?) that it's aligning to
2*height instead of height?


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


[Intel-gfx] [PATCH 1/2] drm/i915: Get CZ clock for VLV

2014-08-04 Thread Vandana Kannan
CZ clock is related to data flow from memory to display plane. This is
required for comparison with CD clock before programming PFI credits.

Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/i915_reg.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 20 
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 174a294..baeb56f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, 
u32 reg, u32 val);
 int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
 
+int valleyview_get_cz_clock_speed(struct drm_device *dev);
+
 #define FORCEWAKE_RENDER   (1  0)
 #define FORCEWAKE_MEDIA(1  1)
 #define FORCEWAKE_ALL  (FORCEWAKE_RENDER | FORCEWAKE_MEDIA)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fe5c276..1b8f095 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -601,6 +601,7 @@ enum punit_power_well {
 #define  DISPLAY_FREQUENCY_STATUS  (0x1f  8)
 #define  DISPLAY_FREQUENCY_STATUS_SHIFT8
 #define  DISPLAY_FREQUENCY_VALUES  (0x1f  0)
+#define CCK_CZ_CONTROL 0x62
 
 /**
  * DOC: DPIO
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 99eb7ca..56a8090 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(struct 
drm_device *dev)
return DIV_ROUND_CLOSEST(vco  1, divider + 1);
 }
 
+int valleyview_get_cz_clock_speed(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int vco = valleyview_get_vco(dev_priv);
+   u32 val;
+   int divider;
+
+   mutex_lock(dev_priv-dpio_lock);
+   val = vlv_cck_read(dev_priv, CCK_CZ_CONTROL);
+   mutex_unlock(dev_priv-dpio_lock);
+
+   divider = val  DISPLAY_FREQUENCY_VALUES;
+
+   WARN((val  DISPLAY_FREQUENCY_STATUS) !=
+(divider  DISPLAY_FREQUENCY_STATUS_SHIFT),
+czclk change in progress\n);
+
+   return DIV_ROUND_CLOSEST(vco  1, divider + 1);
+}
+
 static int i945_get_display_clock_speed(struct drm_device *dev)
 {
return 40;
-- 
2.0.1

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


[Intel-gfx] [PATCH 2/2] drm/i915: Program PFI credits for VLV

2014-08-04 Thread Vandana Kannan
From: Vidya Srinivas vidya.srini...@intel.com

PFI credit programming is required when CD clock (related to data flow from
display pipeline to end display) is greater than CZ clock (related to data
flow from memory to display plane). This programming should be done when all
planes are OFF to avoid intermittent hangs while accessing memory even from
different Gfx units (not just display).

If cdclk/czclk =1, PFI credits could be set as any number. To get better
performance, larger PFI credit can be assigned to PND. Otherwise if
cdclk/czclk1, the default PFI credit of 8 should be set.

v2:
- Change log to lower log level instead of DRM_ERROR
- Change function name to valleyview_program_pfi_credits
- Move program PFI credits to modeset_init instead of intel_set_mode
- Change magic numbers to logical constants

Signed-off-by: Vidya Srinivas vidya.srini...@intel.com
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c  |  6 ++
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/i915_reg.h  |  5 +
 drivers/gpu/drm/i915/intel_display.c |  4 +++-
 drivers/gpu/drm/i915/intel_pm.c  | 22 ++
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b2b649c..73617b5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
 
+   if (IS_VALLEYVIEW(dev))
+   valleyview_program_pfi_credits(dev_priv, false);
+
dev_priv-suspend_count++;
 
intel_display_set_init_power(dev_priv, false);
@@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
restore_gtt_mappings)
dev_priv-modeset_restore = MODESET_DONE;
mutex_unlock(dev_priv-modeset_restore_lock);
 
+   if (IS_VALLEYVIEW(dev))
+   valleyview_program_pfi_credits(dev_priv, true);
+
intel_opregion_notify_adapter(dev, PCI_D0);
 
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index baeb56f..38907ef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2152,6 +2152,8 @@ extern struct i915_params i915 __read_mostly;
 
/* i915_dma.c */
 void i915_update_dri1_breadcrumb(struct drm_device *dev);
+extern void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+  bool flag);
 extern void i915_kernel_lost_context(struct drm_device * dev);
 extern int i915_driver_load(struct drm_device *, unsigned long flags);
 extern int i915_driver_unload(struct drm_device *);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1b8f095..92b8afc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1914,6 +1914,11 @@ enum punit_power_well {
 #define   CZCLK_FREQ_MASK  0xf
 #define GMBUSFREQ_VLV  (VLV_DISPLAY_BASE + 0x6510)
 
+#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
+#define   PFI_CREDIT   (7  28)
+#define   PFI_CREDIT_RESEND(1  27)
+#define   VGA_FAST_MODE_DISABLE (1  14)
+
 /*
  * Palette regs
  */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 56a8090..521943a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12555,8 +12555,10 @@ void intel_modeset_init_hw(struct drm_device *dev)
 {
intel_prepare_ddi(dev);
 
-   if (IS_VALLEYVIEW(dev))
+   if (IS_VALLEYVIEW(dev)) {
vlv_update_cdclk(dev);
+   valleyview_program_pfi_credits(dev-dev_private, true);
+   }
 
intel_init_clock_gating(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3f88f29..fe55c54 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6780,6 +6780,28 @@ void intel_fini_runtime_pm(struct drm_i915_private 
*dev_priv)
pm_runtime_disable(device);
 }
 
+void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+   bool flag)
+{
+   int cd_clk, cz_clk;
+
+   if (!flag) {
+   I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE);
+   return;
+   }
+
+   cd_clk = dev_priv-display.get_display_clock_speed(dev_priv-dev);
+   cz_clk = valleyview_get_cz_clock_speed(dev_priv-dev);
+
+   if (cd_clk = cz_clk) {
+   /* WA - write default credits before re-programming */
+   I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE);
+   I915_WRITE(GCI_CONTROL, (PFI_CREDIT | PFI_CREDIT_RESEND |
+   

Re: [Intel-gfx] [PATCH] drm/i915: Only track real ppgtt for a context

2014-08-04 Thread Thierry, Michel


 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
 Sent: Monday, August 04, 2014 3:21 PM
 To: Intel Graphics Development
 Cc: Daniel Vetter; Thierry, Michel; Ville Syrjälä
 Subject: [PATCH] drm/i915: Only track real ppgtt for a context
 
 There's a bit a confusion since we track the global gtt,
 the aliasing and real ppgtt in the ctx-vm pointer. And not
 all callers really bother to check for the different cases and just
 presume that it points to a real ppgtt.
 
 Now looking closely we don't actually need -vm to always point at an
 address space - the only place that cares actually has fixup code
 already to decide whether to look at the per-proces or the global
 address space.
 
 So switch to just tracking the ppgtt directly and ditch all the
 extraneous code.
 
 v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx-ppgtt.
 Also drop the early exit - without aliasing ppgtt we want to dump all
 the ppgtts of the contexts if we have full ppgtt.
 
 Cc: Thierry, Michel michel.thie...@intel.com
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 OTC-Jira: VIZ-3724
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_debugfs.c| 11 ---
  drivers/gpu/drm/i915/i915_drv.h|  3 +--
  drivers/gpu/drm/i915/i915_gem_context.c| 28 +++-
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 +++--
  drivers/gpu/drm/i915/i915_gpu_error.c  | 10 +++---
  5 files changed, 26 insertions(+), 31 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 3bf1d20c598b..2bd4beada1bd 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data)
  {
   struct intel_context *ctx = ptr;
   struct seq_file *m = data;
 - struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
 + struct i915_hw_ppgtt *ppgtt = ctx-ppgtt;
 +
 + if (!ppgtt) {
 + seq_puts(m,   no ppgtt for context %d\n,
 +  ctx-user_handle);
Should be seq_printf().

 + return 0;
 + }
 
   if (i915_gem_context_is_default(ctx))
   seq_puts(m,   default context:\n);
 @@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m,
 --
 1.9.3

Apart from that, 

Reviewed-by: Michel Thierry michel.thie...@intel.com



smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW

2014-08-04 Thread Rodrigo Vivi
According to spec FBC on BDW and HSW are identical without any gaps.
So let's copy the nuke and let FBC really start compressing stuff.

Without this patch we can verify with false color that nothing is being
compressed. With the nuke in place and false color it is possible
to see false color debugs.

Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on
LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer rend
cache would never been cleaned and FBC would stop compressing buffer.
One alternative is to cache clean on software frontbuffer tracking.

v2: Fix rebase conflict.
v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking.

Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_display.c|  3 +++
 drivers/gpu/drm/i915/intel_pm.c | 10 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +-
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a372f2..25d7365 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct 
drm_device *dev,
 extern void i915_redisable_vga(struct drm_device *dev);
 extern void i915_redisable_vga_power_on(struct drm_device *dev);
 extern bool intel_fbc_enabled(struct drm_device *dev);
+extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);
 extern void intel_disable_fbc(struct drm_device *dev);
 extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
 extern void intel_init_pch_refclk(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 883af0b..c8421cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
 
intel_edp_psr_flush(dev, frontbuffer_bits);
+
+   if (IS_GEN8(dev))
+   gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 684dc5f..de07d3e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv-display.fbc_enabled(dev);
 }
 
+void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
+   if (!IS_GEN8(dev))
+   return;
+
+   I915_WRITE(MSG_FBC_REND_STATE, value);
+}
+
 static void intel_fbc_work_fn(struct work_struct *__work)
 {
struct intel_fbc_work *work =
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2908896..2fe871c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
 {
u32 flags = 0;
u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES;
+   int ret;
 
flags |= PIPE_CONTROL_CS_STALL;
 
@@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
}
 
-   return gen8_emit_pipe_control(ring, flags, scratch_addr);
+   ret = gen8_emit_pipe_control(ring, flags, scratch_addr);
+   if (ret)
+   return ret;
+
+   if (!invalidate_domains  flush_domains)
+   return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
+
+   return 0;
 }
 
 static void ring_write_tail(struct intel_engine_cs *ring,
-- 
1.9.3

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


[Intel-gfx] [PATCH] drm/i915: Only track real ppgtt for a context

2014-08-04 Thread Daniel Vetter
There's a bit a confusion since we track the global gtt,
the aliasing and real ppgtt in the ctx-vm pointer. And not
all callers really bother to check for the different cases and just
presume that it points to a real ppgtt.

Now looking closely we don't actually need -vm to always point at an
address space - the only place that cares actually has fixup code
already to decide whether to look at the per-proces or the global
address space.

So switch to just tracking the ppgtt directly and ditch all the
extraneous code.

v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx-ppgtt.
Also drop the early exit - without aliasing ppgtt we want to dump all
the ppgtts of the contexts if we have full ppgtt.

v3: Actually git add the compile fix.

Reviewed-by: Michel Thierry michel.thie...@intel.com
Cc: Thierry, Michel michel.thie...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
OTC-Jira: VIZ-3724
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_debugfs.c| 11 ---
 drivers/gpu/drm/i915/i915_drv.h|  3 +--
 drivers/gpu/drm/i915/i915_gem_context.c| 28 +++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 +++--
 drivers/gpu/drm/i915/i915_gpu_error.c  | 10 +++---
 5 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3bf1d20c598b..7e8d94b5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data)
 {
struct intel_context *ctx = ptr;
struct seq_file *m = data;
-   struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
+   struct i915_hw_ppgtt *ppgtt = ctx-ppgtt;
+
+   if (!ppgtt) {
+   seq_printf(m,   no ppgtt for context %d\n,
+  ctx-user_handle);
+   return 0;
+   }
 
if (i915_gem_context_is_default(ctx))
seq_puts(m,   default context:\n);
@@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m, struct 
drm_device *dev)
seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd_offset);
 
ppgtt-debug_dump(ppgtt, m);
-   } else
-   return;
+   }
 
list_for_each_entry_reverse(file, dev-filelist, lhead) {
struct drm_i915_file_private *file_priv = file-driver_priv;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0762e19f9bc6..3230b08aff13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -616,7 +616,7 @@ struct intel_context {
uint8_t remap_slice;
struct drm_i915_file_private *file_priv;
struct i915_ctx_hang_stats hang_stats;
-   struct i915_address_space *vm;
+   struct i915_hw_ppgtt *ppgtt;
 
struct {
struct drm_i915_gem_object *rcs_state;
@@ -2504,7 +2504,6 @@ i915_gem_object_ggtt_unbind(struct drm_i915_gem_object 
*obj)
 void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj);
 
 /* i915_gem_context.c */
-#define ctx_to_ppgtt(ctx) container_of((ctx)-vm, struct i915_hw_ppgtt, base)
 int __must_check i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_reset(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 7a455fcee3a7..c00e5d027774 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -136,15 +136,8 @@ void i915_gem_context_free(struct kref *ctx_ref)
 {
struct intel_context *ctx = container_of(ctx_ref,
   typeof(*ctx), ref);
-   struct i915_hw_ppgtt *ppgtt = NULL;
 
-   if (ctx-legacy_hw_ctx.rcs_state) {
-   /* We refcount even the aliasing PPGTT to keep the code 
symmetric */
-   if (USES_PPGTT(ctx-legacy_hw_ctx.rcs_state-base.dev))
-   ppgtt = ctx_to_ppgtt(ctx);
-   }
-
-   i915_ppgtt_put(ppgtt);
+   i915_ppgtt_put(ctx-ppgtt);
if (ctx-legacy_hw_ctx.rcs_state)
drm_gem_object_unreference(ctx-legacy_hw_ctx.rcs_state-base);
list_del(ctx-link);
@@ -240,7 +233,6 @@ i915_gem_create_context(struct drm_device *dev,
bool create_vm)
 {
const bool is_global_default_ctx = file_priv == NULL;
-   struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_context *ctx;
int ret = 0;
 
@@ -274,15 +266,10 @@ i915_gem_create_context(struct drm_device *dev,
 PTR_ERR(ppgtt));
ret = PTR_ERR(ppgtt);
goto err_unpin;
-   } else
-   ctx-vm = ppgtt-base;
-   } else if (USES_PPGTT(dev)) {
-   /* 

[Intel-gfx] [PATCH] drm/i915: Android sync points for i915 v2

2014-08-04 Thread Jesse Barnes
Expose an ioctl to create Android fences based on the Android sync point
infrastructure (which in turn is based on DMA-buf fences).  Just a
sketch at this point, no testing has been done.

There are a couple of goals here:
  1) allow applications and libraries to create fences without an
 associated buffer
  2) re-use a common API so userspace doesn't have to impedance mismatch
 between different driver implementations too much
  3) allow applications and libraries to use explicit synchronization if
 they choose by exposing fences directly

v2: use struct fence directly using Maarten's new interface

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/Kconfig |   2 +
 drivers/gpu/drm/i915/Makefile|   1 +
 drivers/gpu/drm/i915/i915_dma.c  |   1 +
 drivers/gpu/drm/i915/i915_drv.h  |  10 ++
 drivers/gpu/drm/i915/i915_gem.c  |  15 +-
 drivers/gpu/drm/i915/i915_irq.c  |   4 +-
 drivers/gpu/drm/i915/i915_sync.c | 323 +++
 include/uapi/drm/i915_drm.h  |  23 +++
 8 files changed, 373 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_sync.c

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 4e39ab3..cd0f2ec 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -6,6 +6,8 @@ config DRM_I915
select INTEL_GTT
select AGP_INTEL if AGP
select INTERVAL_TREE
+   select ANDROID
+   select SYNC
# we need shmfs for the swappable backing store, and in particular
# the shmem_readpage() which depends upon tmpfs
select SHMEM
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 91bd167..61a3eb5c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -25,6 +25,7 @@ i915-y += i915_cmd_parser.o \
  i915_gem_execbuffer.o \
  i915_gem_gtt.o \
  i915_gem.o \
+ i915_sync.o \
  i915_gem_stolen.o \
  i915_gem_tiling.o \
  i915_gem_userptr.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03a..84086e1 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2043,6 +2043,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_GEM_FENCE, i915_sync_create_fence_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 int i915_max_ioctl = ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d604f4f..6eb119e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1388,6 +1388,8 @@ struct i915_frontbuffer_tracking {
unsigned flip_bits;
 };
 
+struct i915_sync_timeline;
+
 struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1422,6 +1424,8 @@ struct drm_i915_private {
struct drm_i915_gem_object *semaphore_obj;
uint32_t last_seqno, next_seqno;
 
+   struct i915_sync_timeline *sync_tl[I915_NUM_RINGS];
+
drm_dma_handle_t *status_page_dmah;
struct resource mch_res;
 
@@ -2275,6 +2279,12 @@ void i915_init_vm(struct drm_i915_private *dev_priv,
 void i915_gem_free_object(struct drm_gem_object *obj);
 void i915_gem_vma_destroy(struct i915_vma *vma);
 
+/* i915_sync.c */
+int i915_sync_init(struct drm_i915_private *dev_priv);
+void i915_sync_fini(struct drm_i915_private *dev_priv);
+int i915_sync_create_fence_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file);
+
 #define PIN_MAPPABLE 0x1
 #define PIN_NONBLOCK 0x2
 #define PIN_GLOBAL 0x4
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dcd8d7b..ace716e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1146,11 +1146,11 @@ static bool can_wait_boost(struct drm_i915_file_private 
*file_priv)
  * Returns 0 if the seqno was found within the alloted time. Else returns the
  * errno with remaining time filled in timeout argument.
  */
-static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
-   unsigned reset_counter,
-   bool interruptible,
-   struct timespec *timeout,
-   struct drm_i915_file_private *file_priv)
+int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
+unsigned reset_counter,
+bool interruptible,
+struct timespec *timeout,
+struct drm_i915_file_private *file_priv)
 {
struct drm_device *dev = ring-dev;
struct 

[Intel-gfx] [PATCH] drm/i915: lock around link status and link training.

2014-08-04 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

We need to take the connection mutex around the link status
check for non-MST case, but also around the MST link training
on short HPDs.

I suspect we actually should have a dpcd lock in the future as
well, that just lock the local copies of dpcd and flags stored
from that.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0f05b88..145ac7c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3437,6 +3437,7 @@ intel_dp_handle_test_request(struct intel_dp *intel_dp)
 static int
 intel_dp_check_mst_status(struct intel_dp *intel_dp)
 {
+   struct drm_device *dev = intel_dp_to_dev(intel_dp);
bool bret;
 
if (intel_dp-is_mst) {
@@ -3449,12 +3450,14 @@ go_again:
if (bret == true) {
 
/* check link status - esi[10] = 0x200c */
+   drm_modeset_lock(dev-mode_config.connection_mutex, 
NULL);
if (intel_dp-active_mst_links  
!drm_dp_channel_eq_ok(esi[10], intel_dp-lane_count)) {
DRM_DEBUG_KMS(channel EQ not ok, 
retraining\n);
intel_dp_start_link_train(intel_dp);
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
+   drm_modeset_unlock(dev-mode_config.connection_mutex);
 
DRM_DEBUG_KMS(got esi %02x %02x %02x\n, esi[0], 
esi[1], esi[2]);
ret = drm_dp_mst_hpd_irq(intel_dp-mst_mgr, esi, 
handled);
@@ -3502,10 +3505,13 @@ go_again:
 void
 intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
+   struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_encoder *intel_encoder = dp_to_dig_port(intel_dp)-base;
u8 sink_irq_vector;
u8 link_status[DP_LINK_STATUS_SIZE];
 
+   WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex));
+
/* FIXME: This access isn't protected by any locks. */
if (!intel_encoder-connectors_active)
return;
@@ -4021,7 +4027,9 @@ intel_dp_hpd_pulse(struct intel_digital_port 
*intel_dig_port, bool long_hpd)
 * we'll check the link status via the normal hot plug 
path later -
 * but for short hpds we should check it now
 */
+   drm_modeset_lock(dev-mode_config.connection_mutex, 
NULL);
intel_dp_check_link_status(intel_dp);
+   drm_modeset_unlock(dev-mode_config.connection_mutex);
}
}
return false;
-- 
1.9.3

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


[Intel-gfx] [PATCH 2/7] drm/i915: Add thread stall DOP clock gating workaround on Broadwell.

2014-08-04 Thread Rodrigo Vivi
From: Kenneth Graunke kenn...@whitecape.org

Ben and I believe this will be necessary on production hardware.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 684dc5f..f919596 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5412,6 +5412,10 @@ static void gen8_init_clock_gating(struct drm_device 
*dev)
I915_WRITE(GEN8_ROW_CHICKEN,
   _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
 
+   /* WaDisableThreadStallDopClockGating:bdw */
+   I915_WRITE(GEN8_ROW_CHICKEN,
+  _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
+
/*
 * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for
 * pre-production hardware
-- 
1.9.3

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


[Intel-gfx] [PATCH 1/7] drm/i915/bdw: Always issue a force restore

2014-08-04 Thread Rodrigo Vivi
From: Ben Widawsky benjamin.widaw...@intel.com

The PDPs seem to get screwed up otherwise, specifically PDP0. I am not
really clear why this is required, it just works with full PPGTT.

v2: Only do it for gen8, to limit regression potential

v3: Fix the bugzilla links

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78891
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78935
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78936
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78937
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78938

Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 3b99390..56f7b1e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -586,6 +586,9 @@ mi_set_context(struct intel_engine_cs *ring,
else
intel_ring_emit(ring, MI_NOOP);
 
+   if (INTEL_INFO(ring-dev)-gen == 8)
+   hw_flags |= MI_FORCE_RESTORE;
+
intel_ring_emit(ring, MI_NOOP);
intel_ring_emit(ring, MI_SET_CONTEXT);
intel_ring_emit(ring, 
i915_gem_obj_ggtt_offset(new_context-legacy_hw_ctx.rcs_state) |
-- 
1.9.3

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


[Intel-gfx] [PATCH 6/7] drm/i915: BDW Semaphore signal with Post Sync

2014-08-04 Thread Rodrigo Vivi
With this bit set MI_SEMAPHORE_SIGNAL command is executed as a pipelined
PIPE_CONTROL flush command with Semaphore Signal as post sync operation.
However this can only be set only when Fixed Function DOP Clock Gate Disable
is set.

This brought a bit of stability on Semaphores minimizing the hangs.

Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc13961..27a54a0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -269,6 +269,7 @@
 #define MI_SEMAPHORE_SIGNALMI_INSTR(0x1b, 0) /* GEN8+ */
 #define   MI_SEMAPHORE_TARGET(engine)  ((engine)15)
 #define MI_SEMAPHORE_WAIT  MI_INSTR(0x1c, 2) /* GEN8+ */
+#define   MI_SEMAPHORE_POST_SYNC   (121)
 #define   MI_SEMAPHORE_POLL(115)
 #define   MI_SEMAPHORE_SAD_GTE_SDD (112)
 #define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f919596..ab8cbda 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5481,6 +5481,9 @@ static void gen8_init_clock_gating(struct drm_device *dev)
I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL,
   _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE));
 
+   I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL,
+  _MASKED_BIT_ENABLE(GEN8_FF_DOP_CLOCK_GATE_DISABLE));
+
/* WaDisableSDEUnitClockGating:bdw */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cd30b39..edb8234 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -730,6 +730,7 @@ static int gen8_rcs_signal(struct intel_engine_cs 
*signaller,
intel_ring_emit(signaller, signaller-outstanding_lazy_seqno);
intel_ring_emit(signaller, 0);
intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
+  MI_SEMAPHORE_POST_SYNC |
   MI_SEMAPHORE_TARGET(waiter-id));
intel_ring_emit(signaller, 0);
}
-- 
1.9.3

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


[Intel-gfx] [PATCH 3/7] drm/i915/bdw: MI_FLUSH_DW a qword instead of dword

2014-08-04 Thread Rodrigo Vivi
From: Ben Widawsky benjamin.widaw...@intel.com

The actual post sync op is Write Immediate Data QWord. It is therefore
arguable that we should have always done a qword write.

The actual impetus for this patch is our decoder complains when we write
a dword and I was trying to eliminate the spurious errors. With this
patch, I've noticed a really strange reproducible error turns into a
different strange reproducible error - so it does indeed have some
effect of some sort.

This was also recommended to me by someone that is familiar with the
Windows driver.

It's based on top of the semaphore series, so it won't be easily
applied, I'd guess.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 95 +
 1 file changed, 74 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2908896..9a562b5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -727,7 +727,7 @@ static int gen8_rcs_signal(struct intel_engine_cs 
*signaller,
 static int gen8_xcs_signal(struct intel_engine_cs *signaller,
   unsigned int num_dwords)
 {
-#define MBOX_UPDATE_DWORDS 6
+#define MBOX_UPDATE_DWORDS 8
struct drm_device *dev = signaller-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_engine_cs *waiter;
@@ -746,15 +746,18 @@ static int gen8_xcs_signal(struct intel_engine_cs 
*signaller,
if (gtt_offset == MI_SEMAPHORE_SYNC_INVALID)
continue;
 
-   intel_ring_emit(signaller, (MI_FLUSH_DW + 1) |
+   intel_ring_emit(signaller, (MI_FLUSH_DW + 2) |
   MI_FLUSH_DW_OP_STOREDW);
intel_ring_emit(signaller, lower_32_bits(gtt_offset) |
   MI_FLUSH_DW_USE_GTT);
intel_ring_emit(signaller, upper_32_bits(gtt_offset));
intel_ring_emit(signaller, signaller-outstanding_lazy_seqno);
+   intel_ring_emit(signaller, 0); /* upper dword */
+
intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
   MI_SEMAPHORE_TARGET(waiter-id));
intel_ring_emit(signaller, 0);
+   intel_ring_emit(signaller, MI_NOOP);
}
 
return 0;
@@ -1939,8 +1942,6 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs 
*ring,
return ret;
 
cmd = MI_FLUSH_DW;
-   if (INTEL_INFO(ring-dev)-gen = 8)
-   cmd += 1;
/*
 * Bspec vol 1c.5 - video engine command streamer:
 * If ENABLED, all TLBs will be invalidated once the flush
@@ -1952,13 +1953,38 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs 
*ring,
MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW;
intel_ring_emit(ring, cmd);
intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT);
-   if (INTEL_INFO(ring-dev)-gen = 8) {
-   intel_ring_emit(ring, 0); /* upper addr */
-   intel_ring_emit(ring, 0); /* value */
-   } else  {
-   intel_ring_emit(ring, 0);
-   intel_ring_emit(ring, MI_NOOP);
-   }
+   intel_ring_emit(ring, 0);
+   intel_ring_emit(ring, MI_NOOP);
+   intel_ring_advance(ring);
+   return 0;
+}
+
+static int gen8_bsd_ring_flush(struct intel_engine_cs *ring,
+  u32 invalidate, u32 flush)
+{
+   uint32_t cmd;
+   int ret;
+
+   ret = intel_ring_begin(ring, 6);
+   if (ret)
+   return ret;
+
+   cmd = MI_FLUSH_DW + 2;
+   /*
+* Bspec vol 1c.5 - video engine command streamer:
+* If ENABLED, all TLBs will be invalidated once the flush
+* operation is complete. This bit is only valid when the
+* Post-Sync Operation field is a value of 1h or 3h.
+*/
+   if (invalidate  I915_GEM_GPU_DOMAINS)
+   cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD |
+   MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW;
+   intel_ring_emit(ring, cmd);
+   intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT);
+   intel_ring_emit(ring, 0); /* upper addr */
+   intel_ring_emit(ring, 0); /* value */
+   intel_ring_emit(ring, 0); /* value */
+   intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
 }
@@ -2029,8 +2055,38 @@ gen6_ring_dispatch_execbuffer(struct intel_engine_cs 
*ring,
return 0;
 }
 
-/* Blitter support (SandyBridge+) */
+static int gen8_ring_flush(struct intel_engine_cs *ring,
+  u32 invalidate, u32 flush)
+{
+   uint32_t cmd;
+   int ret;
+
+   ret = intel_ring_begin(ring, 6);
+   if (ret)
+   

[Intel-gfx] [PATCH 4/7] drm/i915/bdw: cs-stall before state cache invld w/a

2014-08-04 Thread Rodrigo Vivi
From: Ben Widawsky benjamin.widaw...@intel.com

We do this already for previous GENs. I guess we must do it for BDW too
according to DOCS.

Pipe_control with CS-stall bit set must be issued before a
pipe-control command that has the State Cache Invalidate bit set.

This does not solve the problem I have unfortunately.

I didn't check if this was in Ville's CHV series. If it was, I
apologize.

NOTE: I tried to use smaller lengths for the command, but nothing made
it happy except 6.

Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Jordan Justen jljus...@gmail.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9a562b5..2e566e0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -279,17 +279,25 @@ gen6_render_ring_flush(struct intel_engine_cs *ring,
 static int
 gen7_render_ring_cs_stall_wa(struct intel_engine_cs *ring)
 {
-   int ret;
+   int ret, size = 4;
 
-   ret = intel_ring_begin(ring, 4);
+   if (IS_BROADWELL(ring-dev))
+   size += 2;
+
+   ret = intel_ring_begin(ring, size);
if (ret)
return ret;
 
-   intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
+   intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(size));
intel_ring_emit(ring, PIPE_CONTROL_CS_STALL |
  PIPE_CONTROL_STALL_AT_SCOREBOARD);
intel_ring_emit(ring, 0);
intel_ring_emit(ring, 0);
+   if (IS_BROADWELL(ring-dev)) {
+   intel_ring_emit(ring, 0);
+   intel_ring_emit(ring, 0);
+   }
+
intel_ring_advance(ring);
 
return 0;
@@ -422,6 +430,11 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
flags |= PIPE_CONTROL_QW_WRITE;
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+
+   /* Workaround: we must issue a pipe_control with CS-stall bit
+* set before a pipe_control command that has the state cache
+* invalidate bit set. */
+   gen7_render_ring_cs_stall_wa(ring);
}
 
return gen8_emit_pipe_control(ring, flags, scratch_addr);
-- 
1.9.3

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


[Intel-gfx] [PATCH 5/7] drm/i915: avoid emiting semaphore wait on GEN8 when seqno wrap happened.

2014-08-04 Thread Rodrigo Vivi
C: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 30 +-
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2e566e0..cd30b39 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -867,15 +867,26 @@ gen8_ring_sync(struct intel_engine_cs *waiter,
if (ret)
return ret;
 
-   intel_ring_emit(waiter, MI_SEMAPHORE_WAIT |
-   MI_SEMAPHORE_GLOBAL_GTT |
-   MI_SEMAPHORE_POLL |
-   MI_SEMAPHORE_SAD_GTE_SDD);
-   intel_ring_emit(waiter, seqno);
-   intel_ring_emit(waiter,
-   lower_32_bits(GEN8_WAIT_OFFSET(waiter, signaller-id)));
-   intel_ring_emit(waiter,
-   upper_32_bits(GEN8_WAIT_OFFSET(waiter, signaller-id)));
+   /* If seqno wrap happened, omit the wait with no-ops */
+   if (likely(!i915_gem_has_seqno_wrapped(waiter-dev, seqno))) {
+   intel_ring_emit(waiter, MI_SEMAPHORE_WAIT |
+   MI_SEMAPHORE_GLOBAL_GTT |
+   MI_SEMAPHORE_POLL |
+   MI_SEMAPHORE_SAD_GTE_SDD);
+   intel_ring_emit(waiter, seqno);
+   intel_ring_emit(waiter,
+   lower_32_bits(GEN8_WAIT_OFFSET(waiter,
+  signaller-id)));
+   intel_ring_emit(waiter,
+   upper_32_bits(GEN8_WAIT_OFFSET(waiter,
+  signaller-id)));
+   } else {
+   intel_ring_emit(waiter, MI_NOOP);
+   intel_ring_emit(waiter, MI_NOOP);
+   intel_ring_emit(waiter, MI_NOOP);
+   intel_ring_emit(waiter, MI_NOOP);
+   }
+
intel_ring_advance(waiter);
return 0;
 }
@@ -2572,6 +2583,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
ring-semaphore.mbox.signal[BCS] = GEN6_BVESYNC;
ring-semaphore.mbox.signal[VECS] = GEN6_NOSYNC;
ring-semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
+
}
}
ring-init = init_ring_common;
-- 
1.9.3

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


[Intel-gfx] [PATCH 7/7] Revert drm/i915: Enable semaphores on BDW

2014-08-04 Thread Rodrigo Vivi
This reverts commit 521e62e49a42661a4ee0102644517dbe2f100a23.

Although POST_SYNC brought a bit of stability to Semaphores on BDW
it didn't solved all issues and some hungs can still occour when
semaphores are enabled on BDW. Also some sloweness can be found on some
igt tests, althoguth it apparently doesn't affect real workloads.

Besides that, no real performance gain was found on our tests with different
and even multiple workloads.

Let's disable it again for now. At least until we are sure it is safe
to re-enable it.

Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..ec96f9a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -481,6 +481,10 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
if (i915.semaphores = 0)
return i915.semaphores;
 
+   /* Until we get further testing... */
+   if (IS_GEN8(dev))
+   return false;
+
 #ifdef CONFIG_INTEL_IOMMU
/* Enable semaphores on SNB when IO remapping is off */
if (INTEL_INFO(dev)-gen == 6  intel_iommu_gfx_mapped)
-- 
1.9.3

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


Re: [Intel-gfx] [PATCH 1/7] drm/i915/bdw: Always issue a force restore

2014-08-04 Thread Ben Widawsky
On Mon, Aug 04, 2014 at 11:15:13AM -0700, Rodrigo Vivi wrote:
 From: Ben Widawsky benjamin.widaw...@intel.com
 
 The PDPs seem to get screwed up otherwise, specifically PDP0. I am not
 really clear why this is required, it just works with full PPGTT.
 
 v2: Only do it for gen8, to limit regression potential
 
 v3: Fix the bugzilla links
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78891
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78935
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78936
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78937
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78938
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com

Hi Rodrigo. This patch  was superseded by my [partially merged] series
here:
http://lists.freedesktop.org/archives/intel-gfx/2014-July/048311.html

In that series I went through very thoroughly with design why we need to
do this, and verified those patches do address this issue.

So yeah, NAK from me - and please get those reworked or merged or
whatever.

Thanks

[snip]


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


Re: [Intel-gfx] [PATCH 3/7] drm/i915/bdw: MI_FLUSH_DW a qword instead of dword

2014-08-04 Thread Ben Widawsky
On Mon, Aug 04, 2014 at 11:15:15AM -0700, Rodrigo Vivi wrote:
 From: Ben Widawsky benjamin.widaw...@intel.com
 
 The actual post sync op is Write Immediate Data QWord. It is therefore
 arguable that we should have always done a qword write.
 
 The actual impetus for this patch is our decoder complains when we write
 a dword and I was trying to eliminate the spurious errors. With this
 patch, I've noticed a really strange reproducible error turns into a
 different strange reproducible error - so it does indeed have some
 effect of some sort.
 
 This was also recommended to me by someone that is familiar with the
 Windows driver.
 
 It's based on top of the semaphore series, so it won't be easily
 applied, I'd guess.
 
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com

Did we determine this was needed for anything other than shutting up the
instruction decoder, for debug? Seems like if the existing stuff ain't
broke, don't fix it.

[snip]


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


Re: [Intel-gfx] [PATCH] drm: Docbook fixes

2014-08-04 Thread Dave Airlie
On 30 July 2014 23:36, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 Bunch of small leftovers spotted by looking at the make htmldocs output.

 I've left out dp mst, there's too much amiss there.

(btw this patch doesn't apply cleanly - modeset_lock seems different).

Just spotted this comment!
I do wonder if I should really be documenting the internals of that
struct in docbook,

The main reason its there is to embed in the driver, I don't think
drivers should be
looking inside it too often, and we certainly shouldn't be generating info about
most of the members in the interface docs for driver writers.

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


Re: [Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-08-04 Thread Keith Packard
Eric Anholt e...@anholt.net writes:

 OK, but isn't the problem with the code you're #if 0ing (which, really?
 #if 0 in a patch being submitted for review?) that it's aligning to
 2*height instead of height?

I went and did a bit of archaeology to figure out why it's using
2*height instead of just height. The initial change to pixmap
allocation, which was then copied to the BO validation logic, was here:

commit 736b89504a32239a0c7dfb5961c1b8292dd744bd
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Sun Dec 30 10:32:18 2012 +

uxa: Align surface allocations to even tile rows

Align surface sizes to an even number of tile rows to cater for 
sampler
prefetch. If we read beyond the last page we may catch the PTE in a
state of flux and trigger a GPU hang. Also detected by enabling 
invalid
PTE access checking.

References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index f5ac0a6..2f14173 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -209,7 +209,7 @@ intel_uxa_pixmap_compute_size(PixmapPtr pixmap,
tile_height = 8;
else
tile_height = 32;
-   aligned_h = ALIGN(h, tile_height);
+   aligned_h = ALIGN(h, 2*tile_height);
 
*stride = intel_get_fence_pitch(intel,
ALIGN(pitch, 512),

Look at the referenced bugs, what I found was a long list of random GPU
hangs on ILK and SNB hardware that appear to have been caused by a
kernel change. Daniel and Chris created a number of scatter shot fixes
across the kernel, and this patch to the 2D driver. However, this patch
doesn't appear to have actually solved anything; Norbert Preining was
still crashing with this patch applied:

https://bugs.freedesktop.org/show_bug.cgi?id=55984#c129

Furthermore, SNA has some similar code, but it applies it conditionally
for hardware which doesn't have 'relaxed fencing', which is only
hardware older than i965 when running on a older kernel that doesn't
recognize the HAS_RELAXED_FENCING parameter (the current kernel always
returns 'true').

So, as near as I can tell, this fix should never be necessary as the
reported bug wasn't fixed by it, and SNA does not apply this rule to any
hardware on which either bug was reproduced.

If this fix is actually useful, wouldn't we want it in Mesa as well?

-- 
keith.pack...@intel.com


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