[Intel-gfx] [PATCH] drm/i915: Check hangcheck is functioning before indefinite waits

[Chris Wilson]

Since we rely on hangcheck to wait up and kick us out of an indefinite
wait should the GPU ever stop functioning, it appears sensible that we
should check that hangcheck is indeed active before starting that wait.
This just prevents a driver error in the processing of hangcheck from
appearing to hang the machine.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8670d05..c326a99 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2149,6 +2149,7 @@ extern void intel_console_resume(struct work_struct 
*work);
 
 /* i915_irq.c */
 void i915_queue_hangcheck(struct drm_device *dev);
+void i915_check_hangcheck(struct drm_device *dev);
 __printf(3, 4)
 void i915_handle_error(struct drm_device *dev, bool wedged,
   const char *fmt, ...);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ecb835b..120ed6d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1455,6 +1455,8 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 
seqno,
break;
}
 
+   i915_check_hangcheck(ring-dev);
+
timer.function = NULL;
if (timeout || missed_irq(dev_priv, ring)) {
unsigned long expire;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9e5a295..daa590e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3063,6 +3063,17 @@ void i915_queue_hangcheck(struct drm_device *dev)
  round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
 }
 
+void i915_check_hangcheck(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   if (!i915.enable_hangcheck)
+   return;
+
+   if (!timer_pending(dev_priv-gpu_error.hangcheck_timer))
+   mod_timer(dev_priv-gpu_error.hangcheck_timer,
+ round_jiffies_up(jiffies + 
DRM_I915_HANGCHECK_JIFFIES));
+}
+
 static void ibx_irq_reset(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4f3397f..6365d4d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1634,6 +1634,8 @@ static int ring_wait_for_space(struct intel_engine_cs 
*ring, int n)
master_priv-sarea_priv-perf_boxes |= 
I915_BOX_WAIT;
}
 
+   i915_check_hangcheck(dev);
+
io_schedule_timeout(1);
 
if (dev_priv-mm.interruptible  signal_pending(current)) {
-- 
1.9.1

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

[Intel-gfx] [PATCH] drm/i915: Revert drm/i915: Reject the pin ioctl on gen6+

[Damien Lespiau]

This reverts commit 02f6bcccf7c324115747aae2f0addd6af5d321cd.

The OA buffer can contain global data (in particular, not linked to a
context or a single batch execution) about GPU events (eg. hw context
switches, rc6 transitions, frequency changes, ...) and needs to be
mapped to GGTT. The pin ioctl provided a way to do that.

Admittedly, this change broke what seems to be a valid use case of
pinning a buffer in GGTT, even when PPGTT is used (which is the reason
invoked in the commit message).

Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Tomasz Madajczak tomasz.madajc...@intel.com
Cc: Adam Rutkowski adam.j.rutkow...@intel.com

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_gem.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1794a04..8019809 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4143,9 +4143,6 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj;
int ret;
 
-   if (INTEL_INFO(dev)-gen = 6)
-   return -ENODEV;
-
ret = i915_mutex_lock_interruptible(dev);
if (ret)
return ret;
-- 
1.8.3.1

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

Re: [Intel-gfx] pin OABUFFER to GGTT

[Damien Lespiau]

On Wed, Jul 02, 2014 at 02:19:42PM +0100, Rutkowski, Adam J wrote:
 Having said all this, how about restoring the pin_ioctl? At least for
 some time? We do have a use case and moving to other - better -
 solution would take time. I think backward compatibility is something
 that you take into consideration as well.

So, I just sent a patch reverting the change. Daniel will have an
opinion about this I'm sure, being the original author. Let see what
happens when he's back from holidays.

Cheers,

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

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

[Michael S. Tsirkin]

On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote:
 On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote:
  Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto:
  With this long thread I lost a bit context about the challenges
  that exists. But let me try summarizing it here - which will hopefully
  get some consensus.
  
  1). Fix IGD hardware to not use Southbridge magic addresses.
  We can moan and moan but I doubt it is going to change.
  
  There are two problems:
  
  - Northbridge (i.e. MCH i.e. PCI host bridge) configuration space addresses
 
 Right. So in  drivers/gpu/drm/i915/i915_dma.c:
 1135 #define MCHBAR_I915 0x44 

 1136 #define MCHBAR_I965 0x48 
 
 1147 int reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : MCHBAR_I915; 

 1152 if (INTEL_INFO(dev)-gen = 4)   

 1153 pci_read_config_dword(dev_priv-bridge_dev, reg + 4, 
 temp_hi); 
 1154 pci_read_config_dword(dev_priv-bridge_dev, reg, temp_lo);  

 1155 mchbar_addr = ((u64)temp_hi  32) | temp_lo;
 
 and
 
 1139 #define DEVEN_REG 0x54   

 
 1193 int mchbar_reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
 MCHBAR_I915; 
 1202 if (IS_I915G(dev) || IS_I915GM(dev)) {   

 1203 pci_read_config_dword(dev_priv-bridge_dev, DEVEN_REG, 
 temp);  
 1204 enabled = !!(temp  DEVEN_MCHBAR_EN);

 1205 } else { 

 1206 pci_read_config_dword(dev_priv-bridge_dev, mchbar_reg, 
 temp); 
 1207 enabled = temp  1;  

 1208 }
  
  - Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some versions of
  the driver identify it by class, some versions identify it by slot (1f.0).
 
 Right, So in  drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch
 which sets the pch_type based on :
 
  432 if (pch-vendor == PCI_VENDOR_ID_INTEL) {

  433 unsigned short id = pch-device  
 INTEL_PCH_DEVICE_ID_MASK;
  434 dev_priv-pch_id = id;   

  435  

  436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) { 
 
 It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00.
 The INTEL_PCH_DEVICE_ID_MASK is 0xff00
  
  To solve the first, make a new machine type, PIIX4-based, and pass through
  the registers you need.  The patch must document _exactly_ why the registers
  are safe to pass.  If they are not reserved on PIIX4, the patch must
  document what the same offsets mean on PIIX4, and why it's sensible to
  assume that firmware for virtual machine will not read/write them.  Bonus
  point for also documenting the same for Q35.
 
 OK. They look to be related to setting up an MBAR , but I don't understand
 why it is needed. Hopefully some of the i915 folks CC-ed here can answer.

In particular, I think setting up MBAR should move out of i915 and become
the bridge driver tweak on linux and windows.
It would then run on the priveledged host
and we won't need to deal with it in QEMU.


  
  Regarding the second, fixing IGD hardware to not rely on chipset magic is a
  no-go, I agree.  I disagree that it's a no-go to define a backdoor that
  lets a hypervisor pass the right information to the driver without hacking
  the chipset device model.
  
  The hardware folks would have to give us a place for a pair of registers
  (something like data/address), and a bit somewhere else that would be always
  0 on hardware and always 1 if the hypervisor is implementing the pair of
  registers.  This is similar to CPUID, which has the HYPERVISOR bit +
  hypervisor-defined leaves at 0x4000.
  
  The data/address pair could be in a BAR, in configuration space, in the low
  VGA ports at 0x3c0-0x3df, wherever.  The hypervisor bit can be in the same
  place or somewhere else---again, whatever is convenient for the hardware
  folks.  We just need *one bit* that is known-zero on all hardware, and 8
  bytes in a reserved area.  I don't think it's too hard to find this space,
  and I really, really would like Intel to follow up on a paravirtualized
  backdoor.
  
  That said, we have the problem of existing guests, so I agree something else
  is needed.
  
   a) Two bridges - one 'passthrough' and the legacy ISA bridge
  that QEMU emulates. Both Linux and Windows are OK with
  two bridges (even thought it is pretty weird).
  
  This is pretty much the only solution for existing Linux 

Re: [Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload submission mechanism from execbuffer

[Chris Wilson]

On Thu, Jun 26, 2014 at 02:24:19PM +0100, oscar.ma...@intel.com wrote:
 + i915_gem_execbuffer_move_to_active(vmas, ring);
 + i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);

This is where I start freaking out over the mix of global vs logical
state and the implications of reordering.

The key question for me is how clean busy-ioctl is when it has to pick
up the pieces from a partial failure to submit.
-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 v2 1/2] drm/i915: provide interface for audio driver to query cdclk

[Jani Nikula]

On Thu, 03 Jul 2014, mengdong@intel.com wrote:
 From: Jani Nikula jani.nik...@intel.com

I wrote this as a quick hack patch to try as an alternative to [1] which
ended up not working on Haswell. Please reassure me that this is going
to be a temporary solution until we get a more generic interface between
the audio and display drivers. I don't much like this, but at least it's
isolated and small.

I'd like the commit message amended with something like:


If the display power well has been disabled, the display audio
controller divider values EM4 MVALUE and EM5 NVALUE will have been
lost. The CDCLK frequency is required for reprogramming them. Provide a
private interface for the audio driver to query CDCLK.

This is a stopgap solution until a more generic interface between audio
and display drivers has been implemented.


I'd also like to have an additional Reviewed-by from the i915
side. After that, I'm fine with merging this through alsa.

BR,
Jani.


[1] http://mid.gmane.org/cover.1404222047.git.jani.nik...@intel.com



 Signed-off-by: Jani Nikula jani.nik...@intel.com
 Signed-off-by: Mengdong Lin mengdong@intel.com

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index a90fdbd..21170e5 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -6256,6 +6256,27 @@ int i915_release_power_well(void)
  }
  EXPORT_SYMBOL_GPL(i915_release_power_well);
  
 +/*
 + * Private interface for the audio driver to get CDCLK in kHz.
 + *
 + * Caller must request power well using i915_request_power_well() prior to
 + * making the call.
 + */
 +int i915_get_cdclk_freq(void)
 +{
 + struct drm_i915_private *dev_priv;
 +
 + if (!hsw_pwr)
 + return -ENODEV;
 +
 + dev_priv = container_of(hsw_pwr, struct drm_i915_private,
 + power_domains);
 +
 + return intel_ddi_get_cdclk_freq(dev_priv);
 +}
 +EXPORT_SYMBOL_GPL(i915_get_cdclk_freq);
 +
 +
  #define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
  
  #define HSW_ALWAYS_ON_POWER_DOMAINS (\
 diff --git a/include/drm/i915_powerwell.h b/include/drm/i915_powerwell.h
 index 2baba99..baa6f11 100644
 --- a/include/drm/i915_powerwell.h
 +++ b/include/drm/i915_powerwell.h
 @@ -32,5 +32,6 @@
  /* For use by hda_i915 driver */
  extern int i915_request_power_well(void);
  extern int i915_release_power_well(void);
 +extern int i915_get_cdclk_freq(void);
  
  #endif   /* _I915_POWERWELL_H_ */
 -- 
 1.8.1.2

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

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

Re: [Intel-gfx] [PATCH] drm/i915: Revert drm/i915: Reject the pin ioctl on gen6+

[Chris Wilson]

On Thu, Jul 03, 2014 at 08:12:35AM +0100, Damien Lespiau wrote:
 This reverts commit 02f6bcccf7c324115747aae2f0addd6af5d321cd.
 
 The OA buffer can contain global data (in particular, not linked to a
 context or a single batch execution) about GPU events (eg. hw context
 switches, rc6 transitions, frequency changes, ...) and needs to be
 mapped to GGTT. The pin ioctl provided a way to do that.

 Admittedly, this change broke what seems to be a valid use case of
 pinning a buffer in GGTT, even when PPGTT is used (which is the reason
 invoked in the commit message).
 
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Tomasz Madajczak tomasz.madajc...@intel.com
 Cc: Adam Rutkowski adam.j.rutkow...@intel.com
 
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Acked-by: Chris Wilson ch...@chris-wilson.co.uk # who didn't like his tools
being confiscated
-Chris

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

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

[Jani Nikula]

Hi Dave -

Fixes for 3.16-rc3; most importantly Jesse brings back VGA he took away
on a bunch of machines. Also a vblank fix for BDW and a power workaround
fix for VLV.

BR,
Jani.

The following changes since commit 8525a235c96a548873c6c5644f50df32b31f04c6:

  drm/i915: vlv_prepare_pll is only needed in case of non DSI interfaces 
(2014-06-25 11:22:18 +0300)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-07-03

for you to fetch changes up to 5549d25f642a7e6cfb8744d0031a9da404f696d6:

  drm/i915: Drop early VLV WA to fix Voltage not getting dropped to Vmin 
(2014-07-01 11:43:14 +0300)


Deepak S (1):
  drm/i915: Drop early VLV WA to fix Voltage not getting dropped to Vmin

Jesse Barnes (1):
  drm/i915: only apply crt_present check on VLV

Ville Syrjälä (1):
  drm/i915: Wait for vblank after enabling the primary plane on BDW

 drivers/gpu/drm/i915/intel_display.c | 27 ++-
 drivers/gpu/drm/i915/intel_pm.c  |  8 
 drivers/gpu/drm/i915/intel_sprite.c  |  8 
 3 files changed, 42 insertions(+), 1 deletion(-)


-- 
Jani Nikula, 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 8/8] drm/i915: Extract the actual workload submission mechanism from execbuffer

[Mateo Lozano, Oscar]

 -Original Message-
 From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
 Sent: Thursday, July 03, 2014 8:32 AM
 To: Mateo Lozano, Oscar
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload
 submission mechanism from execbuffer
 
 On Thu, Jun 26, 2014 at 02:24:19PM +0100, oscar.ma...@intel.com wrote:
  +   i915_gem_execbuffer_move_to_active(vmas, ring);
  +   i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
 
 This is where I start freaking out over the mix of global vs logical state and
 the implications of reordering.

I hear you, Chris. This is scary stuff because it has a lot of implications, 
but it has been in review for a long time and we seem to be stalled.

 The key question for me is how clean busy-ioctl is when it has to pick up the
 pieces from a partial failure to submit.

AFAICT, __i915_add_request() needs some changes if we don´t want to reuse the 
intel_ringbuffer.c functionality (Daniel was wondering why I needed to abstract 
__i915_add_request away with another vfunc: this is the reason) and 
i915_gem_retire_requests_ring() needs to update last_retired_head in the 
correct ringbuf (global or logical). Other than that, everything seems healthy, 
as things like the list of active objects and the list of requests reside on 
intel_engine_cs, so they are common for both paths.

How could I put your mind at ease? maybe by creating a topic branch and going 
through QA for a while? or merging it as disabled by default? I don´t feel 
rewriting it forever is going to make it any better :(
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload submission mechanism from execbuffer

[Chris Wilson]

On Thu, Jul 03, 2014 at 09:07:41AM +, Mateo Lozano, Oscar wrote:
  -Original Message-
  From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
  Sent: Thursday, July 03, 2014 8:32 AM
  To: Mateo Lozano, Oscar
  Cc: intel-gfx@lists.freedesktop.org
  Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload
  submission mechanism from execbuffer
  
  On Thu, Jun 26, 2014 at 02:24:19PM +0100, oscar.ma...@intel.com wrote:
   + i915_gem_execbuffer_move_to_active(vmas, ring);
   + i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
  
  This is where I start freaking out over the mix of global vs logical state 
  and
  the implications of reordering.
 
 I hear you, Chris. This is scary stuff because it has a lot of implications, 
 but it has been in review for a long time and we seem to be stalled.
 
  The key question for me is how clean busy-ioctl is when it has to pick up 
  the
  pieces from a partial failure to submit.
 
 AFAICT, __i915_add_request() needs some changes if we don´t want to reuse the 
 intel_ringbuffer.c functionality (Daniel was wondering why I needed to 
 abstract __i915_add_request away with another vfunc: this is the reason) and 
 i915_gem_retire_requests_ring() needs to update last_retired_head in the 
 correct ringbuf (global or logical). Other than that, everything seems 
 healthy, as things like the list of active objects and the list of requests 
 reside on intel_engine_cs, so they are common for both paths.
 
 How could I put your mind at ease? maybe by creating a topic branch and going 
 through QA for a while? or merging it as disabled by default? I don´t feel 
 rewriting it forever is going to make it any better :(

At the moment, I feel happy to merge as-is and then folding back in the new
functionality. The problem with relying solely on i-g-t here is that I
am concerned about the nasty sort of races that only appear on Linus's
machine. My feeling is that some of the more tricksy and problematic global
ring state is actually part of the request state.

I am feeling more confident that we can make intel_context our view of
the hardware state and hang the legacy plumbing off it, and that can be
done afterwards. Though it looks more and more like we need to make the
request as the central object through which we program the hardware.
-Chris

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

Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle

[Chris Wilson]

On Thu, Jun 26, 2014 at 02:24:15PM +0100, oscar.ma...@intel.com wrote:
 From: Oscar Mateo oscar.ma...@intel.com
 
 This is an Execlists preparatory patch, since they make context ID become an
 overloaded term:
 
 - In the software, it was used to distinguish which context userspace was
   trying to use.
 - In the BSpec, the term is used to describe the 20-bits long field the
   hardware uses to it to discriminate the contexts that are submitted to
   the ELSP and inform the driver about their current status (via Context
   Switch Interrupts and Context Status Buffers).
 
 Initially, I tried to make the different meanings converge, but it proved
 impossible:
 
 - The software ctx-id is per-filp, while the hardware one needs to be
   globally unique.
 - Also, we multiplex several backing states objects per intel_context,
   and all of them need unique HW IDs.
 - I tried adding a per-filp ID and then composing the HW context ID as:
   ctx-id + file_priv-id + ring-id, but the fact that the hardware only
   uses 20-bits means we have to artificially limit the number of filps or
   contexts the userspace can create.
 
 The ctx-handle bits are done with this Cocci patch (plus manual frobbing
 of the struct declaration):
 
   @@
   struct intel_context c;
   @@
   - (c).id
   + c.handle
 
   @@
   struct intel_context *c;
   @@
   - (c)-id
   + c-handle
 
 Also, while we are at it, s/DEFAULT_CONTEXT_ID/DEFAULT_CONTEXT_HANDLE.
 
 Signed-off-by: Oscar Mateo oscar.ma...@intel.com

Let's go whole hog here and call it ctx-user_handle.
-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 v2 1/2] drm/i915: provide interface for audio driver to query cdclk

[Lin, Mengdong]

Hi Jani,

 -Original Message-
 From: Nikula, Jani
 Sent: Thursday, July 03, 2014 3:33 PM
 
 I wrote this as a quick hack patch to try as an alternative to [1] which
 ended up not working on Haswell. Please reassure me that this is going to
 be a temporary solution until we get a more generic interface between the
 audio and display drivers. I don't much like this, but at least it's isolated
 and small.

Sure. We'll clean up code when the generic interface is ready. 
This is only a temporary solution, safer than the BIOS notifications.

And would you like to further revise this patch? 
But please keep this API not changed at the moment since the audio driver 
already queries the symbol name.

Thanks
Mengdong

 I'd like the commit message amended with something like:
 
 
 If the display power well has been disabled, the display audio controller
 divider values EM4 MVALUE and EM5 NVALUE will have been lost. The
 CDCLK frequency is required for reprogramming them. Provide a private
 interface for the audio driver to query CDCLK.
 
 This is a stopgap solution until a more generic interface between audio
 and display drivers has been implemented.
 
 
 I'd also like to have an additional Reviewed-by from the i915 side. After
 that, I'm fine with merging this through alsa.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-rcs_state

[Chris Wilson]

On Thu, Jun 26, 2014 at 02:24:13PM +0100, oscar.ma...@intel.com wrote:
 From: Oscar Mateo oscar.ma...@intel.com
 
 This is Execlists preparatory work.
 
 We have already advanced that Logical Ring Contexts have their own kind
 ob backing objects, but everything will be better explained in the Execlists
 series. For now, suffice it to say that this backing object is only
 ever used with the render ring, so we're making this fact more explicit
 (which is a good reason on its own).
 
 Done with the following Coccinelle patch (plus manual renaming of the
 struct field):
 
   @@
   struct intel_context c;
   @@
   - (c).obj
   + c.rcs_state
 
   @@
*c;
   @@
   - (c)-obj
   + c-rcs_state
 
 No functional changes.
 
 v2: Go with rcs_state instead of render_obj, as suggested by Chris Wilson.

Another little change here is ctx-is_initialised if you create
  struct {
  struct drm_i915_gem_object *rcs_state;
  bool initialised;
  } legacy_hw_ctx;
that should also address Daniel's confusion.
-Chris

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

[Intel-gfx] [PATCH 2/2] drm/i915: Update the DSI ULPS entry/exit sequence

[Shobhit Kumar]

We should keep DEVICE_READY bit set in the ULPS enter sequence. In
exit sequence also we should set DEVICE_READY, but thats causing
blankout for me. Also exit sequence is simplified as per hw team
recommendation.

This should fix -
[drm:intel_dsi_clear_device_ready] *ERROR* DSI LP not going Low

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80818
Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com
---
 drivers/gpu/drm/i915/intel_dsi.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index a6a1355..967a5b6 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -117,17 +117,18 @@ static void intel_dsi_device_ready(struct intel_encoder 
*encoder)
/* bandgap reset is needed after everytime we do power gate */
band_gap_reset(dev_priv);
 
+   I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER);
+   usleep_range(2500, 3000);
+
val = I915_READ(MIPI_PORT_CTRL(pipe));
I915_WRITE(MIPI_PORT_CTRL(pipe), val | LP_OUTPUT_HOLD);
usleep_range(1000, 1500);
-   I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_EXIT);
-   usleep_range(2000, 2500);
-   I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY);
-   usleep_range(2000, 2500);
-   I915_WRITE(MIPI_DEVICE_READY(pipe), 0x00);
-   usleep_range(2000, 2500);
+
+   I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT);
+   usleep_range(2500, 3000);
+
I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY);
-   usleep_range(2000, 2500);
+   usleep_range(2500, 3000);
 }
 
 static void intel_dsi_enable(struct intel_encoder *encoder)
@@ -271,23 +272,23 @@ static void intel_dsi_clear_device_ready(struct 
intel_encoder *encoder)
 
DRM_DEBUG_KMS(\n);
 
-   I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER);
+   I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_ENTER);
usleep_range(2000, 2500);
 
-   I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT);
+   I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_EXIT);
usleep_range(2000, 2500);
 
-   I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER);
+   I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_ENTER);
usleep_range(2000, 2500);
 
-   val = I915_READ(MIPI_PORT_CTRL(pipe));
-   I915_WRITE(MIPI_PORT_CTRL(pipe), val  ~LP_OUTPUT_HOLD);
-   usleep_range(1000, 1500);
-
if (wait_for(((I915_READ(MIPI_PORT_CTRL(pipe))  AFE_LATCHOUT)
== 0x0), 30))
DRM_ERROR(DSI LP not going Low\n);
 
+   val = I915_READ(MIPI_PORT_CTRL(pipe));
+   I915_WRITE(MIPI_PORT_CTRL(pipe), val  ~LP_OUTPUT_HOLD);
+   usleep_range(1000, 1500);
+
I915_WRITE(MIPI_DEVICE_READY(pipe), 0x00);
usleep_range(2000, 2500);
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/2] drm/i915: DPI FIFO empty check is not needed

[Shobhit Kumar]

While sending DPI SHUTDOWN command, we cannot wait for FIFO empty as
pipes are not disabled at that time. In case of MIPI we disable port
first and send SHUTDOWN command while pipe is still running and FIFOs
will not be empty, causing spurious error log

Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com
---
 drivers/gpu/drm/i915/intel_dsi_cmd.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c 
b/drivers/gpu/drm/i915/intel_dsi_cmd.c
index 3eeb21b..933c863 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
@@ -404,12 +404,6 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, 
bool hs)
else
cmd |= DPI_LP_MODE;
 
-   /* DPI virtual channel?! */
-
-   mask = DPI_FIFO_EMPTY;
-   if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(pipe))  mask) == mask, 50))
-   DRM_ERROR(Timeout waiting for DPI FIFO empty.\n);
-
/* clear bit */
I915_WRITE(MIPI_INTR_STAT(pipe), SPL_PKT_SENT_INTERRUPT);
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio driver to query cdclk

[Damien Lespiau]

On Thu, Jul 03, 2014 at 10:32:38AM +0300, Jani Nikula wrote:
 On Thu, 03 Jul 2014, mengdong@intel.com wrote:
  From: Jani Nikula jani.nik...@intel.com
 
 I wrote this as a quick hack patch to try as an alternative to [1] which
 ended up not working on Haswell. Please reassure me that this is going
 to be a temporary solution until we get a more generic interface between
 the audio and display drivers. I don't much like this, but at least it's
 isolated and small.
 
 I'd like the commit message amended with something like:
 
 
 If the display power well has been disabled, the display audio
 controller divider values EM4 MVALUE and EM5 NVALUE will have been
 lost. The CDCLK frequency is required for reprogramming them. Provide a
 private interface for the audio driver to query CDCLK.
 
 This is a stopgap solution until a more generic interface between audio
 and display drivers has been implemented.
 
 
 I'd also like to have an additional Reviewed-by from the i915
 side. After that, I'm fine with merging this through alsa.

Sure.

Reviewed-by: Damien Lespiau damien.lesp...@intel.com

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

Re: [Intel-gfx] [PATCH] drm/i915: Try harder to get FBC

[Jani Nikula]

On Tue, 01 Jul 2014, Rodrigo Vivi rodrigo.v...@gmail.com wrote:
 Jani, please ignore the 4th patch on this series and merge the 3 I've
 reviewed and tested already.

Patches 1-3 pushed to dinq. Thanks for the patches and review.

BR,
Jani.


 They are essential to allow FBC work on BDW without changing bios
 configuration and allow PC7 residency.

 Thanks,
 Rodrigo.


 On Mon, Jun 30, 2014 at 10:41 AM, Rodrigo Vivi rodrigo.v...@intel.com
 wrote:

 From: Ben Widawsky benjamin.widaw...@intel.com

 The GEN FBC unit provides the ability to set a low pass on frames it
 attempts to compress. If a frame is less than a certain amount
 compressibility (2:1, 4:1) it will not bother. This allows the driver to
 reduce the size it requests out of stolen memory.

 Unluckily, a few months ago, Ville actually began using this feature for
 framebuffers that are 16bpp (not sure why not 8bpp). In those cases, we
 are already using this mechanism for a different purpose, and so we can
 only achieve one further level of compression (2:1 - 4:1)

 FBC GEN1, ie. pre-G45 is ignored.

 The cleverness of the patch is Art's. The bugs are mine.

 v2: Update message and including missing threshold case 3 (Spotted by
 Arthur).

 Reviewedby: Rodrigo Vivi rodrigo.v...@intel.com
 Cc: Art Runyan arthur.j.run...@intel.com
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h|  3 +-
  drivers/gpu/drm/i915/i915_gem_stolen.c | 54
 +-
  drivers/gpu/drm/i915/intel_pm.c| 30 +--
  3 files changed, 69 insertions(+), 18 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h
 b/drivers/gpu/drm/i915/i915_drv.h
 index 5b7aed2..9953ea8 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -600,6 +600,7 @@ struct intel_context {

  struct i915_fbc {
 unsigned long size;
 +   unsigned threshold;
 unsigned int fb_id;
 enum plane plane;
 int y;
 @@ -2489,7 +2490,7 @@ static inline void i915_gem_chipset_flush(struct
 drm_device *dev)

  /* i915_gem_stolen.c */
  int i915_gem_init_stolen(struct drm_device *dev);
 -int i915_gem_stolen_setup_compression(struct drm_device *dev, int size);
 +int i915_gem_stolen_setup_compression(struct drm_device *dev, int size,
 int fb_cpp);
  void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
  void i915_gem_cleanup_stolen(struct drm_device *dev);
  struct drm_i915_gem_object *
 diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
 b/drivers/gpu/drm/i915/i915_gem_stolen.c
 index a86b331..b695d18 100644
 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
 +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
 @@ -105,35 +105,61 @@ static unsigned long i915_stolen_to_physical(struct
 drm_device *dev)

  static int find_compression_threshold(struct drm_device *dev,
   struct drm_mm_node *node,
 - int size)
 + int size,
 + int fb_cpp)
  {
 struct drm_i915_private *dev_priv = dev-dev_private;
 -   const int compression_threshold = 1;
 +   int compression_threshold = 1;
 int ret;

 -   /* Try to over-allocate to reduce reallocations and fragmentation
 */
 +   /* HACK: This code depends on what we will do in *_enable_fbc. If
 that
 +* code changes, this code needs to change as well.
 +*
 +* The enable_fbc code will attempt to use one of our 2 compression
 +* thresholds, therefore, in that case, we only have 1 resort.
 +*/
 +
 +   /* Try to over-allocate to reduce reallocations and fragmentation.
 */
 ret = drm_mm_insert_node(dev_priv-mm.stolen, node,
  size = 1, 4096, DRM_MM_SEARCH_DEFAULT);
 -   if (ret)
 -   ret = drm_mm_insert_node(dev_priv-mm.stolen, node,
 -size = 1, 4096,
 -DRM_MM_SEARCH_DEFAULT);
 -   if (ret)
 +   if (ret == 0)
 +   return compression_threshold;
 +
 +again:
 +   /* HW's ability to limit the CFB is 1:4 */
 +   if (compression_threshold  4 ||
 +   (fb_cpp == 2  compression_threshold == 2))
 return 0;
 -   else
 +
 +   ret = drm_mm_insert_node(dev_priv-mm.stolen, node,
 +size = 1, 4096,
 +DRM_MM_SEARCH_DEFAULT);
 +   if (ret  INTEL_INFO(dev)-gen = 4) {
 +   return 0;
 +   } else if (ret) {
 +   compression_threshold = 1;
 +   goto again;
 +   } else {
 return compression_threshold;
 +   }
  }

 -static int i915_setup_compression(struct drm_device *dev, int size)
 +static int i915_setup_compression(struct drm_device *dev, int size, int
 fb_cpp)
  {
 

Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle

[Mateo Lozano, Oscar]

 -Original Message-
 From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
 Sent: Thursday, July 03, 2014 10:53 AM
 To: Mateo Lozano, Oscar; intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
 
 On Thu, Jul 03, 2014 at 10:30:52AM +0100, Chris Wilson wrote:
  On Thu, Jun 26, 2014 at 02:24:15PM +0100, oscar.ma...@intel.com
 wrote:
   From: Oscar Mateo oscar.ma...@intel.com
  
   This is an Execlists preparatory patch, since they make context ID
   become an overloaded term:
  
   - In the software, it was used to distinguish which context userspace was
 trying to use.
   - In the BSpec, the term is used to describe the 20-bits long field the
 hardware uses to it to discriminate the contexts that are submitted to
 the ELSP and inform the driver about their current status (via Context
 Switch Interrupts and Context Status Buffers).
  
   Initially, I tried to make the different meanings converge, but it
   proved
   impossible:
  
   - The software ctx-id is per-filp, while the hardware one needs to be
 globally unique.
   - Also, we multiplex several backing states objects per intel_context,
 and all of them need unique HW IDs.
   - I tried adding a per-filp ID and then composing the HW context ID as:
 ctx-id + file_priv-id + ring-id, but the fact that the hardware only
 uses 20-bits means we have to artificially limit the number of filps or
 contexts the userspace can create.
  
   The ctx-handle bits are done with this Cocci patch (plus manual
   frobbing of the struct declaration):
  
 @@
 struct intel_context c;
 @@
 - (c).id
 + c.handle
  
 @@
 struct intel_context *c;
 @@
 - (c)-id
 + c-handle
  
   Also, while we are at it,
 s/DEFAULT_CONTEXT_ID/DEFAULT_CONTEXT_HANDLE.
  
   Signed-off-by: Oscar Mateo oscar.ma...@intel.com
 
  Let's go whole hog here and call it ctx-user_handle.

ACK

 And it's unsigned and only 32bits...

ACK, I´ll change the type to unit32_t
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-rcs_state

[Mateo Lozano, Oscar]

 -Original Message-
 From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
 Sent: Thursday, July 03, 2014 10:47 AM
 To: Mateo Lozano, Oscar
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-
 rcs_state
 
 On Thu, Jun 26, 2014 at 02:24:13PM +0100, oscar.ma...@intel.com wrote:
  From: Oscar Mateo oscar.ma...@intel.com
 
  This is Execlists preparatory work.
 
  We have already advanced that Logical Ring Contexts have their own
  kind ob backing objects, but everything will be better explained in
  the Execlists series. For now, suffice it to say that this backing
  object is only ever used with the render ring, so we're making this
  fact more explicit (which is a good reason on its own).
 
  Done with the following Coccinelle patch (plus manual renaming of the
  struct field):
 
  @@
  struct intel_context c;
  @@
  - (c).obj
  + c.rcs_state
 
  @@
   *c;
  @@
  - (c)-obj
  + c-rcs_state
 
  No functional changes.
 
  v2: Go with rcs_state instead of render_obj, as suggested by Chris Wilson.
 
 Another little change here is ctx-is_initialised if you create
   struct {
 struct drm_i915_gem_object *rcs_state;
 bool initialised;
   } legacy_hw_ctx;
 that should also address Daniel's confusion.
 -Chris

Daniel said exactly the same thing, but I´m reusing the rcs_initialized field 
in Execlists to mark the default render context as ready after setting the 
golden/null state (so that I only do it after module load, and not after 
reset/thaw). I can add a new field for this, but IMHO this one makes sense.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/8] drm/i915: Rename ctx-is_initialized to ctx-rcs_is_initialized

[Mateo Lozano, Oscar]

 -Original Message-
 From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
 Sent: Thursday, July 03, 2014 10:49 AM
 To: Mateo Lozano, Oscar
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 3/8] drm/i915: Rename ctx-is_initialized to
 ctx-rcs_is_initialized
 
 On Thu, Jun 26, 2014 at 02:24:14PM +0100, oscar.ma...@intel.com wrote:
  From: Oscar Mateo oscar.ma...@intel.com
 
  We only use this flag to signify that the render state (a.k.a. golden
  context, a.k.a. null context) has been initialized. It doesn't mean
  anything for the other engines, so make that distinction obvious.
  This renaming was suggested by Daniel Vetter.
 
  Implemented with this cocci script (plus manual changes to the struct
  declaration):
 
  @
  struct intel_context c;
  @@
  - (c).is_initialized
  + c.rcs_is_initialized
 
  @@
  struct intel_context *c;
  @@
  - (c)-is_initialized
  + c-rcs_is_initialized
 
  No functional changes.
 
  Signed-off-by: Oscar Mateo oscar.ma...@intel.com
 
 Ugh. This works better with the rearrangement, and
 s/rcs_is_initialized/rcs_initialised/

ACK to the renaming. Re the rearrangement, I replied in the previous 
conservation.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-rcs_state

[Chris Wilson]

On Thu, Jul 03, 2014 at 12:08:42PM +, Mateo Lozano, Oscar wrote:
  -Original Message-
  From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
  Sent: Thursday, July 03, 2014 10:47 AM
  To: Mateo Lozano, Oscar
  Cc: intel-gfx@lists.freedesktop.org
  Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-
  rcs_state
  
  On Thu, Jun 26, 2014 at 02:24:13PM +0100, oscar.ma...@intel.com wrote:
   From: Oscar Mateo oscar.ma...@intel.com
  
   This is Execlists preparatory work.
  
   We have already advanced that Logical Ring Contexts have their own
   kind ob backing objects, but everything will be better explained in
   the Execlists series. For now, suffice it to say that this backing
   object is only ever used with the render ring, so we're making this
   fact more explicit (which is a good reason on its own).
  
   Done with the following Coccinelle patch (plus manual renaming of the
   struct field):
  
 @@
 struct intel_context c;
 @@
 - (c).obj
 + c.rcs_state
  
 @@
  *c;
 @@
 - (c)-obj
 + c-rcs_state
  
   No functional changes.
  
   v2: Go with rcs_state instead of render_obj, as suggested by Chris Wilson.
  
  Another little change here is ctx-is_initialised if you create
struct {
struct drm_i915_gem_object *rcs_state;
bool initialised;
} legacy_hw_ctx;
  that should also address Daniel's confusion.
  -Chris
 
 Daniel said exactly the same thing, but I´m reusing the rcs_initialized field 
 in Execlists to mark the default render context as ready after setting the 
 golden/null state (so that I only do it after module load, and not after 
 reset/thaw). I can add a new field for this, but IMHO this one makes sense.

I would isolate the two. The use may be mutually exclusive, but the
semantics are not...
-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 2/2] drm/i915: Update the DSI ULPS entry/exit sequence

[Chris Wilson]

On Thu, Jul 03, 2014 at 04:35:41PM +0530, Shobhit Kumar wrote:
 We should keep DEVICE_READY bit set in the ULPS enter sequence. In
 exit sequence also we should set DEVICE_READY, but thats causing
 blankout for me. Also exit sequence is simplified as per hw team
 recommendation.
 
 This should fix -
 [drm:intel_dsi_clear_device_ready] *ERROR* DSI LP not going Low
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80818
 Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com

They silence that warning and appears to still work,
Tested-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

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

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-rcs_state

[Mateo Lozano, Oscar]

 -Original Message-
 From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
 Sent: Thursday, July 03, 2014 1:28 PM
 To: Mateo Lozano, Oscar
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to ctx-
 rcs_state
 
 On Thu, Jul 03, 2014 at 12:08:42PM +, Mateo Lozano, Oscar wrote:
   -Original Message-
   From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
   Sent: Thursday, July 03, 2014 10:47 AM
   To: Mateo Lozano, Oscar
   Cc: intel-gfx@lists.freedesktop.org
   Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rename ctx-obj to
   ctx-
   rcs_state
  
   On Thu, Jun 26, 2014 at 02:24:13PM +0100, oscar.ma...@intel.com
 wrote:
From: Oscar Mateo oscar.ma...@intel.com
   
This is Execlists preparatory work.
   
We have already advanced that Logical Ring Contexts have their own
kind ob backing objects, but everything will be better explained
in the Execlists series. For now, suffice it to say that this
backing object is only ever used with the render ring, so we're
making this fact more explicit (which is a good reason on its own).
   
Done with the following Coccinelle patch (plus manual renaming of
the struct field):
   
@@
struct intel_context c;
@@
- (c).obj
+ c.rcs_state
   
@@
 *c;
@@
- (c)-obj
+ c-rcs_state
   
No functional changes.
   
v2: Go with rcs_state instead of render_obj, as suggested by Chris
 Wilson.
  
   Another little change here is ctx-is_initialised if you create
 struct {
   struct drm_i915_gem_object *rcs_state;
   bool initialised;
 } legacy_hw_ctx;
   that should also address Daniel's confusion.
   -Chris
 
  Daniel said exactly the same thing, but I´m reusing the rcs_initialized 
  field in
 Execlists to mark the default render context as ready after setting the
 golden/null state (so that I only do it after module load, and not after
 reset/thaw). I can add a new field for this, but IMHO this one makes sense.
 
 I would isolate the two. The use may be mutually exclusive, but the
 semantics are not...

But... are we arguing or *debating* semantics? :)
Ok, I´ll rearrange the HW context stuff and create a separate initialized flag 
for the Execlists case.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Aggressively downclock Baytrail

[Chris Wilson]

Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
downclock if less than 70% busy over 450ms). This causes Baytrail to use
maximum clocks, and for them to stay high, when doing simple tasks such as
scrolling through webpages. However, we can take a leaf out of the same
wait-boost mechansim and apply the aggressive downclocking strategy from
Sandybridge+ as well.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7005ea3595e2..009f81a922e4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3090,14 +3090,6 @@ static void gen6_set_rps_thresholds(struct 
drm_i915_private *dev_priv, u8 val)
/* Downclock if less than 85% busy over 32ms */
I915_WRITE(GEN6_RP_DOWN_EI, 25000);
I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 21250);
-
-   I915_WRITE(GEN6_RP_CONTROL,
-  GEN6_RP_MEDIA_TURBO |
-  GEN6_RP_MEDIA_HW_NORMAL_MODE |
-  GEN6_RP_MEDIA_IS_GFX |
-  GEN6_RP_ENABLE |
-  GEN6_RP_UP_BUSY_AVG |
-  GEN6_RP_DOWN_IDLE_AVG);
break;
 
case BETWEEN:
@@ -3108,14 +3100,6 @@ static void gen6_set_rps_thresholds(struct 
drm_i915_private *dev_priv, u8 val)
/* Downclock if less than 75% busy over 32ms */
I915_WRITE(GEN6_RP_DOWN_EI, 25000);
I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 18750);
-
-   I915_WRITE(GEN6_RP_CONTROL,
-  GEN6_RP_MEDIA_TURBO |
-  GEN6_RP_MEDIA_HW_NORMAL_MODE |
-  GEN6_RP_MEDIA_IS_GFX |
-  GEN6_RP_ENABLE |
-  GEN6_RP_UP_BUSY_AVG |
-  GEN6_RP_DOWN_IDLE_AVG);
break;
 
case HIGH_POWER:
@@ -3126,17 +3110,17 @@ static void gen6_set_rps_thresholds(struct 
drm_i915_private *dev_priv, u8 val)
/* Downclock if less than 60% busy over 32ms */
I915_WRITE(GEN6_RP_DOWN_EI, 25000);
I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 15000);
-
-   I915_WRITE(GEN6_RP_CONTROL,
-  GEN6_RP_MEDIA_TURBO |
-  GEN6_RP_MEDIA_HW_NORMAL_MODE |
-  GEN6_RP_MEDIA_IS_GFX |
-  GEN6_RP_ENABLE |
-  GEN6_RP_UP_BUSY_AVG |
-  GEN6_RP_DOWN_IDLE_AVG);
break;
}
 
+   I915_WRITE(GEN6_RP_CONTROL,
+  GEN6_RP_MEDIA_TURBO |
+  GEN6_RP_MEDIA_HW_NORMAL_MODE |
+  GEN6_RP_MEDIA_IS_GFX |
+  GEN6_RP_ENABLE |
+  GEN6_RP_UP_BUSY_AVG |
+  GEN6_RP_DOWN_IDLE_AVG);
+
dev_priv-rps.power = new_power;
dev_priv-rps.last_adj = 0;
 }
@@ -3210,29 +3194,25 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
+   u32 val = dev_priv-rps.min_freq_softlimit;
+
/*
 * When we are idle.  Drop to min voltage state.
 */
-
-   if (dev_priv-rps.cur_freq = dev_priv-rps.min_freq_softlimit)
+   if (dev_priv-rps.cur_freq = val)
return;
 
/* Mask turbo interrupt so that they will not come in between */
I915_WRITE(GEN6_PMINTRMSK, 0x);
-
vlv_force_gfx_clock(dev_priv, true);
 
-   dev_priv-rps.cur_freq = dev_priv-rps.min_freq_softlimit;
-
-   vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
-   dev_priv-rps.min_freq_softlimit);
-
+   gen6_set_rps_thresholds(dev_priv, val);
+   vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
 GENFREQSTATUS) == 0, 5))
DRM_ERROR(timed out waiting for Punit\n);
 
vlv_force_gfx_clock(dev_priv, false);
-
I915_WRITE(GEN6_PMINTRMSK,
   gen6_rps_pm_mask(dev_priv, dev_priv-rps.cur_freq));
 }
@@ -3280,8 +3260,10 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 dev_priv-rps.cur_freq,
 vlv_gpu_freq(dev_priv, val), val);
 
-   if (val != dev_priv-rps.cur_freq)
+   if (val != dev_priv-rps.cur_freq) {
+   gen6_set_rps_thresholds(dev_priv, val);
vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+   }
 
I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
-- 
2.0.1

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio driver to query cdclk

[Lin, Mengdong]

 -Original Message-
 From: Lespiau, Damien
 Sent: Thursday, July 03, 2014 7:19 PM
 To: Nikula, Jani
 Cc: Lin, Mengdong; alsa-de...@alsa-project.org; ti...@suse.de;
 intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio
 driver to query cdclk
 
 On Thu, Jul 03, 2014 at 10:32:38AM +0300, Jani Nikula wrote:
  On Thu, 03 Jul 2014, mengdong@intel.com wrote:
   From: Jani Nikula jani.nik...@intel.com
 
  I wrote this as a quick hack patch to try as an alternative to [1]
  which ended up not working on Haswell. Please reassure me that this is
  going to be a temporary solution until we get a more generic interface
  between the audio and display drivers. I don't much like this, but at
  least it's isolated and small.
 
  I'd like the commit message amended with something like:
 
  
  If the display power well has been disabled, the display audio
  controller divider values EM4 MVALUE and EM5 NVALUE will have been
  lost. The CDCLK frequency is required for reprogramming them. Provide
  a private interface for the audio driver to query CDCLK.
 
  This is a stopgap solution until a more generic interface between
  audio and display drivers has been implemented.
  
 
  I'd also like to have an additional Reviewed-by from the i915 side.
  After that, I'm fine with merging this through alsa.
 
 Sure.
 
 Reviewed-by: Damien Lespiau damien.lesp...@intel.com
 
 --
 Damien

Thank you, Jani and Damien!

I'll add the comment message and submit the revised patch tomorrow.

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

[Intel-gfx] [PATCH 7/8] drm/i915: Generalize intel_ring_get_tail to take a ringbuf

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

Again, it's low-level enough to simply take a ringbuf and nothing
else.

Trivial change.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/i915_gem.c | 4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f6d1238..ac7d50a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2330,7 +2330,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
u32 request_ring_position, request_start;
int ret;
 
-   request_start = intel_ring_get_tail(ring);
+   request_start = intel_ring_get_tail(ring-buffer);
/*
 * Emit any outstanding flushes - execbuf can fail to emit the flush
 * after having emitted the batchbuffer command. Hence we need to fix
@@ -2351,7 +2351,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
 * GPU processing the request, we never over-estimate the
 * position of the head.
 */
-   request_ring_position = intel_ring_get_tail(ring);
+   request_ring_position = intel_ring_get_tail(ring-buffer);
 
ret = ring-add_request(ring);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index e72017b..070568b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -318,9 +318,9 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev);
 u64 intel_ring_get_active_head(struct intel_engine_cs *ring);
 void intel_ring_setup_status_page(struct intel_engine_cs *ring);
 
-static inline u32 intel_ring_get_tail(struct intel_engine_cs *ring)
+static inline u32 intel_ring_get_tail(struct intel_ringbuffer *ringbuf)
 {
-   return ring-buffer-tail;
+   return ringbuf-tail;
 }
 
 static inline u32 intel_ring_get_seqno(struct intel_engine_cs *ring)
-- 
1.9.0

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

[Intel-gfx] [PATCH 6/8] drm/i915: Generalize ring_space to take a ringbuf

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

It's simple enough that it doesn't need to know anything about the
engine.

Trivial change.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ffdb366..405edec 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -48,9 +48,8 @@ static inline int __ring_space(int head, int tail, int size)
return space;
 }
 
-static inline int ring_space(struct intel_engine_cs *ring)
+static inline int ring_space(struct intel_ringbuffer *ringbuf)
 {
-   struct intel_ringbuffer *ringbuf = ring-buffer;
return __ring_space(ringbuf-head  HEAD_ADDR, ringbuf-tail, 
ringbuf-size);
 }
 
@@ -545,7 +544,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
else {
ringbuf-head = I915_READ_HEAD(ring);
ringbuf-tail = I915_READ_TAIL(ring)  TAIL_ADDR;
-   ringbuf-space = ring_space(ring);
+   ringbuf-space = ring_space(ringbuf);
ringbuf-last_retired_head = -1;
}
 
@@ -1537,7 +1536,7 @@ static int intel_ring_wait_request(struct intel_engine_cs 
*ring, int n)
ringbuf-head = ringbuf-last_retired_head;
ringbuf-last_retired_head = -1;
 
-   ringbuf-space = ring_space(ring);
+   ringbuf-space = ring_space(ringbuf);
if (ringbuf-space = n)
return 0;
}
@@ -1560,7 +1559,7 @@ static int intel_ring_wait_request(struct intel_engine_cs 
*ring, int n)
ringbuf-head = ringbuf-last_retired_head;
ringbuf-last_retired_head = -1;
 
-   ringbuf-space = ring_space(ring);
+   ringbuf-space = ring_space(ringbuf);
return 0;
 }
 
@@ -1589,7 +1588,7 @@ static int ring_wait_for_space(struct intel_engine_cs 
*ring, int n)
trace_i915_ring_wait_begin(ring);
do {
ringbuf-head = I915_READ_HEAD(ring);
-   ringbuf-space = ring_space(ring);
+   ringbuf-space = ring_space(ringbuf);
if (ringbuf-space = n) {
ret = 0;
break;
@@ -1641,7 +1640,7 @@ static int intel_wrap_ring_buffer(struct intel_engine_cs 
*ring)
iowrite32(MI_NOOP, virt++);
 
ringbuf-tail = 0;
-   ringbuf-space = ring_space(ring);
+   ringbuf-space = ring_space(ringbuf);
 
return 0;
 }
-- 
1.9.0

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

[Intel-gfx] [PATCH 5/8] drm/i915: Extract ringbuffer destroy generalize alloc to take a ringbuf

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

More prep work: with Execlists, we are going to start creating a lot
of extra ringbuffers soon, so these functions are handy.

No functional changes.

v2: rename allocate/destroy_ring_buffer to alloc/destroy_ringbuffer_obj
because the name is more meaningful and to mirror a similar function in
the context world: i915_gem_alloc_context_obj(). Change suggested by Brad
Volkin.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2faef26..ffdb366 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1380,15 +1380,25 @@ static int init_phys_status_page(struct intel_engine_cs 
*ring)
return 0;
 }
 
-static int allocate_ring_buffer(struct intel_engine_cs *ring)
+static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
+{
+   if (!ringbuf-obj)
+   return;
+
+   iounmap(ringbuf-virtual_start);
+   i915_gem_object_ggtt_unpin(ringbuf-obj);
+   drm_gem_object_unreference(ringbuf-obj-base);
+   ringbuf-obj = NULL;
+}
+
+static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
+ struct intel_ringbuffer *ringbuf)
 {
-   struct drm_device *dev = ring-dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   struct intel_ringbuffer *ringbuf = ring-buffer;
struct drm_i915_gem_object *obj;
int ret;
 
-   if (intel_ring_initialized(ring))
+   if (ringbuf-obj)
return 0;
 
obj = NULL;
@@ -1460,7 +1470,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
goto error;
}
 
-   ret = allocate_ring_buffer(ring);
+   ret = intel_alloc_ringbuffer_obj(dev, ringbuf);
if (ret) {
DRM_ERROR(Failed to allocate ringbuffer %s: %d\n, ring-name, 
ret);
goto error;
@@ -1501,11 +1511,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs 
*ring)
intel_stop_ring_buffer(ring);
WARN_ON(!IS_GEN2(ring-dev)  (I915_READ_MODE(ring)  MODE_IDLE) == 0);
 
-   iounmap(ringbuf-virtual_start);
-
-   i915_gem_object_ggtt_unpin(ringbuf-obj);
-   drm_gem_object_unreference(ringbuf-obj-base);
-   ringbuf-obj = NULL;
+   intel_destroy_ringbuffer_obj(ringbuf);
ring-preallocated_lazy_request = NULL;
ring-outstanding_lazy_seqno = 0;
 
-- 
1.9.0

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

[Intel-gfx] [PATCH 1/8] drm/i915: Extract context backing object allocation

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

This is preparatory work for Execlists: we plan to use it later to
allocate our own context objects (since Logical Ring Contexts do
not have the same kind of backing objects).

No functional changes.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_context.c | 54 +
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 0d2c75b..9f8d78b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -198,6 +198,36 @@ void i915_gem_context_free(struct kref *ctx_ref)
kfree(ctx);
 }
 
+static struct drm_i915_gem_object *
+i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
+{
+   struct drm_i915_gem_object *obj;
+   int ret;
+
+   obj = i915_gem_alloc_object(dev, size);
+   if (obj == NULL)
+   return ERR_PTR(-ENOMEM);
+
+   /*
+* Try to make the context utilize L3 as well as LLC.
+*
+* On VLV we don't have L3 controls in the PTEs so we
+* shouldn't touch the cache level, especially as that
+* would make the object snooped which might have a
+* negative performance impact.
+*/
+   if (INTEL_INFO(dev)-gen = 7  !IS_VALLEYVIEW(dev)) {
+   ret = i915_gem_object_set_cache_level(obj, I915_CACHE_L3_LLC);
+   /* Failure shouldn't ever happen this early */
+   if (WARN_ON(ret)) {
+   drm_gem_object_unreference(obj-base);
+   return ERR_PTR(ret);
+   }
+   }
+
+   return obj;
+}
+
 static struct i915_hw_ppgtt *
 create_vm_for_ctx(struct drm_device *dev, struct intel_context *ctx)
 {
@@ -234,27 +264,13 @@ __create_hw_context(struct drm_device *dev,
list_add_tail(ctx-link, dev_priv-context_list);
 
if (dev_priv-hw_context_size) {
-   ctx-obj = i915_gem_alloc_object(dev, 
dev_priv-hw_context_size);
-   if (ctx-obj == NULL) {
-   ret = -ENOMEM;
+   struct drm_i915_gem_object *obj =
+   i915_gem_alloc_context_obj(dev, 
dev_priv-hw_context_size);
+   if (IS_ERR(obj)) {
+   ret = PTR_ERR(obj);
goto err_out;
}
-
-   /*
-* Try to make the context utilize L3 as well as LLC.
-*
-* On VLV we don't have L3 controls in the PTEs so we
-* shouldn't touch the cache level, especially as that
-* would make the object snooped which might have a
-* negative performance impact.
-*/
-   if (INTEL_INFO(dev)-gen = 7  !IS_VALLEYVIEW(dev)) {
-   ret = i915_gem_object_set_cache_level(ctx-obj,
- 
I915_CACHE_L3_LLC);
-   /* Failure shouldn't ever happen this early */
-   if (WARN_ON(ret))
-   goto err_out;
-   }
+   ctx-obj = obj;
}
 
/* Default context will never have a file_priv */
-- 
1.9.0

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

[Intel-gfx] [PATCH 3/8] drm/i915: Emphasize that ctx-id is merely a user handle

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

This is an Execlists preparatory patch, since they make context ID become an
overloaded term:

- In the software, it was used to distinguish which context userspace was
  trying to use.
- In the BSpec, the term is used to describe the 20-bits long field the
  hardware uses to it to discriminate the contexts that are submitted to
  the ELSP and inform the driver about their current status (via Context
  Switch Interrupts and Context Status Buffers).

Initially, I tried to make the different meanings converge, but it proved
impossible:

- The software ctx-id is per-filp, while the hardware one needs to be
  globally unique.
- Also, we multiplex several backing states objects per intel_context,
  and all of them need unique HW IDs.
- I tried adding a per-filp ID and then composing the HW context ID as:
  ctx-id + file_priv-id + ring-id, but the fact that the hardware only
  uses 20-bits means we have to artificially limit the number of filps or
  contexts the userspace can create.

The ctx-user_handle renaming bits are done with this Cocci patch (plus
manual frobbing of the struct declaration):

@@
struct intel_context c;
@@
- (c).id
+ c.user_handle

@@
struct intel_context *c;
@@
- (c)-id
+ c-user_handle

Also, while we are at it, s/DEFAULT_CONTEXT_ID/DEFAULT_CONTEXT_HANDLE and
change the type to unsigned 32 bits.

v2: s/handle/user_handle and change the type to uint32_t as suggested by
Chris Wilson.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org (v1)
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c|  2 +-
 drivers/gpu/drm/i915/i915_drv.h|  6 +++---
 drivers/gpu/drm/i915/i915_gem_context.c| 12 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c|  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 5e94e58..23f2893 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1869,7 +1869,7 @@ static int per_file_ctx(int id, void *ptr, void *data)
if (i915_gem_context_is_default(ctx))
seq_puts(m,   default context:\n);
else
-   seq_printf(m,   context %d:\n, ctx-id);
+   seq_printf(m,   context %d:\n, ctx-user_handle);
ppgtt-debug_dump(ppgtt, m);
 
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ae32885..1cebefc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -584,10 +584,10 @@ struct i915_ctx_hang_stats {
 };
 
 /* This must match up with the value previously used for execbuf2.rsvd1. */
-#define DEFAULT_CONTEXT_ID 0
+#define DEFAULT_CONTEXT_HANDLE 0
 struct intel_context {
struct kref ref;
-   int id;
+   int user_handle;
uint8_t remap_slice;
struct drm_i915_file_private *file_priv;
struct i915_ctx_hang_stats hang_stats;
@@ -2461,7 +2461,7 @@ static inline void i915_gem_context_unreference(struct 
intel_context *ctx)
 
 static inline bool i915_gem_context_is_default(const struct intel_context *c)
 {
-   return c-id == DEFAULT_CONTEXT_ID;
+   return c-user_handle == DEFAULT_CONTEXT_HANDLE;
 }
 
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index cc67ef4..889b6de 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -276,14 +276,14 @@ __create_hw_context(struct drm_device *dev,
/* Default context will never have a file_priv */
if (file_priv != NULL) {
ret = idr_alloc(file_priv-context_idr, ctx,
-   DEFAULT_CONTEXT_ID, 0, GFP_KERNEL);
+   DEFAULT_CONTEXT_HANDLE, 0, GFP_KERNEL);
if (ret  0)
goto err_out;
} else
-   ret = DEFAULT_CONTEXT_ID;
+   ret = DEFAULT_CONTEXT_HANDLE;
 
ctx-file_priv = file_priv;
-   ctx-id = ret;
+   ctx-user_handle = ret;
/* NB: Mark all slices as needing a remap so that when the context first
 * loads it will restore whatever remap state already exists. If there
 * is no remap info, it will be a NOP. */
@@ -789,7 +789,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, 
void *data,
if (IS_ERR(ctx))
return PTR_ERR(ctx);
 
-   args-ctx_id = ctx-id;
+   args-ctx_id = ctx-user_handle;
DRM_DEBUG_DRIVER(HW context %d created\n, args-ctx_id);
 
return 0;
@@ -803,7 +803,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, 
void *data,
struct intel_context *ctx;
int ret;
 
-   if (args-ctx_id == 

[Intel-gfx] [PATCH 4/8] drm/i915: Add kerneldoc comments to the intel_context struct

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

A bit of background on the context elements.

Cc: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1cebefc..6289d46 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -585,6 +585,21 @@ struct i915_ctx_hang_stats {
 
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_HANDLE 0
+/**
+ * struct intel_context - as the name implies, represents a context.
+ * @ref: reference count.
+ * @user_handle: userspace tracking identity for this context.
+ * @remap_slice: l3 row remapping information.
+ * @file_priv: filp associated with this context (NULL for global default 
context).
+ * @hang_stats: information about the role of this context in possible GPU 
hangs.
+ * @vm: virtual memory space used by this context.
+ * @legacy_hw_ctx: render context backing object and whether it is correctly
+ *initialized (legacy ring submission mechanism only).
+ * @link: link in the global list of contexts.
+ *
+ * Contexts are memory images used by the hardware to store copies of their 
internal
+ * state.
+ */
 struct intel_context {
struct kref ref;
int user_handle;
-- 
1.9.0

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

[Intel-gfx] [PATCH 8/8] drm/i915: Extract the actual workload submission mechanism from execbuffer

[oscar . mateo]

From: Oscar Mateo oscar.ma...@intel.com

So that we isolate the legacy ringbuffer submission mechanism, which becomes
a good candidate to be abstracted away. This is prep-work for Execlists (which
will its own workload submission mechanism).

No functional changes.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 298 -
 1 file changed, 162 insertions(+), 136 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c97178e..60998fc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1026,6 +1026,163 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
return 0;
 }
 
+static int
+legacy_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
+struct intel_engine_cs *ring,
+struct intel_context *ctx,
+struct drm_i915_gem_execbuffer2 *args,
+struct list_head *vmas,
+struct drm_i915_gem_object *batch_obj,
+u64 exec_start, u32 flags)
+{
+   struct drm_clip_rect *cliprects = NULL;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u64 exec_len;
+   int instp_mode;
+   u32 instp_mask;
+   int i, ret = 0;
+
+   if (args-num_cliprects != 0) {
+   if (ring != dev_priv-ring[RCS]) {
+   DRM_DEBUG(clip rectangles are only valid with the 
render ring\n);
+   return -EINVAL;
+   }
+
+   if (INTEL_INFO(dev)-gen = 5) {
+   DRM_DEBUG(clip rectangles are only valid on 
pre-gen5\n);
+   return -EINVAL;
+   }
+
+   if (args-num_cliprects  UINT_MAX / sizeof(*cliprects)) {
+   DRM_DEBUG(execbuf with %u cliprects\n,
+ args-num_cliprects);
+   return -EINVAL;
+   }
+
+   cliprects = kcalloc(args-num_cliprects,
+   sizeof(*cliprects),
+   GFP_KERNEL);
+   if (cliprects == NULL) {
+   ret = -ENOMEM;
+   goto error;
+   }
+
+   if (copy_from_user(cliprects,
+  to_user_ptr(args-cliprects_ptr),
+  sizeof(*cliprects)*args-num_cliprects)) {
+   ret = -EFAULT;
+   goto error;
+   }
+   } else {
+   if (args-DR4 == 0x) {
+   DRM_DEBUG(UXA submitting garbage DR4, fixing up\n);
+   args-DR4 = 0;
+   }
+
+   if (args-DR1 || args-DR4 || args-cliprects_ptr) {
+   DRM_DEBUG(0 cliprects but dirt in cliprects fields\n);
+   return -EINVAL;
+   }
+   }
+
+   ret = i915_gem_execbuffer_move_to_gpu(ring, vmas);
+   if (ret)
+   goto error;
+
+   ret = i915_switch_context(ring, ctx);
+   if (ret)
+   goto error;
+
+   instp_mode = args-flags  I915_EXEC_CONSTANTS_MASK;
+   instp_mask = I915_EXEC_CONSTANTS_MASK;
+   switch (instp_mode) {
+   case I915_EXEC_CONSTANTS_REL_GENERAL:
+   case I915_EXEC_CONSTANTS_ABSOLUTE:
+   case I915_EXEC_CONSTANTS_REL_SURFACE:
+   if (instp_mode != 0  ring != dev_priv-ring[RCS]) {
+   DRM_DEBUG(non-0 rel constants mode on non-RCS\n);
+   ret = -EINVAL;
+   goto error;
+   }
+
+   if (instp_mode != dev_priv-relative_constants_mode) {
+   if (INTEL_INFO(dev)-gen  4) {
+   DRM_DEBUG(no rel constants on pre-gen4\n);
+   ret = -EINVAL;
+   goto error;
+   }
+
+   if (INTEL_INFO(dev)-gen  5 
+   instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
+   DRM_DEBUG(rel surface constants mode invalid 
on gen5+\n);
+   ret = -EINVAL;
+   goto error;
+   }
+
+   /* The HW changed the meaning on this bit on gen6 */
+   if (INTEL_INFO(dev)-gen = 6)
+   instp_mask = ~I915_EXEC_CONSTANTS_REL_SURFACE;
+   }
+   break;
+   default:
+   DRM_DEBUG(execbuf with unknown constants: %d\n, instp_mode);
+   ret = -EINVAL;
+   goto error;
+   }
+
+   if (ring == dev_priv-ring[RCS] 
+

Re: [Intel-gfx] [PATCH 4/8] drm/i915: Add kerneldoc comments to the intel_context struct

[Mateo Lozano, Oscar]

 -Original Message-
 From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
 Of oscar.ma...@intel.com
 Sent: Thursday, July 03, 2014 4:28 PM
 To: intel-gfx@lists.freedesktop.org
 Subject: [Intel-gfx] [PATCH 4/8] drm/i915: Add kerneldoc comments to the
 intel_context struct
 
 From: Oscar Mateo oscar.ma...@intel.com
 
 A bit of background on the context elements.
 
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Oscar Mateo oscar.ma...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h | 15 +++
  1 file changed, 15 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h
 b/drivers/gpu/drm/i915/i915_drv.h index 1cebefc..6289d46 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -585,6 +585,21 @@ struct i915_ctx_hang_stats {
 
  /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_HANDLE 0
 +/**
 + * struct intel_context - as the name implies, represents a context.
 + * @ref: reference count.
 + * @user_handle: userspace tracking identity for this context.
 + * @remap_slice: l3 row remapping information.
 + * @file_priv: filp associated with this context (NULL for global default
 context).
 + * @hang_stats: information about the role of this context in possible GPU
 hangs.
 + * @vm: virtual memory space used by this context.
 + * @legacy_hw_ctx: render context backing object and whether it is
 correctly
 + *initialized (legacy ring submission mechanism only).

Jesse: do you know how to comment this? According to the kerneldoc howto 
Nesting of declarations is not supported and I couldn´t find a similar 
construct with kdoc comments anywhere :(

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

Re: [Intel-gfx] [PATCH] drm/i915: Check hangcheck is functioning before indefinite waits

[Jesse Barnes]

On Thu,  3 Jul 2014 08:09:01 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 Since we rely on hangcheck to wait up and kick us out of an indefinite
 wait should the GPU ever stop functioning, it appears sensible that we
 should check that hangcheck is indeed active before starting that wait.
 This just prevents a driver error in the processing of hangcheck from
 appearing to hang the machine.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_drv.h |  1 +
  drivers/gpu/drm/i915/i915_gem.c |  2 ++
  drivers/gpu/drm/i915/i915_irq.c | 11 +++
  drivers/gpu/drm/i915/intel_ringbuffer.c |  2 ++
  4 files changed, 16 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 8670d05..c326a99 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2149,6 +2149,7 @@ extern void intel_console_resume(struct work_struct 
 *work);
  
  /* i915_irq.c */
  void i915_queue_hangcheck(struct drm_device *dev);
 +void i915_check_hangcheck(struct drm_device *dev);
  __printf(3, 4)
  void i915_handle_error(struct drm_device *dev, bool wedged,
  const char *fmt, ...);
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index ecb835b..120ed6d 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -1455,6 +1455,8 @@ static int __wait_seqno(struct intel_engine_cs *ring, 
 u32 seqno,
   break;
   }
  
 + i915_check_hangcheck(ring-dev);
 +
   timer.function = NULL;
   if (timeout || missed_irq(dev_priv, ring)) {
   unsigned long expire;
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 9e5a295..daa590e 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -3063,6 +3063,17 @@ void i915_queue_hangcheck(struct drm_device *dev)
 round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
  }
  
 +void i915_check_hangcheck(struct drm_device *dev)
 +{
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + if (!i915.enable_hangcheck)
 + return;
 +
 + if (!timer_pending(dev_priv-gpu_error.hangcheck_timer))
 + mod_timer(dev_priv-gpu_error.hangcheck_timer,
 +   round_jiffies_up(jiffies + 
 DRM_I915_HANGCHECK_JIFFIES));
 +}
 +
  static void ibx_irq_reset(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index 4f3397f..6365d4d 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -1634,6 +1634,8 @@ static int ring_wait_for_space(struct intel_engine_cs 
 *ring, int n)
   master_priv-sarea_priv-perf_boxes |= 
 I915_BOX_WAIT;
   }
  
 + i915_check_hangcheck(dev);
 +
   io_schedule_timeout(1);
  
   if (dev_priv-mm.interruptible  signal_pending(current)) {

Are there any bugs associated with this?

i915_rearm_hangcheck() or something might more accurately describe
what's going on here.

I suppose both of these paths are protected by the struct_mutex?  If
not, might we race and mod_timer() this twice from two threads in
succession?  I guess that's harmless...

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

Re: [Intel-gfx] [PATCH] drm/i915: Aggressively downclock Baytrail

[Jesse Barnes]

On Thu,  3 Jul 2014 15:29:26 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
 a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
 downclock if less than 70% busy over 450ms). This causes Baytrail to use
 maximum clocks, and for them to stay high, when doing simple tasks such as
 scrolling through webpages. However, we can take a leaf out of the same
 wait-boost mechansim and apply the aggressive downclocking strategy from
 Sandybridge+ as well.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---

We really need a thorough test suite to cover stuff like this, mapping
frequency, power, and total energy over a big set of workloads to make
sure we're not adding big regressions.

I know you have the cairo traces, but did you also try this with a GL
benchmark suite?

I'm like the change (well you did mix in a cleanup to
set_rps_thresholds), I just want us to get better at collecting numbers
for this stuff...

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

Re: [Intel-gfx] [PATCH] drm/i915: Check hangcheck is functioning before indefinite waits

[Chris Wilson]

On Thu, Jul 03, 2014 at 08:44:20AM -0700, Jesse Barnes wrote:
 On Thu,  3 Jul 2014 08:09:01 +0100
 Chris Wilson ch...@chris-wilson.co.uk wrote:
 
  Since we rely on hangcheck to wait up and kick us out of an indefinite
  wait should the GPU ever stop functioning, it appears sensible that we
  should check that hangcheck is indeed active before starting that wait.
  This just prevents a driver error in the processing of hangcheck from
  appearing to hang the machine.

 Are there any bugs associated with this?

No open bugs. They have cropped up during dev though, and I think I am
not alone. I believe that both Ben and I have tried to convince Daniel
the merits of having this security blanket.
 
 i915_rearm_hangcheck() or something might more accurately describe
 what's going on here.

How about i915_ensure_hangcheck()? (I agree that rearm is better than
check.)
 
 I suppose both of these paths are protected by the struct_mutex?  If
 not, might we race and mod_timer() this twice from two threads in
 succession?  I guess that's harmless...

Concurrently arming a timer within a jiffie or two isn't going to make
too much difference, or even pushing an almost firing timer off by
another hangcheck interval. Conversely, since we already have read the
hangcheck counter, if the hangcheck does fire before we schedule(), that
will immediately wake us up and we will spot the hang.
-Chris

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

Re: [Intel-gfx] [PATCH] drm/i915: Aggressively downclock Baytrail

[Chris Wilson]

On Thu, Jul 03, 2014 at 08:49:22AM -0700, Jesse Barnes wrote:
 On Thu,  3 Jul 2014 15:29:26 +0100
 Chris Wilson ch...@chris-wilson.co.uk wrote:
 
  Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
  a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
  downclock if less than 70% busy over 450ms). This causes Baytrail to use
  maximum clocks, and for them to stay high, when doing simple tasks such as
  scrolling through webpages. However, we can take a leaf out of the same
  wait-boost mechansim and apply the aggressive downclocking strategy from
  Sandybridge+ as well.
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  ---
 
 We really need a thorough test suite to cover stuff like this, mapping
 frequency, power, and total energy over a big set of workloads to make
 sure we're not adding big regressions.
 
 I know you have the cairo traces, but did you also try this with a GL
 benchmark suite?

glxgears went from max clocks to min clocks whilst hitting 60fps.

Note that you first have to disable the cmdparser to make the machine
pleasant to use.

 I'm like the change (well you did mix in a cleanup to
 set_rps_thresholds),

Actually, I left it replicated originally because they used different
strategies at one point and keeping it separate eased experimentation.

The only thing that is missing is a comment to explain that I found I
needed to rewrite the control register every time for the change in
thresholds to take effect.

 I just want us to get better at collecting numbers
 for this stuff...

It's not like we have pretty tools to overlay realtime GPU usage and
bottlenecks...
-Chris

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

Re: [Intel-gfx] [PATCH] drm/i915: Check hangcheck is functioning before indefinite waits

[Jesse Barnes]

On Thu, 3 Jul 2014 16:51:11 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 On Thu, Jul 03, 2014 at 08:44:20AM -0700, Jesse Barnes wrote:
  On Thu,  3 Jul 2014 08:09:01 +0100
  Chris Wilson ch...@chris-wilson.co.uk wrote:
  
   Since we rely on hangcheck to wait up and kick us out of an indefinite
   wait should the GPU ever stop functioning, it appears sensible that we
   should check that hangcheck is indeed active before starting that wait.
   This just prevents a driver error in the processing of hangcheck from
   appearing to hang the machine.
 
  Are there any bugs associated with this?
 
 No open bugs. They have cropped up during dev though, and I think I am
 not alone. I believe that both Ben and I have tried to convince Daniel
 the merits of having this security blanket.
  
  i915_rearm_hangcheck() or something might more accurately describe
  what's going on here.
 
 How about i915_ensure_hangcheck()? (I agree that rearm is better than
 check.)
  
  I suppose both of these paths are protected by the struct_mutex?  If
  not, might we race and mod_timer() this twice from two threads in
  succession?  I guess that's harmless...
 
 Concurrently arming a timer within a jiffie or two isn't going to make
 too much difference, or even pushing an almost firing timer off by
 another hangcheck interval. Conversely, since we already have read the
 hangcheck counter, if the hangcheck does fire before we schedule(), that
 will immediately wake us up and we will spot the hang.

Sounds good.  ensure_hangcheck() or update_hangcheck() are fine with me
too.

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

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

Re: [Intel-gfx] [PATCH] drm/i915: Aggressively downclock Baytrail

[Jesse Barnes]

On Thu, 3 Jul 2014 16:59:17 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 On Thu, Jul 03, 2014 at 08:49:22AM -0700, Jesse Barnes wrote:
  On Thu,  3 Jul 2014 15:29:26 +0100
  Chris Wilson ch...@chris-wilson.co.uk wrote:
  
   Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
   a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
   downclock if less than 70% busy over 450ms). This causes Baytrail to use
   maximum clocks, and for them to stay high, when doing simple tasks such as
   scrolling through webpages. However, we can take a leaf out of the same
   wait-boost mechansim and apply the aggressive downclocking strategy from
   Sandybridge+ as well.
   
   Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
   ---
  
  We really need a thorough test suite to cover stuff like this, mapping
  frequency, power, and total energy over a big set of workloads to make
  sure we're not adding big regressions.
  
  I know you have the cairo traces, but did you also try this with a GL
  benchmark suite?
 
 glxgears went from max clocks to min clocks whilst hitting 60fps.
 
 Note that you first have to disable the cmdparser to make the machine
 pleasant to use.
 
  I'm like the change (well you did mix in a cleanup to
  set_rps_thresholds),
 
 Actually, I left it replicated originally because they used different
 strategies at one point and keeping it separate eased experimentation.
 
 The only thing that is missing is a comment to explain that I found I
 needed to rewrite the control register every time for the change in
 thresholds to take effect.
 
  I just want us to get better at collecting numbers
  for this stuff...
 
 It's not like we have pretty tools to overlay realtime GPU usage and
 bottlenecks...

Yeah I know we have nice tools, I'm just thinking of data over time so
we can see regressions easily etc.  The power stuff QA collects ought
to catch it, but I don't think we've done specific runs with turbo
changes recently; would be good to get Wendy to do that for this patch
and see what happens.

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

Re: [Intel-gfx] pin OABUFFER to GGTT

[Ben Widawsky]

On Thu, Jul 03, 2014 at 08:17:32AM +0100, Damien Lespiau wrote:
 On Wed, Jul 02, 2014 at 02:19:42PM +0100, Rutkowski, Adam J wrote:
  Having said all this, how about restoring the pin_ioctl? At least for
  some time? We do have a use case and moving to other - better -
  solution would take time. I think backward compatibility is something
  that you take into consideration as well.
 
 So, I just sent a patch reverting the change. Daniel will have an
 opinion about this I'm sure, being the original author. Let see what
 happens when he's back from holidays.
 
 Cheers,
 
 -- 
 Damien

Just a note for a future ppgtt people - this adds another way to get
multiple VMAs for a single BO. To this point, it had only been flink,
and dmabuf. IIRC there are few unhandled corner cases for this. Also
note that if the BO is still referenced within a batch, we need the flag
to tell us it needs global binding.

FWIW, I remain in favor of the relocation idea unless someone already
expressed why we need multiple processes to have the relocation info.

-- 
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/i915/vlv: T12 eDP panel timing enforcement during reboot

[Clint Taylor]

On 07/02/2014 01:35 AM, Jani Nikula wrote:

From: Clint Taylor clinton.a.tay...@intel.com

The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.

Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg

Ver3: moved SYS_RESTART check earlier, new name for pp_div.

Ver4: Minor issue changes

Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com

---

I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.


Quick review complete. Everything appears OK.




Thanks,
Jani.
---
  drivers/gpu/drm/i915/intel_dp.c  | 40 
  drivers/gpu/drm/i915/intel_drv.h |  2 ++
  2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
  #include linux/i2c.h
  #include linux/slab.h
  #include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
  #include drm/drmP.h
  #include drm/drm_crtc.h
  #include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
  }

+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+   struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+   struct drm_device *dev = intel_dp_to_dev(intel_dp);
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 pp_div;
+   u32 pp_ctrl_reg, pp_div_reg;
+   enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+   if (!is_edp(intel_dp) || code != SYS_RESTART)
+   return 0;
+
+   if (IS_VALLEYVIEW(dev)) {
+   pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+   pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
+   pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+   pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+   /* 0x1F write to PP_DIV_REG sets max cycle delay */
+   I915_WRITE(pp_div_reg, pp_div | 0x1F);
+   I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+   msleep(intel_dp-panel_power_cycle_delay);
+   }
+
+   return 0;
+}
+
  static bool edp_have_panel_power(struct intel_dp *intel_dp)
  {
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder 
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+   if (intel_dp-edp_notifier.notifier_call) {
+   unregister_reboot_notifier(intel_dp-edp_notifier);
+   intel_dp-edp_notifier.notifier_call = NULL;
+   }
}
kfree(intel_dig_port);
  }
@@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+   if (IS_VALLEYVIEW(dev)) {
+   intel_dp-edp_notifier.notifier_call = 
edp_notify_handler;
+   register_reboot_notifier(intel_dp-edp_notifier);
+   }
}

intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd94d90..87d1715db21d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+   struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;




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

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

[Konrad Rzeszutek Wilk]

On Thu, Jul 03, 2014 at 10:32:12AM +0300, Michael S. Tsirkin wrote:
 On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote:
  On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote:
   Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto:
   With this long thread I lost a bit context about the challenges
   that exists. But let me try summarizing it here - which will hopefully
   get some consensus.
   
   1). Fix IGD hardware to not use Southbridge magic addresses.
   We can moan and moan but I doubt it is going to change.
   
   There are two problems:
   
   - Northbridge (i.e. MCH i.e. PCI host bridge) configuration space 
   addresses
  
  Right. So in  drivers/gpu/drm/i915/i915_dma.c:
  1135 #define MCHBAR_I915 0x44   
   
  1136 #define MCHBAR_I965 0x48 
  
  1147 int reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
  MCHBAR_I915;
  1152 if (INTEL_INFO(dev)-gen = 4) 
   
  1153 pci_read_config_dword(dev_priv-bridge_dev, reg + 4, 
  temp_hi); 
  1154 pci_read_config_dword(dev_priv-bridge_dev, reg, temp_lo);
   
  1155 mchbar_addr = ((u64)temp_hi  32) | temp_lo;
  
  and
  
  1139 #define DEVEN_REG 0x54 
   
  
  1193 int mchbar_reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
  MCHBAR_I915; 
  1202 if (IS_I915G(dev) || IS_I915GM(dev)) { 
   
  1203 pci_read_config_dword(dev_priv-bridge_dev, DEVEN_REG, 
  temp);  
  1204 enabled = !!(temp  DEVEN_MCHBAR_EN);  
   
  1205 } else {   
   
  1206 pci_read_config_dword(dev_priv-bridge_dev, 
  mchbar_reg, temp); 
  1207 enabled = temp  1;
   
  1208 }
   
   - Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some versions 
   of
   the driver identify it by class, some versions identify it by slot (1f.0).
  
  Right, So in  drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch
  which sets the pch_type based on :
  
   432 if (pch-vendor == PCI_VENDOR_ID_INTEL) {  
   
   433 unsigned short id = pch-device  
  INTEL_PCH_DEVICE_ID_MASK;
   434 dev_priv-pch_id = id; 
   
   435
   
   436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) { 
  
  It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00.
  The INTEL_PCH_DEVICE_ID_MASK is 0xff00
   
   To solve the first, make a new machine type, PIIX4-based, and pass through
   the registers you need.  The patch must document _exactly_ why the 
   registers
   are safe to pass.  If they are not reserved on PIIX4, the patch must
   document what the same offsets mean on PIIX4, and why it's sensible to
   assume that firmware for virtual machine will not read/write them.  Bonus
   point for also documenting the same for Q35.
  
  OK. They look to be related to setting up an MBAR , but I don't understand
  why it is needed. Hopefully some of the i915 folks CC-ed here can answer.
 
 In particular, I think setting up MBAR should move out of i915 and become
 the bridge driver tweak on linux and windows.

That is an excellent idea.

However I am still curious to _why_ it has to be done in the first place.

 It would then run on the priveledged host
 and we won't need to deal with it in QEMU.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jesse Barnes]

On Thu, 3 Jul 2014 14:26:12 -0400
Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote:

 On Thu, Jul 03, 2014 at 10:32:12AM +0300, Michael S. Tsirkin wrote:
  On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote:
   On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote:
Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto:
With this long thread I lost a bit context about the challenges
that exists. But let me try summarizing it here - which will hopefully
get some consensus.

1). Fix IGD hardware to not use Southbridge magic addresses.
We can moan and moan but I doubt it is going to change.

There are two problems:

- Northbridge (i.e. MCH i.e. PCI host bridge) configuration space 
addresses
   
   Right. So in  drivers/gpu/drm/i915/i915_dma.c:
   1135 #define MCHBAR_I915 0x44 
  
   1136 #define MCHBAR_I965 0x48 
   
   1147 int reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
   MCHBAR_I915;
   1152 if (INTEL_INFO(dev)-gen = 4)   
  
   1153 pci_read_config_dword(dev_priv-bridge_dev, reg + 4, 
   temp_hi); 
   1154 pci_read_config_dword(dev_priv-bridge_dev, reg, temp_lo);  
  
   1155 mchbar_addr = ((u64)temp_hi  32) | temp_lo;
   
   and
   
   1139 #define DEVEN_REG 0x54   
  
   
   1193 int mchbar_reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
   MCHBAR_I915; 
   1202 if (IS_I915G(dev) || IS_I915GM(dev)) {   
  
   1203 pci_read_config_dword(dev_priv-bridge_dev, 
   DEVEN_REG, temp);  
   1204 enabled = !!(temp  DEVEN_MCHBAR_EN);
  
   1205 } else { 
  
   1206 pci_read_config_dword(dev_priv-bridge_dev, 
   mchbar_reg, temp); 
   1207 enabled = temp  1;  
  
   1208 }

- Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some 
versions of
the driver identify it by class, some versions identify it by slot 
(1f.0).
   
   Right, So in  drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch
   which sets the pch_type based on :
   
432 if (pch-vendor == PCI_VENDOR_ID_INTEL) {
  
433 unsigned short id = pch-device  
   INTEL_PCH_DEVICE_ID_MASK;
434 dev_priv-pch_id = id;   
  
435  
  
436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) { 
   
   It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00.
   The INTEL_PCH_DEVICE_ID_MASK is 0xff00

To solve the first, make a new machine type, PIIX4-based, and pass 
through
the registers you need.  The patch must document _exactly_ why the 
registers
are safe to pass.  If they are not reserved on PIIX4, the patch must
document what the same offsets mean on PIIX4, and why it's sensible to
assume that firmware for virtual machine will not read/write them.  
Bonus
point for also documenting the same for Q35.
   
   OK. They look to be related to setting up an MBAR , but I don't understand
   why it is needed. Hopefully some of the i915 folks CC-ed here can answer.
  
  In particular, I think setting up MBAR should move out of i915 and become
  the bridge driver tweak on linux and windows.
 
 That is an excellent idea.
 
 However I am still curious to _why_ it has to be done in the first place.

The display part of the GPU is partly on the PCH, and it's possible to
mix  match them a bit, so we have this detection code to figure things
out.  In some cases, the PCH display portion may not even be present,
so we look for that too.

Practically speaking, we could probably assume specific CPU/PCH combos,
as I don't think they're generally mixed across generations, though SNB
and IVB did have compatible sockets, so there is the possibility of
mixing CPT and PPT PCHs, but those are register identical as far as the
graphics driver is concerned, so even that should be safe.

Beyond that, the other MCH data we need to look at is mirrored into the
GPU's MMIO space on current gens.  On older gens, we do need to poke at
the memory controller a bit to get some info (see
intel_setup_mchbar()), but that's not true of newer stuff.  Looks like
we only short circuit that on VLV though; we could probably do it on
SNB+.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list

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

[Michael S. Tsirkin]

On Thu, Jul 03, 2014 at 12:09:28PM -0700, Jesse Barnes wrote:
 On Thu, 3 Jul 2014 14:26:12 -0400
 Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote:
 
  On Thu, Jul 03, 2014 at 10:32:12AM +0300, Michael S. Tsirkin wrote:
   On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote:
On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote:
 Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto:
 With this long thread I lost a bit context about the challenges
 that exists. But let me try summarizing it here - which will 
 hopefully
 get some consensus.
 
 1). Fix IGD hardware to not use Southbridge magic addresses.
 We can moan and moan but I doubt it is going to change.
 
 There are two problems:
 
 - Northbridge (i.e. MCH i.e. PCI host bridge) configuration space 
 addresses

Right. So in  drivers/gpu/drm/i915/i915_dma.c:
1135 #define MCHBAR_I915 0x44   
 
1136 #define MCHBAR_I965 0x48 

1147 int reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
MCHBAR_I915;
1152 if (INTEL_INFO(dev)-gen = 4) 
 
1153 pci_read_config_dword(dev_priv-bridge_dev, reg + 
4, temp_hi); 
1154 pci_read_config_dword(dev_priv-bridge_dev, reg, 
temp_lo); 
1155 mchbar_addr = ((u64)temp_hi  32) | temp_lo;  
  

and

1139 #define DEVEN_REG 0x54 
 

1193 int mchbar_reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : 
MCHBAR_I915; 
1202 if (IS_I915G(dev) || IS_I915GM(dev)) { 
 
1203 pci_read_config_dword(dev_priv-bridge_dev, 
DEVEN_REG, temp);  
1204 enabled = !!(temp  DEVEN_MCHBAR_EN);  
 
1205 } else {   
 
1206 pci_read_config_dword(dev_priv-bridge_dev, 
mchbar_reg, temp); 
1207 enabled = temp  1;
 
1208 }
 
 - Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some 
 versions of
 the driver identify it by class, some versions identify it by slot 
 (1f.0).

Right, So in  drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch
which sets the pch_type based on :

 432 if (pch-vendor == PCI_VENDOR_ID_INTEL) {  
 
 433 unsigned short id = pch-device  
INTEL_PCH_DEVICE_ID_MASK;
 434 dev_priv-pch_id = id; 
 
 435
 
 436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) { 

It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00.
The INTEL_PCH_DEVICE_ID_MASK is 0xff00
 
 To solve the first, make a new machine type, PIIX4-based, and pass 
 through
 the registers you need.  The patch must document _exactly_ why the 
 registers
 are safe to pass.  If they are not reserved on PIIX4, the patch must
 document what the same offsets mean on PIIX4, and why it's sensible to
 assume that firmware for virtual machine will not read/write them.  
 Bonus
 point for also documenting the same for Q35.

OK. They look to be related to setting up an MBAR , but I don't 
understand
why it is needed. Hopefully some of the i915 folks CC-ed here can 
answer.
   
   In particular, I think setting up MBAR should move out of i915 and become
   the bridge driver tweak on linux and windows.
  
  That is an excellent idea.
  
  However I am still curious to _why_ it has to be done in the first place.
 
 The display part of the GPU is partly on the PCH, and it's possible to
 mix  match them a bit, so we have this detection code to figure things
 out.  In some cases, the PCH display portion may not even be present,
 so we look for that too.
 
 Practically speaking, we could probably assume specific CPU/PCH combos,
 as I don't think they're generally mixed across generations, though SNB
 and IVB did have compatible sockets, so there is the possibility of
 mixing CPT and PPT PCHs, but those are register identical as far as the
 graphics driver is concerned, so even that should be safe.
 
 Beyond that, the other MCH data we need to look at is mirrored into the
 GPU's MMIO space on current gens.  On older gens, we do need to poke at
 the memory controller a bit to get some info (see
 intel_setup_mchbar()), but that's not true of newer stuff.  Looks like
 we only short 

Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot

[Jani Nikula]

On Thu, 03 Jul 2014, Clint Taylor clinton.a.tay...@intel.com wrote:
 On 07/02/2014 01:35 AM, Jani Nikula wrote:
 From: Clint Taylor clinton.a.tay...@intel.com

 The panel power sequencer on vlv doesn't appear to accept changes to its
 T12 power down duration during warm reboots. This change forces a delay
 for warm reboots to the T12 panel timing as defined in the VBT table for
 the connected panel.

 Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg

 Ver3: moved SYS_RESTART check earlier, new name for pp_div.

 Ver4: Minor issue changes

 Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
 [Jani: rebased on current -nightly.]
 Signed-off-by: Jani Nikula jani.nik...@intel.com

 ---

 I ended up doing the rebase myself, but I'd like to have a quick review
 before pushing.

 Quick review complete. Everything appears OK.

See Paulo's review; want to take over?

Jani.





 Thanks,
 Jani.
 ---
   drivers/gpu/drm/i915/intel_dp.c  | 40 
 
   drivers/gpu/drm/i915/intel_drv.h |  2 ++
   2 files changed, 42 insertions(+)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c 
 b/drivers/gpu/drm/i915/intel_dp.c
 index b5ec48913b47..f0d23c435cf6 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -28,6 +28,8 @@
   #include linux/i2c.h
   #include linux/slab.h
   #include linux/export.h
 +#include linux/notifier.h
 +#include linux/reboot.h
   #include drm/drmP.h
   #include drm/drm_crtc.h
   #include drm/drm_crtc_helper.h
 @@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
  return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
   }

 +/* Reboot notifier handler to shutdown panel power to guarantee T12 timing 
 */
 +static int edp_notify_handler(struct notifier_block *this, unsigned long 
 code,
 +  void *unused)
 +{
 +struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
 + edp_notifier);
 +struct drm_device *dev = intel_dp_to_dev(intel_dp);
 +struct drm_i915_private *dev_priv = dev-dev_private;
 +u32 pp_div;
 +u32 pp_ctrl_reg, pp_div_reg;
 +enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
 +
 +if (!is_edp(intel_dp) || code != SYS_RESTART)
 +return 0;
 +
 +if (IS_VALLEYVIEW(dev)) {
 +pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
 +pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
 +pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
 +pp_div = PP_REFERENCE_DIVIDER_MASK;
 +
 +/* 0x1F write to PP_DIV_REG sets max cycle delay */
 +I915_WRITE(pp_div_reg, pp_div | 0x1F);
 +I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
 +msleep(intel_dp-panel_power_cycle_delay);
 +}
 +
 +return 0;
 +}
 +
   static bool edp_have_panel_power(struct intel_dp *intel_dp)
   {
  struct drm_device *dev = intel_dp_to_dev(intel_dp);
 @@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder 
 *encoder)
  drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
  edp_panel_vdd_off_sync(intel_dp);
  drm_modeset_unlock(dev-mode_config.connection_mutex);
 +if (intel_dp-edp_notifier.notifier_call) {
 +unregister_reboot_notifier(intel_dp-edp_notifier);
 +intel_dp-edp_notifier.notifier_call = NULL;
 +}
  }
  kfree(intel_dig_port);
   }
 @@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port 
 *intel_dig_port,
  if (is_edp(intel_dp)) {
  intel_dp_init_panel_power_timestamps(intel_dp);
  intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
 +if (IS_VALLEYVIEW(dev)) {
 +intel_dp-edp_notifier.notifier_call = 
 edp_notify_handler;
 +register_reboot_notifier(intel_dp-edp_notifier);
 +}
  }

  intel_dp_aux_init(intel_dp, intel_connector);
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 5f7c7bd94d90..87d1715db21d 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -541,6 +541,8 @@ struct intel_dp {
  unsigned long last_power_cycle;
  unsigned long last_power_on;
  unsigned long last_backlight_off;
 +struct notifier_block edp_notifier;
 +
  bool use_tps3;
  struct intel_connector *attached_connector;




-- 
Jani Nikula, 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] pin OABUFFER to GGTT

[Ben Widawsky]

On Thu, Jul 03, 2014 at 10:10:48AM -0700, Ben Widawsky wrote:
 On Thu, Jul 03, 2014 at 08:17:32AM +0100, Damien Lespiau wrote:
  On Wed, Jul 02, 2014 at 02:19:42PM +0100, Rutkowski, Adam J wrote:
   Having said all this, how about restoring the pin_ioctl? At least for
   some time? We do have a use case and moving to other - better -
   solution would take time. I think backward compatibility is something
   that you take into consideration as well.
  
  So, I just sent a patch reverting the change. Daniel will have an
  opinion about this I'm sure, being the original author. Let see what
  happens when he's back from holidays.
  
  Cheers,
  
  -- 
  Damien
 
 Just a note for a future ppgtt people - this adds another way to get
 multiple VMAs for a single BO. To this point, it had only been flink,
 and dmabuf. IIRC there are few unhandled corner cases for this. Also
 note that if the BO is still referenced within a batch, we need the flag
 to tell us it needs global binding.
 
 FWIW, I remain in favor of the relocation idea unless someone already
 expressed why we need multiple processes to have the relocation info.
 

I just realized there isn't really a good way to make the buffer persist
at the same offset if we use the relocation method. So I take back the
statement that it's a good idea.

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

[Intel-gfx] [PATCH 00/10] drm-intel-collector - update

[Rodrigo Vivi]

This is another drm-intel-collector updated notice:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

It was 4 rounds out of date what made it hard to get old patches. However 
Daniel and Jani didn't leave
many patches behind.
0 on Apr 4 - Apr 16
1 on Apr 16 - May 6
2 on May 6 - May 23
3 on May 23 - Jun 6

Next round Jun 6 to Jun 20 is only after next drm-intel-testing update.

Here goes the update list in order for better reviewers assignment:

Patch drm/i915: Bring UP Power Wells before disabling RC6. - Reviewer: 
Paulo Zanoni paulo.r.zan...@intel.com - Reviewer:
Patch drm/i915: Don't save/restore RS when not used - Reviewer:
Patch drm/i915: Upgrade execbuffer fail after resume failure to EIO - 
Reviewer:
Patch drm/i915: Add property to set HDMI aspect ratio - Reviewer: Ville 
Syrjälä ville.syrj...@linux.intel.com - Reviewer:
Patch drm/i915/vlv: WA for Turbo and RC6 to work together. - Reviewer:
Patch drm/i915: honour forced connector modes - Reviewer:
Patch drm/i915: HWS must be in the mappable region for g33 - Reviewer:
Patch drm/i915: Don't promote UC to WT automagically - Reviewer:
Patch drm/i915/bdw: Always issue a force restore - Reviewer:
Patch drm/i915/vlv: T12 eDP panel timing enforcement during reboot. - 
Reviewer:


There are some reasons that some patches can be left behind:
1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
2. Kernel didn't compiled with your patch.
3. I simply missed it. If you believe this is the case please warn me.
4. Remind that any reply to your email automatically take your patch to next 
round.

Please help me to get these patches reviewed and queued by Daniel.

Thanks,
Rodrigo.


Ben Widawsky (2):
  drm/i915: Don't save/restore RS when not used
  drm/i915/bdw: Always issue a force restore

Chris Wilson (3):
  drm/i915: Upgrade execbuffer fail after resume failure to EIO
  drm/i915: honour forced connector modes
  drm/i915: HWS must be in the mappable region for g33

Clint Taylor (1):
  drm/i915/vlv: T12 eDP panel timing enforcement during reboot.

Deepak S (2):
  drm/i915: Bring UP Power Wells before disabling RC6.
  drm/i915/vlv: WA for Turbo and RC6 to work together.

Vandana Kannan (1):
  drm/i915: Add property to set HDMI aspect ratio

Ville Syrjälä (1):
  drm/i915: Don't promote UC to WT automagically

 drivers/gpu/drm/i915/i915_drv.h|  16 
 drivers/gpu/drm/i915/i915_gem.c|   9 +-
 drivers/gpu/drm/i915/i915_gem_context.c|  15 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  15 +++-
 drivers/gpu/drm/i915/i915_irq.c| 133 -
 drivers/gpu/drm/i915/i915_reg.h|  11 +++
 drivers/gpu/drm/i915/intel_dp.c|  42 +
 drivers/gpu/drm/i915/intel_drv.h   |   4 +
 drivers/gpu/drm/i915/intel_fbdev.c |  33 +++
 drivers/gpu/drm/i915/intel_hdmi.c  |  12 +++
 drivers/gpu/drm/i915/intel_modes.c |  28 ++
 drivers/gpu/drm/i915/intel_pm.c|  18 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.c|  16 +++-
 13 files changed, 318 insertions(+), 34 deletions(-)

-- 
1.9.0

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

[Intel-gfx] [PATCH 01/10] drm/i915: Bring UP Power Wells before disabling RC6.

[Rodrigo Vivi]

From: Deepak S deepa...@intel.com

We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.

v2: updated commit message. (Daniel)

Reviewer: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Deepak S deepa...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d771e82..1e4611a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3354,8 +3354,14 @@ static void valleyview_disable_rps(struct drm_device 
*dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
+   /* we're doing forcewake before Disabling RC6,
+* This what the BIOS expects when going into suspend */
+   gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
I915_WRITE(GEN6_RC_CONTROL, 0);
 
+   gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
gen6_disable_rps_interrupts(dev);
 }
 
-- 
1.9.0

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

[Intel-gfx] [PATCH 02/10] drm/i915: Don't save/restore RS when not used

[Rodrigo Vivi]

From: Ben Widawsky benjamin.widaw...@intel.com

Cc: 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/i915_gem_context.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 21eda88..633e318 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -545,6 +545,7 @@ mi_set_context(struct intel_engine_cs *ring,
   struct intel_context *new_context,
   u32 hw_flags)
 {
+   u32 flags = hw_flags | MI_MM_SPACE_GTT;
int ret;
 
/* w/a: If Flush TLB Invalidation Mode is enabled, driver must do a TLB
@@ -558,6 +559,10 @@ mi_set_context(struct intel_engine_cs *ring,
return ret;
}
 
+   /* These flags are for resource streamer on HSW+ */
+   if (!IS_HASWELL(ring-dev)  INTEL_INFO(ring-dev)-gen  8)
+   flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
+
ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
@@ -570,11 +575,8 @@ mi_set_context(struct intel_engine_cs *ring,
 
intel_ring_emit(ring, MI_NOOP);
intel_ring_emit(ring, MI_SET_CONTEXT);
-   intel_ring_emit(ring, i915_gem_obj_ggtt_offset(new_context-obj) |
-   MI_MM_SPACE_GTT |
-   MI_SAVE_EXT_STATE_EN |
-   MI_RESTORE_EXT_STATE_EN |
-   hw_flags);
+   intel_ring_emit(ring,
+   i915_gem_obj_ggtt_offset(new_context-obj) | flags);
/*
 * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
 * WaMiSetContext_Hang:snb,ivb,vlv
-- 
1.9.0

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

[Intel-gfx] [PATCH 00/10] drm-intel-collector - update

[Rodrigo Vivi]

This is another drm-intel-collector updated notice:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

It was 4 rounds out of date what made it hard to get old patches. However 
Daniel and Jani didn't leave
many patches behind.
0 on Apr 4 - Apr 16
1 on Apr 16 - May 6
2 on May 6 - May 23
3 on May 23 - Jun 6

Next round Jun 6 to Jun 20 is only after next drm-intel-testing update.

Here goes the update list in order for better reviewers assignment:

Patch drm/i915: Bring UP Power Wells before disabling RC6. - Reviewer: 
Paulo Zanoni paulo.r.zan...@intel.com - Reviewer:
Patch drm/i915: Don't save/restore RS when not used - Reviewer:
Patch drm/i915: Upgrade execbuffer fail after resume failure to EIO - 
Reviewer:
Patch drm/i915: Add property to set HDMI aspect ratio - Reviewer: Ville 
Syrjälä ville.syrj...@linux.intel.com - Reviewer:
Patch drm/i915/vlv: WA for Turbo and RC6 to work together. - Reviewer:
Patch drm/i915: honour forced connector modes - Reviewer:
Patch drm/i915: HWS must be in the mappable region for g33 - Reviewer:
Patch drm/i915: Don't promote UC to WT automagically - Reviewer:
Patch drm/i915/bdw: Always issue a force restore - Reviewer:
Patch drm/i915/vlv: T12 eDP panel timing enforcement during reboot. - 
Reviewer:


There are some reasons that some patches can be left behind:
1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
2. Kernel didn't compiled with your patch.
3. I simply missed it. If you believe this is the case please warn me.
4. Remind that any reply to your email automatically take your patch to next 
round.

Please help me to get these patches reviewed and queued by Daniel.

Thanks,
Rodrigo.


Ben Widawsky (2):
  drm/i915: Don't save/restore RS when not used
  drm/i915/bdw: Always issue a force restore

Chris Wilson (3):
  drm/i915: Upgrade execbuffer fail after resume failure to EIO
  drm/i915: honour forced connector modes
  drm/i915: HWS must be in the mappable region for g33

Clint Taylor (1):
  drm/i915/vlv: T12 eDP panel timing enforcement during reboot.

Deepak S (2):
  drm/i915: Bring UP Power Wells before disabling RC6.
  drm/i915/vlv: WA for Turbo and RC6 to work together.

Vandana Kannan (1):
  drm/i915: Add property to set HDMI aspect ratio

Ville Syrjälä (1):
  drm/i915: Don't promote UC to WT automagically

 drivers/gpu/drm/i915/i915_drv.h|  16 
 drivers/gpu/drm/i915/i915_gem.c|   9 +-
 drivers/gpu/drm/i915/i915_gem_context.c|  15 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  15 +++-
 drivers/gpu/drm/i915/i915_irq.c| 133 -
 drivers/gpu/drm/i915/i915_reg.h|  11 +++
 drivers/gpu/drm/i915/intel_dp.c|  42 +
 drivers/gpu/drm/i915/intel_drv.h   |   4 +
 drivers/gpu/drm/i915/intel_fbdev.c |  33 +++
 drivers/gpu/drm/i915/intel_hdmi.c  |  12 +++
 drivers/gpu/drm/i915/intel_modes.c |  28 ++
 drivers/gpu/drm/i915/intel_pm.c|  18 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.c|  16 +++-
 13 files changed, 318 insertions(+), 34 deletions(-)

-- 
1.9.0

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

[Intel-gfx] [PATCH 03/10] drm/i915: Upgrade execbuffer fail after resume failure to EIO

[Rodrigo Vivi]

From: Chris Wilson ch...@chris-wilson.co.uk

If we try to execute on a known ring, but it has failed to be
initialised correctly, report that the GPU is hung rather than the
command invalid. This leaves us reporting EINVAL only if the user
requests execution on a ring that is not supported by the device.

This should prevent UXA from getting stuck in a null render loop after a
failed resume.

v2 (Rodrigo): Fix conflict and add VCS2 ring and
  s/intel_ring_buffer/intel_engine_cs.

Reported-by: Jiri Kosina ji...@jikos.cz
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d815ef5..23786ab 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1076,6 +1076,19 @@ eb_get_batch(struct eb_vmas *eb)
return vma-obj;
 }
 
+static bool
+intel_ring_valid(struct intel_engine_cs *ring)
+{
+   switch (ring-id) {
+   case RCS: return true;
+   case VCS: return HAS_BSD(ring-dev);
+   case BCS: return HAS_BLT(ring-dev);
+   case VECS: return HAS_VEBOX(ring-dev);
+   case VCS2: return HAS_BSD2(ring-dev);
+   default: return false;
+   }
+}
+
 static int
 i915_gem_do_execbuffer(struct drm_device *dev, void *data,
   struct drm_file *file,
@@ -1133,7 +1146,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (!intel_ring_initialized(ring)) {
DRM_DEBUG(execbuf with invalid ring: %d\n,
  (int)(args-flags  I915_EXEC_RING_MASK));
-   return -EINVAL;
+   return intel_ring_valid(ring) ? -EIO : -EINVAL;
}
 
mode = args-flags  I915_EXEC_CONSTANTS_MASK;
-- 
1.9.0

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

[Intel-gfx] [PATCH 04/10] drm/i915: Add property to set HDMI aspect ratio

[Rodrigo Vivi]

From: Vandana Kannan vandana.kan...@intel.com

Added a property to enable user space to set aspect ratio for HDMI displays.
If there is no user specified value, then PAR_NONE/Automatic option is set
by default. User can select aspect ratio 4:3 or 16:9. The aspect ratio
selected by user would come into effect with a mode set.

v2: Daniel's review comments incorporated.
Call for a mode set to update property.

Reviewer: Ville Syrjälä ville.syrj...@linux.intel.com
Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Jesse Barnes jesse.bar...@intel.com
Cc: Vijay Purushothaman vijay.a.purushotha...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|  1 +
 drivers/gpu/drm/i915/intel_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c  | 12 
 drivers/gpu/drm/i915/intel_modes.c | 28 
 4 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cea596..1bf277e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1529,6 +1529,7 @@ struct drm_i915_private {
 
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+   struct drm_property *aspect_ratio_property;
 
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd..7b4d743 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -495,6 +495,7 @@ struct intel_hdmi {
bool has_audio;
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
+   enum hdmi_picture_aspect aspect_ratio;
void (*write_infoframe)(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len);
@@ -923,6 +924,7 @@ int intel_connector_update_modes(struct drm_connector 
*connector,
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_aspect_ratio_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 2422413..1851284 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder 
*encoder,
union hdmi_infoframe frame;
int ret;
 
+   /* Set user selected PAR to incoming mode's member */
+   adjusted_mode-picture_aspect_ratio = intel_hdmi-aspect_ratio;
+
ret = drm_hdmi_avi_infoframe_from_display_mode(frame.avi,
   adjusted_mode);
if (ret  0) {
@@ -1124,6 +1127,11 @@ intel_hdmi_set_property(struct drm_connector *connector,
goto done;
}
 
+   if (property == dev_priv-aspect_ratio_property) {
+   intel_hdmi-aspect_ratio = val;
+   goto done;
+   }
+
return -EINVAL;
 
 done:
@@ -1484,6 +1492,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
 {
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
+   intel_attach_aspect_ratio_property(connector);
intel_hdmi-color_range_auto = true;
 }
 
@@ -1551,6 +1560,9 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
intel_connector-get_hw_state = intel_connector_get_hw_state;
intel_connector-unregister = intel_connector_unregister;
 
+   /* Initialize aspect ratio member of intel_hdmi */
+   intel_hdmi-aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
intel_hdmi_add_properties(intel_hdmi, connector);
 
intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_modes.c 
b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f3..6f814da 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,31 @@ intel_attach_broadcast_rgb_property(struct drm_connector 
*connector)
 
drm_object_attach_property(connector-base, prop, 0);
 }
+
+static const struct drm_prop_enum_list aspect_ratio_names[] = {
+   { HDMI_PICTURE_ASPECT_NONE, Automatic },
+   { HDMI_PICTURE_ASPECT_4_3, 4:3 },
+   { HDMI_PICTURE_ASPECT_16_9, 16:9 },
+};
+
+void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+   struct drm_device *dev = connector-dev;
+   struct drm_i915_private *dev_priv 

[Intel-gfx] [PATCH 10/10] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.

[Rodrigo Vivi]

From: Clint Taylor clinton.a.tay...@intel.com

The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.

Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg

Ver3: moved SYS_RESTART check earlier, new name for pp_div.

Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c  | 42 
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..ece8f28 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
 #include linux/i2c.h
 #include linux/slab.h
 #include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
 #include drm/drmP.h
 #include drm/drm_crtc.h
 #include drm/drm_crtc_helper.h
@@ -336,6 +338,38 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
 }
 
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+   struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 pp_div;
+   u32 pp_ctrl_reg, pp_div_reg;
+   enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+   if ((!is_edp(intel_dp)) 
+   (code != SYS_RESTART ))
+   return 0;
+
+   if (IS_VALLEYVIEW(dev)) {
+   pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+   pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
+   pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+   pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+   /* 0x1F write to PP_DIV_REG sets max cycle delay */
+   I915_WRITE(pp_div_reg , pp_div | 0x1F);
+   I915_WRITE(pp_ctrl_reg,
+  PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+   msleep(intel_dp-panel_power_cycle_delay);
+   }
+   return 0;
+}
+
 static bool edp_have_panel_power(struct intel_dp *intel_dp)
 {
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3819,10 @@ void intel_dp_encoder_destroy(struct drm_encoder 
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+   if (intel_dp-edp_notifier.notifier_call) {
+   unregister_reboot_notifier(intel_dp-edp_notifier);
+   intel_dp-edp_notifier.notifier_call = NULL;
+   }
}
kfree(intel_dig_port);
 }
@@ -4353,6 +4391,10 @@ intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+   if (IS_VALLEYVIEW(dev)) {
+   intel_dp-edp_notifier.notifier_call = 
edp_notify_handler;
+   register_reboot_notifier(intel_dp-edp_notifier);
+   }
}
 
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7b4d743..c52e879 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -542,6 +542,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+   struct notifier_block  edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
 
-- 
1.9.0

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

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

[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 633e318..61b60b6 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -573,6 +573,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,
-- 
1.9.0

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

[Intel-gfx] [PATCH 06/10] drm/i915: honour forced connector modes

[Rodrigo Vivi]

From: Chris Wilson ch...@chris-wilson.co.uk

In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off.  Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config that tries to enable a connector (disabling is easy!).

Based on earlier patches by Jesse Barnes.

v2: Remove Jesse's patch

Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_fbdev.c | 33 -
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
b/drivers/gpu/drm/i915/intel_fbdev.c
index 226fbc7..34c1a3d 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -331,24 +331,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
int num_connectors_enabled = 0;
int num_connectors_detected = 0;
 
-   /*
-* If the user specified any force options, just bail here
-* and use that config.
-*/
-   for (i = 0; i  fb_helper-connector_count; i++) {
-   struct drm_fb_helper_connector *fb_conn;
-   struct drm_connector *connector;
-
-   fb_conn = fb_helper-connector_info[i];
-   connector = fb_conn-connector;
-
-   if (!enabled[i])
-   continue;
-
-   if (connector-force != DRM_FORCE_UNSPECIFIED)
-   return false;
-   }
-
save_enabled = kcalloc(dev-mode_config.num_connector, sizeof(bool),
   GFP_KERNEL);
if (!save_enabled)
@@ -374,8 +356,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
continue;
}
 
+   if (connector-force == DRM_FORCE_OFF) {
+   DRM_DEBUG_KMS(connector %s is disabled by user, 
skipping\n,
+ connector-name);
+   enabled[i] = false;
+   continue;
+   }
+
encoder = connector-encoder;
if (!encoder || WARN_ON(!encoder-crtc)) {
+   if (connector-force  DRM_FORCE_OFF)
+   goto bail;
+
DRM_DEBUG_KMS(connector %s has no encoder or crtc, 
skipping\n,
  connector-name);
enabled[i] = false;
@@ -394,8 +386,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
for (j = 0; j  fb_helper-connector_count; j++) {
if (crtcs[j] == new_crtc) {
DRM_DEBUG_KMS(fallback: cloned 
configuration\n);
-   fallback = true;
-   goto out;
+   goto bail;
}
}
 
@@ -466,8 +457,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
fallback = true;
}
 
-out:
if (fallback) {
+bail:
DRM_DEBUG_KMS(Not using firmware configuration\n);
memcpy(enabled, save_enabled, dev-mode_config.num_connector);
kfree(save_enabled);
-- 
1.9.0

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

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

[Ben Widawsky]

On Thu, Jul 03, 2014 at 05:33:05PM -0400, 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
 ---
  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 633e318..61b60b6 100644
 --- a/drivers/gpu/drm/i915/i915_gem_context.c
 +++ b/drivers/gpu/drm/i915/i915_gem_context.c
 @@ -573,6 +573,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,

Ville had a good point on this patch wrt to note setting both
MI_FORCE_RESTORE, and MI_RESTORE_INHIBIT (though it seems to cause no
problems).

I think also with some of the do_switch() cleanups recently submitted,
this one may no longer be necessary - not sure.

-- 
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 09/10] drm/i915/bdw: Always issue a force restore

[Rodrigo Vivi]

Thanks

Please just ignore this one for now. It will be removed on next round.


On Thu, Jul 3, 2014 at 5:38 PM, Ben Widawsky benjamin.widaw...@intel.com
wrote:

 On Thu, Jul 03, 2014 at 05:33:05PM -0400, 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
  ---
   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 633e318..61b60b6 100644
  --- a/drivers/gpu/drm/i915/i915_gem_context.c
  +++ b/drivers/gpu/drm/i915/i915_gem_context.c
  @@ -573,6 +573,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,

 Ville had a good point on this patch wrt to note setting both
 MI_FORCE_RESTORE, and MI_RESTORE_INHIBIT (though it seems to cause no
 problems).

 I think also with some of the do_switch() cleanups recently submitted,
 this one may no longer be necessary - not sure.

 --
 Ben Widawsky, Intel Open Source Technology Center




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

Re: [Intel-gfx] how to build intel-gpu-tools without cairo

[Liu, Ying2]

Damien,

We run intel-gpu-tool in VMware esx console. We didn't port display part of 
intel gpu driver to esx, so we don't need any display tests at all.
If you could provide us a solution to run intel gpu tools without cairo, that 
would be great.

Thanks

Ying


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

[Intel-gfx] [PATCH 1/2] drm/i915/gen8: Invalidate TLBs before PDP reload

[Ben Widawsky]

This is a spec requirement for all rings.

Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 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 5b4a9a0..1ac648f 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -870,6 +870,9 @@ int i915_switch_context(struct intel_engine_cs *ring,
if (from == to  !to-remap_slice)
return 0;
 
+   if (IS_GEN8(ring-dev))
+   WARN_ON(ring-flush(ring, I915_GEM_GPU_DOMAINS, 0));
+
if (ring-id == RCS)
return do_switch_rcs(ring, from, to);
else
-- 
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: Remove false assertion in ppgtt_release

[Ben Widawsky]

Originally the thought for the assertion was that if there are no real
VMAs (died during execbuf), or there is only 1 VMA, but the VMA is on
the active list, it's a bug. The former case is pretty obvious. The
later case simply meant to assert the context unref/object retire
interactions were working properly

There is a flaw in the logic of the second when an object has multiple
VMAs. If there are multiple VMAs, it's possible that the object
continually had it's seqno increased as it was used by another context.
In this case, the context ref will die, but the VMA will not be taking
off the active list because of the missing retire seqno for a VMA.

Like some of the other fixes I've submitted recently, this should be
fixed by the eventual work Daniel will do.

This is pretty easy to reproduce whenever mesa uses the blit engine.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 1ac648f..2b39fca 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -145,8 +145,7 @@ static int do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt)
 
do_idle = true;
list_for_each_entry(vma, vm-active_list, mm_list)
-   if (WARN_ON(list_empty(vma-vma_link) ||
-   list_is_singular(vma-vma_link)))
+   if (WARN_ON(list_empty(vma-vma_link)))
break;
} else
i915_gem_retire_requests(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] drm/i915/vlv: T12 eDP panel timing enforcement during reboot

[Clint Taylor]

On 07/02/2014 07:40 AM, Paulo Zanoni wrote:

2014-07-02 5:35 GMT-03:00 Jani Nikula jani.nik...@intel.com:

From: Clint Taylor clinton.a.tay...@intel.com

The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.

Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg

Ver3: moved SYS_RESTART check earlier, new name for pp_div.

Ver4: Minor issue changes

Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com

---

I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.

Thanks,
Jani.
---
  drivers/gpu/drm/i915/intel_dp.c  | 40 
  drivers/gpu/drm/i915/intel_drv.h |  2 ++
  2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
  #include linux/i2c.h
  #include linux/slab.h
  #include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
  #include drm/drmP.h
  #include drm/drm_crtc.h
  #include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
  }

+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+   struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+   struct drm_device *dev = intel_dp_to_dev(intel_dp);
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 pp_div;
+   u32 pp_ctrl_reg, pp_div_reg;
+   enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+   if (!is_edp(intel_dp) || code != SYS_RESTART)


What if someone does a power off and _very quickly_ starts the system again? =P
insert same statement for the other code possibilities

If someone removes and applies power within ~300ms this W/A will break 
down and the power sequence will not meet the eDP T12 timing. Since the 
PPS sequencer does not allow modifications to the default time intervals 
during the initial sequence the only way to guarantee we meet T12 time 
would be to delay display block power ungate for 300ms. Further delay of 
the boot time was not an acceptable solution for the customers.



Also, depending based on the suggestions below, you may not need the
is_edp() check (or you may want to convert it to a WARN).


+   return 0;
+
+   if (IS_VALLEYVIEW(dev)) {


This check is not really needed. It could also be an early return or a WARN.


Since we currently don't handle PCH offsets this was a safe way to 
allowing adding of the PCH functionality later if necessary.




+   pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+   pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
+   pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));


Or pp_div = I915_READ(pp_div_reg);, since we just defined it :)


Agreed that's another way to do the same thing.





+   pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+   /* 0x1F write to PP_DIV_REG sets max cycle delay */
+   I915_WRITE(pp_div_reg, pp_div | 0x1F);
+   I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);


So this is basically turning the panel off and turning the force VDD
bit off too, and we're not putting any power domain references back.
Even though this might not be a big problem since we're shutting down
the machine anyway, did we attach a serial cable and check if this
won't print any WARNs while shutting down? Shouldn't we try to make
this function call the other functions that shut down stuff instead of
forcing the panel down here?


Development of this W/A was done with serial port attached. This 
function is the last method called in the I915 driver before power is 
removed. At reboot notifier time there is no error handling possible 
calling the normal shutdown functions does more housekeeping then we 
need for a system that will not have power in  2 ms.






+   msleep(intel_dp-panel_power_cycle_delay);
+   }
+
+   return 0;
+}
+
  static bool edp_have_panel_power(struct intel_dp *intel_dp)
  {
 struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder 
*encoder)
 drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
 edp_panel_vdd_off_sync(intel_dp);
 drm_modeset_unlock(dev-mode_config.connection_mutex);

[Intel-gfx] [PATCH v3 0/3] drm/i915: fix backlight regression caused by misconfigured VBT

[Scot Doyle]

Submitted with git to correct whitespace problems.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 1/3] drm/i915: quirk asserts controllable backlight presence, overriding VBT

[Scot Doyle]

commit c675949ec58ca50d5a3ae3c757892f1560f6e896
  drm/i915: do not setup backlight if not available according to VBT

caused a regression on machines with a misconfigured VBT. Add a quirk to
assert the presence of a controllable backlight. Use it to ignore the VBT
backlight presence check during backlight setup.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79813
Tested-by: James Duley jagdu...@gmail.com
Tested-by: Michael Mullin masmul...@gmail.com
Reviewed-by: Jani Nikula jani.nik...@intel.com
Signed-off-by: Scot Doyle lkm...@scotdoyle.com
---
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_display.c |8 
 drivers/gpu/drm/i915/intel_panel.c   |8 ++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9953ea8..ac06c0f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -658,6 +658,7 @@ enum intel_sbi_destination {
 #define QUIRK_PIPEA_FORCE (10)
 #define QUIRK_LVDS_SSC_DISABLE (11)
 #define QUIRK_INVERT_BRIGHTNESS (12)
+#define QUIRK_BACKLIGHT_PRESENT (13)
 
 struct intel_fbdev;
 struct intel_fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a72b55f..fae289f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12315,6 +12315,14 @@ static void quirk_invert_brightness(struct drm_device 
*dev)
DRM_INFO(applying inverted panel brightness quirk\n);
 }
 
+/* Some VBT's incorrectly indicate no backlight is present */
+static void quirk_backlight_present(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   dev_priv-quirks |= QUIRK_BACKLIGHT_PRESENT;
+   DRM_INFO(applying backlight present quirk\n);
+}
+
 struct intel_quirk {
int device;
int subsystem_vendor;
diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 38a9857..628cd89 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1118,8 +1118,12 @@ int intel_panel_setup_backlight(struct drm_connector 
*connector)
int ret;
 
if (!dev_priv-vbt.backlight.present) {
-   DRM_DEBUG_KMS(native backlight control not available per 
VBT\n);
-   return 0;
+   if (dev_priv-quirks  QUIRK_BACKLIGHT_PRESENT) {
+   DRM_DEBUG_KMS(no backlight present per VBT, but 
present per quirk\n);
+   } else {
+   DRM_DEBUG_KMS(no backlight present per VBT\n);
+   return 0;
+   }
}
 
/* set level and max in panel struct */
-- 
1.7.10.4

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

[Intel-gfx] [PATCH v3 3/3] drm/i915: Toshiba CB35 has a controllable backlight

[Scot Doyle]

The Toshiba CB35 Chromebook (with Celeron 2955U CPU) has a controllable
backlight although its VBT reports otherwise. Apply quirk to ignore the
backlight presence check during backlight setup.

Patch tested by author on Toshiba CB35.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79813
Signed-off-by: Scot Doyle lkm...@scotdoyle.com
CC: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7345441..c12a5da 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12394,6 +12394,9 @@ static struct intel_quirk intel_quirks[] = {
 
/* Acer C720 and C720P Chromebooks (Celeron 2955U) have backlights */
{ 0x0a06, 0x1025, 0x0a11, quirk_backlight_present },
+
+   /* Toshiba CB35 Chromebook (Celeron 2955U) */
+   { 0x0a06, 0x1179, 0x0a88, quirk_backlight_present },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
-- 
1.7.10.4

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

[Intel-gfx] [PATCH v3 2/3] drm/i915: Acer C720 and C720P have controllable backlights

[Scot Doyle]

The Acer C720 and C720P Chromebooks (with Celeron 2955U CPU) have a
controllable backlight although their VBT reports otherwise. Apply quirk
to ignore the backlight presence check during backlight setup.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79813
Tested-by: James Duley jagdu...@gmail.com
Tested-by: Michael Mullin masmul...@gmail.com
Signed-off-by: Scot Doyle lkm...@scotdoyle.com
CC: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fae289f..7345441 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12391,6 +12391,9 @@ static struct intel_quirk intel_quirks[] = {
 
/* Acer Aspire 5336 */
{ 0x2a42, 0x1025, 0x048a, quirk_invert_brightness },
+
+   /* Acer C720 and C720P Chromebooks (Celeron 2955U) have backlights */
+   { 0x0a06, 0x1025, 0x0a11, quirk_backlight_present },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
-- 
1.7.10.4

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

[Intel-gfx] [PATCH v3 1/2] drm/i915: provide interface for audio driver to query cdclk

[mengdong . lin]

From: Jani Nikula jani.nik...@intel.com

For Haswell and Broadwell, if the display power well has been disabled,
the display audio controller divider values EM4 M VALUE and EM5 N VALUE
will have been lost. The CDCLK frequency is required for reprogramming them
to generate 24MHz HD-A link BCLK. So provide a private interface for the
audio driver to query CDCLK.

This is a stopgap solution until a more generic interface between audio
and display drivers has been implemented.

Signed-off-by: Jani Nikula jani.nik...@intel.com
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Mengdong Lin mengdong@intel.com

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a90fdbd..21170e5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6256,6 +6256,27 @@ int i915_release_power_well(void)
 }
 EXPORT_SYMBOL_GPL(i915_release_power_well);
 
+/*
+ * Private interface for the audio driver to get CDCLK in kHz.
+ *
+ * Caller must request power well using i915_request_power_well() prior to
+ * making the call.
+ */
+int i915_get_cdclk_freq(void)
+{
+   struct drm_i915_private *dev_priv;
+
+   if (!hsw_pwr)
+   return -ENODEV;
+
+   dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+   power_domains);
+
+   return intel_ddi_get_cdclk_freq(dev_priv);
+}
+EXPORT_SYMBOL_GPL(i915_get_cdclk_freq);
+
+
 #define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
 
 #define HSW_ALWAYS_ON_POWER_DOMAINS (  \
diff --git a/include/drm/i915_powerwell.h b/include/drm/i915_powerwell.h
index 2baba99..baa6f11 100644
--- a/include/drm/i915_powerwell.h
+++ b/include/drm/i915_powerwell.h
@@ -32,5 +32,6 @@
 /* For use by hda_i915 driver */
 extern int i915_request_power_well(void);
 extern int i915_release_power_well(void);
+extern int i915_get_cdclk_freq(void);
 
 #endif /* _I915_POWERWELL_H_ */
-- 
1.8.1.2

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