Re: [Intel-gfx] drm/i915 stable backport request
On Tue, Sep 17, 2013 at 01:13:26AM +0200, Daniel Vetter wrote: Dear stable team, Please packport the following upstream commit to 3.10: commit 2960bc9cceecb5d556ce1c07656a66 09e2f7e8b0 Author: Imre Deak imre.d...@intel.com Date: Tue Jul 30 13:36:32 2013 +0300 drm/i915: make user mode sync polarity setting explicit Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69400 Odd, it doesn't apply to 3.10-stable, but it does apply to 3.11-stable, is that what you meant? If 3.10, can you send a backported version that I can apply? thanks, greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] DPF (GPU l3 parity detection) improvements
I see. I had thought the hang bit was part of the test injection, when it's actually modifying the behavior or L3 errors. Any opinions on what the default should be (agreed that policy should be controlled by user space, but we can control the default)? What does a hang mean exactly, is the rest of memory still responsive, L3? On Mon, Sep 16, 2013 at 5:52 PM, Bell, Bryan J bryan.j.b...@intel.com wrote: The hang injection is for the scenarios like: (1) L3 error occurs (2) Workload completion, reported to user mode driver, e.g. OpenCL (3) L3 error interrupt, handled. If (2) occurs before (3), it's possible to report that a GPGPU workload successfully completed when in fact it did not due to the L3 error. It should be up to the user mode if the hang bit is set. --Thanks Bryan -Original Message- From: Ben Widawsky [mailto:benjamin.widaw...@intel.com] Sent: Thursday, September 12, 2013 10:28 PM To: intel-gfx@lists.freedesktop.org Cc: Venkatesh, Vishnu; Bell, Bryan J; Widawsky, Benjamin Subject: [PATCH 0/8] DPF (GPU l3 parity detection) improvements Since IVB, our driver has supported GPU L3 cacheline remapping for parity errors. This is known as, DPF for Dynamic Parity Feature. I am told such an error is a good predictor for a subsequent error in the same part of the cache. To address this possible issue for workloads requiring precise and correct data, like GPGPU workloads the HW has extra space in the cache which can be dynamically remapped to fill in the old, faulting parts of the cache. I should also note, to my knowledge, no such error has actually been seen on either Ivybridge or Haswell in the wild. Note, and reminder: GPU L3 is not the same thing as L3. It is a special (usually incoherent) cache that is only used by certain components within the GPU. Included in the patches: 1. Fix HSW test cases previously submitted and bikeshedded by Ville. 2. Support for an extra area of L3 added in certain HSW SKUs 3. Error injection support from the user space for test. 4. A reference daemon for listening to the parity error events. Caveats: * I've not implemented the hang injection. I was not clear what it does, and I don't really see how it benefits testing the software I have written. * I am currently missing a test which uses the error injection. Volunteers who want to help, please raise your hand. If not, I'll get to it as soon as possible. * We do have a race with the udev mechanism of error delivery. If I understand the way udev works, if we have more than 1 event before the daemon is woken, the properties will get us the failing cache location of the last error only. I think this is okay because of the earlier statement that a parity error is a good indicator of a future parity error. One thing which I've not done is trying to track when there are missed errors which should be possible even if the info about the location of the error can't be retrieved. * There is no way to read out the per context remapping information through sysfs. I only expose whether or not a context has outstanding remaps through debugfs. This does effect the testability a bit, but the implementation is simple enough that I'm not terrible worried. Ben Widawsky (8): drm/i915: Remove extra ring drm/i915: Round l3 parity reads down drm/i915: Fix l3 parity user buffer offset drm/i915: Fix HSW parity test drm/i915: Add second slice l3 remapping drm/i915: Make l3 remapping use the ring drm/i915: Keep a list of all contexts drm/i915: Do remaps for all contexts drivers/gpu/drm/i915/i915_debugfs.c | 23 ++--- drivers/gpu/drm/i915/i915_drv.h | 13 +++-- drivers/gpu/drm/i915/i915_gem.c | 46 +- drivers/gpu/drm/i915/i915_gem_context.c | 20 +++- drivers/gpu/drm/i915/i915_irq.c | 84 + drivers/gpu/drm/i915/i915_reg.h | 6 +++ drivers/gpu/drm/i915/i915_sysfs.c | 57 +++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +-- include/uapi/drm/i915_drm.h | 8 ++-- 9 files changed, 175 insertions(+), 88 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Add bind/unbind object functions to VM
On Mon, Sep 16, 2013 at 11:13:02PM +0100, Chris Wilson wrote:
On Mon, Sep 16, 2013 at 11:23:43AM -0700, Ben Widawsky wrote:
On Mon, Sep 16, 2013 at 10:25:28AM +0100, Chris Wilson wrote:
On Sat, Sep 14, 2013 at 03:03:17PM -0700, Ben Widawsky wrote:
+static void gen6_ggtt_bind_vma(struct i915_vma *vma,
+ enum i915_cache_level cache_level,
+ u32 flags)
+{
+ struct drm_device *dev = vma-vm-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_i915_gem_object *obj = vma-obj;
+ const unsigned long entry = vma-node.start PAGE_SHIFT;
+
+ /* If there is an aliasing PPGTT, and the user didn't
explicitly ask for
+* the global, just use aliasing */
+ if (!dev_priv-mm.aliasing_ppgtt || flags GLOBAL_BIND) {
+ /* If the object is unbound, or we're change the cache
bits */
+ if (!obj-has_global_gtt_mapping ||
+ (cache_level != obj-cache_level)) {
+ gen6_ggtt_insert_entries(vma-vm, obj-pages,
entry,
+cache_level);
+ obj-has_global_gtt_mapping = 1;
+ }
+ }
+
+ /* If put the mapping in the aliasing PPGTT as well as Global
if we have
+* aliasing, but the user requested global. */
Why? As a proponent of full-ppgtt I thought you would be envisoning a
future where the aliasing_ppgtt was used far less (i.e. never), and the
ggtt would only continue to be used for the truly global entries such as
scanouts, contexts, pdes, execlists etc.
Firstly, I've still yet to expose the grand plan at this point in the
series, so I am not really certain if you're just complaining for the
fun of it, or what. I'd like to make everything functionally the same,
just with VMA support.
I'm complaining because the comment is awful: telling me what the code
is doing but not why. It doesn't seem obvious that if the user
explicitly wanted a global mapping and that the object is not already in
aliasing ppgtt that it is likely to be used in the aliasing ppgtt in the
near future.
Secondly, I was under the impression that for Sandybridge we had to have
all global mappings in the aliasing to support PIPE_CONTROL, or some
command like that. It's a bit mixed up in my head atm, and I'm too lazy
to look at the exact reason.
It does, but if we never enable full-ppgtt for SNB we don't have to
worry about full-ppgtt being unusable for OpenGL (at least not without a
1:1 ppgtt to global mapping of all oq objects).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
I'm sorry. After reading my comments again, you're absolutely right.
How's this?
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2a71a29..fcf36ae 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -658,10 +658,17 @@ static void gen6_ggtt_bind_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma-obj;
const unsigned long entry = vma-node.start PAGE_SHIFT;
- /* If there is an aliasing PPGTT, and the user didn't explicitly ask for
-* the global, just use aliasing */
+ /* If there is no aliasing PPGTT, or the caller needs a global mapping,
+* or we have a global mapping already but the cacheability flags have
+* changed, set the global PTES.
+*
+* If there is an aliasing PPGTT it is anecdotally faster, so use that
+* instead if none of the above hold true.
+*
+* NB: A global mapping should only be needed for special regions like
+* gtt mappable, SNB errata, or if specified via special execbuf flags
+*/
if (!dev_priv-mm.aliasing_ppgtt || flags GLOBAL_BIND) {
- /* If the object is unbound, or we're change the cache bits */
if (!obj-has_global_gtt_mapping ||
(cache_level != obj-cache_level)) {
gen6_ggtt_insert_entries(vma-vm, obj-pages, entry,
@@ -670,8 +677,6 @@ static void gen6_ggtt_bind_vma(struct i915_vma *vma,
}
}
- /* If put the mapping in the aliasing PPGTT as well as Global if we have
-* aliasing, but the user requested global. */
if (dev_priv-mm.aliasing_ppgtt
(!obj-has_aliasing_ppgtt_mapping ||
(cache_level != obj-cache_level))) {
--
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: relayout intel_panel.c to obey 80 char limit
Heh, I was reading the subject as relay-out, not re-layout for a while
there. -ENOCOFFEE.
On Tue, 17 Sep 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
Especially intel_gmch_panel_fitting was shifting way too much over the
right edge and also was way too long. So extract two helpers, one for
gen4+ and one for gen2/3. Now the entire thing is again almost
readable ...
Spurred by checkpatch freaking out about a Ville's pipeconfig rework
in intel_panel.c
Otherwise just two lines that needed appropriate breaking.
I suppose it should be kind of obvious, but I do like the explicit no
functional changes in the commit message when none are intended.
With the reduced indent, you could have re-flowed some statements to
fewer lines while at it... but meh. Good stuff.
Reviewed-by: Jani Nikula jani.nik...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_panel.c | 152
+
1 file changed, 88 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index c9dba46..e36149d 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -73,8 +73,10 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
case DRM_MODE_SCALE_ASPECT:
/* Scale but preserve the aspect ratio */
{
- u32 scaled_width = adjusted_mode-hdisplay *
pipe_config-pipe_src_h;
- u32 scaled_height = pipe_config-pipe_src_w *
adjusted_mode-vdisplay;
+ u32 scaled_width = adjusted_mode-hdisplay
+ * pipe_config-pipe_src_h;
+ u32 scaled_height = pipe_config-pipe_src_w
+ * adjusted_mode-vdisplay;
if (scaled_width scaled_height) { /* pillar */
width = scaled_height / pipe_config-pipe_src_h;
if (width 1)
@@ -169,6 +171,83 @@ static inline u32 panel_fitter_scaling(u32 source, u32
target)
return (FACTOR * ratio + FACTOR/2) / FACTOR;
}
+static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
+ u32 *pfit_control)
+{
+ struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode;
+ u32 scaled_width = adjusted_mode-hdisplay *
+ pipe_config-pipe_src_h;
+ u32 scaled_height = pipe_config-pipe_src_w *
+ adjusted_mode-vdisplay;
+
+ /* 965+ is easy, it does everything in hw */
+ if (scaled_width scaled_height)
+ *pfit_control |= PFIT_ENABLE |
+ PFIT_SCALING_PILLAR;
+ else if (scaled_width scaled_height)
+ *pfit_control |= PFIT_ENABLE |
+ PFIT_SCALING_LETTER;
+ else if (adjusted_mode-hdisplay != pipe_config-pipe_src_w)
+ *pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+}
+
+static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
+ u32 *pfit_control, u32 *pfit_pgm_ratios,
+ u32 *border)
+{
+ struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode;
+ u32 scaled_width = adjusted_mode-hdisplay *
+ pipe_config-pipe_src_h;
+ u32 scaled_height = pipe_config-pipe_src_w *
+ adjusted_mode-vdisplay;
+ u32 bits;
+
+ /*
+ * For earlier chips we have to calculate the scaling
+ * ratio by hand and program it into the
+ * PFIT_PGM_RATIO register
+ */
+ if (scaled_width scaled_height) { /* pillar */
+ centre_horizontally(adjusted_mode,
+ scaled_height /
+ pipe_config-pipe_src_h);
+
+ *border = LVDS_BORDER_ENABLE;
+ if (pipe_config-pipe_src_h != adjusted_mode-vdisplay) {
+ bits = panel_fitter_scaling(pipe_config-pipe_src_h,
+ adjusted_mode-vdisplay);
+
+ *pfit_pgm_ratios |= (bits PFIT_HORIZ_SCALE_SHIFT |
+ bits PFIT_VERT_SCALE_SHIFT);
+ *pfit_control |= (PFIT_ENABLE |
+ VERT_INTERP_BILINEAR |
+ HORIZ_INTERP_BILINEAR);
+ }
+ } else if (scaled_width scaled_height) { /* letter */
+ centre_vertically(adjusted_mode,
+ scaled_width /
+ pipe_config-pipe_src_w);
+
+ *border = LVDS_BORDER_ENABLE;
+ if (pipe_config-pipe_src_w != adjusted_mode-hdisplay) {
+ bits = panel_fitter_scaling(pipe_config-pipe_src_w,
+
Re: [Intel-gfx] drm/i915 stable backport request
On Tue, Sep 17, 2013 at 5:28 AM, Greg KH gre...@linuxfoundation.org wrote: On Tue, Sep 17, 2013 at 01:13:26AM +0200, Daniel Vetter wrote: Dear stable team, Please packport the following upstream commit to 3.10: commit 2960bc9cceecb5d556ce1c07656a66 09e2f7e8b0 Author: Imre Deak imre.d...@intel.com Date: Tue Jul 30 13:36:32 2013 +0300 drm/i915: make user mode sync polarity setting explicit Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69400 Odd, it doesn't apply to 3.10-stable, but it does apply to 3.11-stable, is that what you meant? If 3.10, can you send a backported version that I can apply? Oops, I've meant 3.11 only. I haven't adjusted to the new release number yet ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] DPF (GPU l3 parity detection) improvements
On Tue, Sep 17, 2013 at 6:15 AM, Ben Widawsky benjamin.widaw...@intel.com wrote: I see. I had thought the hang bit was part of the test injection, when it's actually modifying the behavior or L3 errors. Any opinions on what the default should be (agreed that policy should be controlled by user space, but we can control the default)? What does a hang mean exactly, is the rest of memory still responsive, L3? I guess we want a hang-on-L3-error bit at context creation. But that seems orthogonal to fixing l3 error reporting on hsw and wiring up the test facilities, so I'd wait until someone screams for this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: relayout intel_panel.c to obey 80 char limit
On Tue, Sep 17, 2013 at 9:11 AM, Jani Nikula jani.nik...@linux.intel.com wrote: Heh, I was reading the subject as relay-out, not re-layout for a while there. -ENOCOFFEE. Added a dash to the commit message. On Tue, 17 Sep 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote: Especially intel_gmch_panel_fitting was shifting way too much over the right edge and also was way too long. So extract two helpers, one for gen4+ and one for gen2/3. Now the entire thing is again almost readable ... Spurred by checkpatch freaking out about a Ville's pipeconfig rework in intel_panel.c Otherwise just two lines that needed appropriate breaking. I suppose it should be kind of obvious, but I do like the explicit no functional changes in the commit message when none are intended. Me too, but somehow I wanted to add it but then forgotten. Fixed. With the reduced indent, you could have re-flowed some statements to fewer lines while at it... but meh. Good stuff. Reviewed-by: Jani Nikula jani.nik...@intel.com Thanks for the review, patch slurped in. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Add bind/unbind object functions to VM
On Mon, Sep 16, 2013 at 10:44:29PM -0700, Ben Widawsky wrote: I'm sorry. After reading my comments again, you're absolutely right. How's this? I'm liking it better, since it gives some insight into why GLOBAL_BIND is required. diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 2a71a29..fcf36ae 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -658,10 +658,17 @@ static void gen6_ggtt_bind_vma(struct i915_vma *vma, struct drm_i915_gem_object *obj = vma-obj; const unsigned long entry = vma-node.start PAGE_SHIFT; - /* If there is an aliasing PPGTT, and the user didn't explicitly ask for -* the global, just use aliasing */ + /* If there is no aliasing PPGTT, or the caller needs a global mapping, +* or we have a global mapping already but the cacheability flags have +* changed, set the global PTES. +* +* If there is an aliasing PPGTT it is anecdotally faster, so use that +* instead if none of the above hold true. +* +* NB: A global mapping should only be needed for special regions like +* gtt mappable, SNB errata, or if specified via special execbuf flags + At all other times, the GPU will use the aliasing ppgtt. Just adds that extra bit of stress that the predominant access is through the aliasing ppgtt. -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 6/6] drm/i915: Convert overlay double wide check over to pipe config
On Wed, Sep 04, 2013 at 06:30:07PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Entire series merged to dinq, thanks for the patches.
-Daniel
---
drivers/gpu/drm/i915/intel_overlay.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index ddfd0ae..8d6d0a1 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -821,14 +821,11 @@ int intel_overlay_switch_off(struct intel_overlay
*overlay)
static int check_overlay_possible_on_crtc(struct intel_overlay *overlay,
struct intel_crtc *crtc)
{
- drm_i915_private_t *dev_priv = overlay-dev-dev_private;
-
if (!crtc-active)
return -EINVAL;
/* can't use the overlay with double wide pipe */
- if (INTEL_INFO(overlay-dev)-gen 4
- (I915_READ(PIPECONF(crtc-pipe)) (PIPECONF_DOUBLE_WIDE |
PIPECONF_ENABLE)) != PIPECONF_ENABLE)
+ if (crtc-config.double_wide)
return -EINVAL;
return 0;
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Add intel_dotclock_calculate()
On Mon, Sep 16, 2013 at 10:41:38PM +0200, Daniel Vetter wrote: On Mon, Sep 16, 2013 at 02:14:47PM +0300, Jani Nikula wrote: On Fri, 13 Sep 2013, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Extract the code to calculate the dotclock from the link clock and M/N values into a new function from ironlake_crtc_clock_get(). The new function can be used to calculate the dotclock for both FDI and DP cases. Also simplify the code a bit along the way. v2: Don't forget about non-pch encoders in ironlake_crtc_clock_get() Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 43 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 ++ 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c0ee41c..13dea9b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7405,16 +7405,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, pipe_config-adjusted_mode.clock = clock.dot; } -static void ironlake_crtc_clock_get(struct intel_crtc *crtc, - struct intel_crtc_config *pipe_config) +int intel_dotclock_calculate(int link_freq, + const struct intel_link_m_n *m_n) intel_dotclock_calculate is an awfully generic name for something which computes the dotclock for an fdi/dp link ... Maybe intel_dotclock_from_m_n instead? Patch merged since I don't want to block this any longer (and maybe it makes more sense in the end, haven't checked). Probably doesn't. It should be obvious by now that I suck at naming. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Mon, Sep 16, 2013 at 06:48:53PM +0100, Damien Lespiau wrote:
There are a few things to be flushed out if we want to allow multiple
buffers stereo framebuffers:
- What with drm_planes? what semantics do they follow, what is the
hardware able to do with them?
- How do we define which buffer if the right/left one
- Do we allow flips between 1 buffer fbs and 2 buffers fbs (No.)
So for now, and until I get access to hardware that can do that, let's
just disallow 2 buffers stereo framebuffers to not introduce ABI we
wouldn't be able to change afterwards.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_crtc.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 39f60ec..91d1c4b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2131,6 +2131,17 @@ int drm_mode_setcrtc(struct drm_device *dev, void
*data,
goto out;
}
+ /*
+ * Do not allow the use of framebuffers consisting of multiple
+ * buffers with stereo modes until all the details API details
+ * are fleshed out (eg. interaction with drm_planes, switch
+ * between a 1 buffers and a 2 buffers fb, ...)
+ */
+ if (fb-num_buffers 1 drm_mode_is_stereo(mode)) {
+ ret = -EINVAL;
+ goto out;
+ }
This would prevent planar buffers in stereo modes. I'm think we just
ignore the matter for now and let drivers deal with it. We don't have
enough handles anyway for planar stereo, so maybe we even want to add
separate left/right fb attachment properties to the planes instead of
tying it up in inside a single fb. Or we cook up addfb3 when we hit
this problem for real. I think we'd anyway need some kind of flag for
the fb if it contains both left and right buffers.
+
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
hdisplay = mode-hdisplay;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] drm: Make drm_match_cea_mode() return the underlying 2D VIC for 3d modes
On Mon, Sep 16, 2013 at 06:48:54PM +0100, Damien Lespiau wrote:
When scanning out a stereo mode, the AVI infoframe vic field has to be
the underlyng 2D VIC. Before that commit, we weren't matching the CEA
mode because of the extra stereo flag and then were setting the VIC
field in the AVI infoframe to 0.
I think this is where we hit problems. How do we identify the 2D VIC
when the timings of the adjusted mode got mangled already to include
both eyes?
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_edid.c | 4 ++--
drivers/gpu/drm/drm_modes.c | 18 --
include/drm/drm_crtc.h | 2 +-
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9a36b6e..6b74698 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2401,7 +2401,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode
*to_match)
if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) ||
KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2))
- drm_mode_equal_no_clocks(to_match, cea_mode))
+ drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode))
return mode + 1;
}
return 0;
@@ -2450,7 +2450,7 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) ||
KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2))
- drm_mode_equal_no_clocks(to_match, hdmi_mode))
+ drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
return mode + 1;
}
return 0;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 504a602..76adee0 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -851,12 +851,16 @@ bool drm_mode_equal(const struct drm_display_mode
*mode1, const struct drm_displ
} else if (mode1-clock != mode2-clock)
return false;
- return drm_mode_equal_no_clocks(mode1, mode2);
+ if ((mode1-flags DRM_MODE_FLAG_3D_MASK) !=
+ (mode2-flags DRM_MODE_FLAG_3D_MASK))
+ return false;
+
+ return drm_mode_equal_no_clocks_no_stereo(mode1, mode2);
}
EXPORT_SYMBOL(drm_mode_equal);
/**
- * drm_mode_equal_no_clocks - test modes for equality
+ * drm_mode_equal_no_clocks_no_stereo - test modes for equality
* @mode1: first mode
* @mode2: second mode
*
@@ -864,12 +868,13 @@ EXPORT_SYMBOL(drm_mode_equal);
* None.
*
* Check to see if @mode1 and @mode2 are equivalent, but
- * don't check the pixel clocks.
+ * don't check the pixel clocks nor the stereo layout.
*
* RETURNS:
* True if the modes are equal, false otherwise.
*/
-bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const
struct drm_display_mode *mode2)
+bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
+ const struct drm_display_mode *mode2)
{
if (mode1-hdisplay == mode2-hdisplay
mode1-hsync_start == mode2-hsync_start
@@ -881,12 +886,13 @@ bool drm_mode_equal_no_clocks(const struct
drm_display_mode *mode1, const struct
mode1-vsync_end == mode2-vsync_end
mode1-vtotal == mode2-vtotal
mode1-vscan == mode2-vscan
- mode1-flags == mode2-flags)
+ (mode1-flags ~DRM_MODE_FLAG_3D_MASK) ==
+ (mode2-flags ~DRM_MODE_FLAG_3D_MASK))
return true;
return false;
}
-EXPORT_SYMBOL(drm_mode_equal_no_clocks);
+EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
/**
* drm_mode_validate_size - make sure modes adhere to size constraints
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e685baf..f0c5530 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -943,7 +943,7 @@ extern void drm_mode_config_reset(struct drm_device *dev);
extern void drm_mode_config_cleanup(struct drm_device *dev);
extern void drm_mode_set_name(struct drm_display_mode *mode);
extern bool drm_mode_equal(const struct drm_display_mode *mode1, const
struct drm_display_mode *mode2);
-extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1,
const struct drm_display_mode *mode2);
+extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode
*mode1, const struct drm_display_mode *mode2);
extern int drm_mode_width(const struct drm_display_mode *mode);
extern int drm_mode_height(const struct drm_display_mode *mode);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Re: [Intel-gfx] [PATCH 11/12] drm: Make drm_match_cea_mode() return the underlying 2D VIC for 3d modes
On Tue, Sep 17, 2013 at 11:22:26AM +0300, Ville Syrjälä wrote: On Mon, Sep 16, 2013 at 06:48:54PM +0100, Damien Lespiau wrote: When scanning out a stereo mode, the AVI infoframe vic field has to be the underlyng 2D VIC. Before that commit, we weren't matching the CEA mode because of the extra stereo flag and then were setting the VIC field in the AVI infoframe to 0. I think this is where we hit problems. How do we identify the 2D VIC when the timings of the adjusted mode got mangled already to include both eyes? Oh, indeed, that wouldn't work. Note that Daniel had the same remark as you regarding the mangling of the modes, leaking how we implement the modes (in theory hw could need a different way to be programmed). I wondered about that as well and now that we have a couple of good reasons, in kernel timing adjustment for stereo modes will happen in a v5 of this series. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/3] Fix Win8 backlight issue
v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Apply on top of v3.12-rc1. Aaron Lu (3): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists drivers/acpi/internal.h | 5 +- drivers/acpi/video.c| 442 drivers/acpi/video_detect.c | 14 +- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h| 2 + include/linux/backlight.h | 4 + 6 files changed, 300 insertions(+), 198 deletions(-) -- 1.8.4.12.g2ea3df6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/3] ACPI / video: seperate backlight control and event interface
The backlight control and event delivery functionality provided by ACPI
video module is mixed together and registered all during video device
enumeration time. As a result, the two functionality are also removed
together on module unload time or by the acpi_video_unregister function.
The two functionalities are actually independent and one may be useful
while the other one may be broken, so it is desirable to seperate the
two functionalities such that it is clear and easy to disable one
functionality without affecting the other one.
APIs to selectively remove backlight control interface and/or event
delivery functionality can be easily added once needed.
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/acpi/video.c | 451 ++-
include/acpi/video.h | 2 +
2 files changed, 264 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aebcf63..5ad5a71 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
static int register_count;
+static struct mutex video_list_lock;
+static struct list_head video_bus_head;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
@@ -157,6 +159,7 @@ struct acpi_video_bus {
struct acpi_video_bus_flags flags;
struct list_head video_device_list;
struct mutex device_list_lock; /* protects video_device_list */
+ struct list_head entry;
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
@@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_has_method(device-dev-handle, _DDC))
device-cap._DDC = 1;
-
- if (acpi_video_backlight_support()) {
- struct backlight_properties props;
- struct pci_dev *pdev;
- acpi_handle acpi_parent;
- struct device *parent = NULL;
- int result;
- static int count;
- char *name;
-
- result = acpi_video_init_brightness(device);
- if (result)
- return;
- name = kasprintf(GFP_KERNEL, acpi_video%d, count);
- if (!name)
- return;
- count++;
-
- acpi_get_parent(device-dev-handle, acpi_parent);
-
- pdev = acpi_get_pci_dev(acpi_parent);
- if (pdev) {
- parent = pdev-dev;
- pci_dev_put(pdev);
- }
-
- memset(props, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_FIRMWARE;
- props.max_brightness = device-brightness-count - 3;
- device-backlight = backlight_device_register(name,
- parent,
- device,
-
acpi_backlight_ops,
- props);
- kfree(name);
- if (IS_ERR(device-backlight))
- return;
-
- /*
-* Save current brightness level in case we have to restore it
-* before acpi_video_device_lcd_set_level() is called next time.
-*/
- device-backlight-props.brightness =
- acpi_video_get_brightness(device-backlight);
-
- device-cooling_dev = thermal_cooling_device_register(LCD,
- device-dev, video_cooling_ops);
- if (IS_ERR(device-cooling_dev)) {
- /*
-* Set cooling_dev to NULL so we don't crash trying to
-* free it.
-* Also, why the hell we are returning early and
-* not attempt to register video output if cooling
-* device registration failed?
-* -- dtor
-*/
- device-cooling_dev = NULL;
- return;
- }
-
- dev_info(device-dev-dev, registered as cooling_device%d\n,
-device-cooling_dev-id);
- result = sysfs_create_link(device-dev-dev.kobj,
- device-cooling_dev-device.kobj,
- thermal_cooling);
- if (result)
- printk(KERN_ERR PREFIX Create sysfs link\n);
- result = sysfs_create_link(device-cooling_dev-device.kobj,
-
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 10:03:12AM +0100, Damien Lespiau wrote:
On Tue, Sep 17, 2013 at 11:20:46AM +0300, Ville Syrjälä wrote:
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2131,6 +2131,17 @@ int drm_mode_setcrtc(struct drm_device *dev, void
*data,
goto out;
}
+ /*
+ * Do not allow the use of framebuffers consisting of multiple
+ * buffers with stereo modes until all the details API details
+ * are fleshed out (eg. interaction with drm_planes, switch
+ * between a 1 buffers and a 2 buffers fb, ...)
+ */
+ if (fb-num_buffers 1 drm_mode_is_stereo(mode)) {
+ ret = -EINVAL;
+ goto out;
+ }
This would prevent planar buffers in stereo modes. I'm think we just
ignore the matter for now and let drivers deal with it. We don't have
enough handles anyway for planar stereo, so maybe we even want to add
separate left/right fb attachment properties to the planes instead of
tying it up in inside a single fb. Or we cook up addfb3 when we hit
this problem for real. I think we'd anyway need some kind of flag for
the fb if it contains both left and right buffers.
I'm quite happy to ignore 3 planes YUV stereo fbs for now :) (2 planes
YUV stereo fbs still fit!).
Are you fine with this test though, or do you mean ignore the whole
matter of forbidding this case (or just the multiplane stereo fb case)?
I was just thinking that I missed the addition of this check in the page
flip ioctl.
Yeah, I was thinking we that we can ignore this issue for now, and so we
wouldn't need the check. Currently for non-stereo cases the only thing
we check is that there is a valid handle for each plane. If userspace
passes more handles, we simply ignore the extra ones.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges
On Mon, Sep 16, 2013 at 11:52:17PM +0200, Daniel Vetter wrote:
On Wed, Sep 04, 2013 at 06:25:31PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
First of all we should not be looking at fb-{width,height} as those do
not tell us what the actual pipe size is. Second of all we need to use
= for the comparison.
So fix the comparison, and make use of the new pipe_src_{w,h} to
determine the real pipe source dimensions.
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Merged the entire series to dinq, thanks for patchesreview. I've also
made a small note that we should have an igt testcase for these cursor
corner-cases somewhere ...
Which reminds me that I started to write one. Need to finish it up...
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 12:54:09PM +0300, Ville Syrjälä wrote:
On Tue, Sep 17, 2013 at 10:03:12AM +0100, Damien Lespiau wrote:
On Tue, Sep 17, 2013 at 11:20:46AM +0300, Ville Syrjälä wrote:
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2131,6 +2131,17 @@ int drm_mode_setcrtc(struct drm_device *dev,
void *data,
goto out;
}
+ /*
+* Do not allow the use of framebuffers consisting of
multiple
+* buffers with stereo modes until all the details API
details
+* are fleshed out (eg. interaction with drm_planes,
switch
+* between a 1 buffers and a 2 buffers fb, ...)
+*/
+ if (fb-num_buffers 1 drm_mode_is_stereo(mode)) {
+ ret = -EINVAL;
+ goto out;
+ }
This would prevent planar buffers in stereo modes. I'm think we just
ignore the matter for now and let drivers deal with it. We don't have
enough handles anyway for planar stereo, so maybe we even want to add
separate left/right fb attachment properties to the planes instead of
tying it up in inside a single fb. Or we cook up addfb3 when we hit
this problem for real. I think we'd anyway need some kind of flag for
the fb if it contains both left and right buffers.
I'm quite happy to ignore 3 planes YUV stereo fbs for now :) (2 planes
YUV stereo fbs still fit!).
Are you fine with this test though, or do you mean ignore the whole
matter of forbidding this case (or just the multiplane stereo fb case)?
I was just thinking that I missed the addition of this check in the page
flip ioctl.
Yeah, I was thinking we that we can ignore this issue for now, and so we
wouldn't need the check. Currently for non-stereo cases the only thing
we check is that there is a valid handle for each plane. If userspace
passes more handles, we simply ignore the extra ones.
I guess we should start to check that. For 3d framebuffers with 2
separate buffer handles for each plane I think we need to add another flag
to addfb2, e.g.
#define DRM_MODE_FB_3D_2_FRAMES (11) /* separate left/right buffers, doubles
plane count */
and then also throw in the respective check code into the core that
userspace supplies sufficient amounts of buffers in framebuffer_check()
by adjusting drM_format_num_planes and drm_format_plane_cpp.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 12:37:48PM +0200, Daniel Vetter wrote:
On Tue, Sep 17, 2013 at 12:54:09PM +0300, Ville Syrjälä wrote:
On Tue, Sep 17, 2013 at 10:03:12AM +0100, Damien Lespiau wrote:
On Tue, Sep 17, 2013 at 11:20:46AM +0300, Ville Syrjälä wrote:
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2131,6 +2131,17 @@ int drm_mode_setcrtc(struct drm_device *dev,
void *data,
goto out;
}
+ /*
+ * Do not allow the use of framebuffers consisting of
multiple
+ * buffers with stereo modes until all the details API
details
+ * are fleshed out (eg. interaction with drm_planes,
switch
+ * between a 1 buffers and a 2 buffers fb, ...)
+ */
+ if (fb-num_buffers 1 drm_mode_is_stereo(mode)) {
+ ret = -EINVAL;
+ goto out;
+ }
This would prevent planar buffers in stereo modes. I'm think we just
ignore the matter for now and let drivers deal with it. We don't have
enough handles anyway for planar stereo, so maybe we even want to add
separate left/right fb attachment properties to the planes instead of
tying it up in inside a single fb. Or we cook up addfb3 when we hit
this problem for real. I think we'd anyway need some kind of flag for
the fb if it contains both left and right buffers.
I'm quite happy to ignore 3 planes YUV stereo fbs for now :) (2 planes
YUV stereo fbs still fit!).
Are you fine with this test though, or do you mean ignore the whole
matter of forbidding this case (or just the multiplane stereo fb case)?
I was just thinking that I missed the addition of this check in the page
flip ioctl.
Yeah, I was thinking we that we can ignore this issue for now, and so we
wouldn't need the check. Currently for non-stereo cases the only thing
we check is that there is a valid handle for each plane. If userspace
passes more handles, we simply ignore the extra ones.
I guess we should start to check that. For 3d framebuffers with 2
separate buffer handles for each plane I think we need to add another flag
to addfb2, e.g.
#define DRM_MODE_FB_3D_2_FRAMES (11) /* separate left/right buffers,
doubles plane count */
and then also throw in the respective check code into the core that
userspace supplies sufficient amounts of buffers in framebuffer_check()
by adjusting drM_format_num_planes and drm_format_plane_cpp.
Well, we shouldn't mix the plane vs. buffers/handles/whatever concepts. The
number of planes is clearly defined by the pixel format. But yes, we should
probably add a drm_format_num_buffers() function to figure out how many buffers
are required.
But the problem is that addfb2 can't supply more than 4. So we need a new
ioctl if we want to collect all that information inside a single drm_fb
object. If we do add another ioctl then I think we should go at least to
16, since we might also want to have separate buffers for each field in
interlaced framebuffers. And then we need to clearly define which handle
is which plane/field/eye. Something like this for example:
eye=left field=top plane=0 [plane=1 ...] [field=bottom plane=0 [plane=1 ...]]
[eye=right field=top plane=0 [plane=1 ...] [field=bottom plane=0 [plane=1
...]]]
so you first have all the planes for left/top, then all planes for
left/bottom, then right/top, and finally right/bottom.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: only report hpd connector status change when it actually changed
This reduces dmesg noise when there's a glitch on the hpd line, or there
are more than one connectors on the same hpd line and only one of them
changes.
While at it, switch to use the friendly status names instead of numbers.
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 13d26cf..a42f30b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device
*dev, int pipe,
crtc);
}
-static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector
*connector)
+static bool intel_hpd_irq_event(struct drm_device *dev,
+ struct drm_connector *connector)
{
enum drm_connector_status old_status;
@@ -673,11 +674,16 @@ static int intel_hpd_irq_event(struct drm_device *dev,
struct drm_connector *con
old_status = connector-status;
connector-status = connector-funcs-detect(connector, false);
- DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %d to %d\n,
+ if (old_status == connector-status)
+ return false;
+
+ DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %s to %s\n,
connector-base.id,
drm_get_connector_name(connector),
- old_status, connector-status);
- return (old_status != connector-status);
+ drm_get_connector_status_name(old_status),
+ drm_get_connector_status_name(connector-status));
+
+ return true;
}
/*
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/3] ACPI / video: seperate backlight control and event interface
On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: The backlight control and event delivery functionality provided by ACPI video module is mixed together and registered all during video device enumeration time. As a result, the two functionality are also removed together on module unload time or by the acpi_video_unregister function. The two functionalities are actually independent and one may be useful while the other one may be broken, so it is desirable to seperate the two functionalities such that it is clear and easy to disable one functionality without affecting the other one. APIs to selectively remove backlight control interface and/or event delivery functionality can be easily added once needed. Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/video.c | 451 ++- include/acpi/video.h | 2 + 2 files changed, 264 insertions(+), 189 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.12.0-0.rc1.git0.1.fc20.x86_64 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 12:37:48PM +0200, Daniel Vetter wrote: I guess we should start to check that. For 3d framebuffers with 2 separate buffer handles for each plane I think we need to add another flag to addfb2, e.g. #define DRM_MODE_FB_3D_2_FRAMES (11) /* separate left/right buffers, doubles plane count */ and then also throw in the respective check code into the core that userspace supplies sufficient amounts of buffers in framebuffer_check() by adjusting drM_format_num_planes and drm_format_plane_cpp. Would we really need that flag? we can just count the number of buffers and decide from that number whether we're in the 1 or 2 buffer(s) case? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 02:20:41PM +0100, Damien Lespiau wrote: On Tue, Sep 17, 2013 at 12:37:48PM +0200, Daniel Vetter wrote: I guess we should start to check that. For 3d framebuffers with 2 separate buffer handles for each plane I think we need to add another flag to addfb2, e.g. #define DRM_MODE_FB_3D_2_FRAMES (11) /* separate left/right buffers, doubles plane count */ and then also throw in the respective check code into the core that userspace supplies sufficient amounts of buffers in framebuffer_check() by adjusting drM_format_num_planes and drm_format_plane_cpp. Would we really need that flag? we can just count the number of buffers and decide from that number whether we're in the 1 or 2 buffer(s) case? I'd go with explicit flags. We have one for interlaced already. Although ATM the interlaced flag must mean that the fields are interleaved in the same buffer. But if we want separate buffers for each field, we need a flag for that too. Or we just pretend the old interlace flag doesn't exist yet and steal it for that purpose. I don't think anyone is using the flag yet. But anyway we'd need a new ioctl :( -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 3:20 PM, Damien Lespiau damien.lesp...@intel.com wrote: On Tue, Sep 17, 2013 at 12:37:48PM +0200, Daniel Vetter wrote: I guess we should start to check that. For 3d framebuffers with 2 separate buffer handles for each plane I think we need to add another flag to addfb2, e.g. #define DRM_MODE_FB_3D_2_FRAMES (11) /* separate left/right buffers, doubles plane count */ and then also throw in the respective check code into the core that userspace supplies sufficient amounts of buffers in framebuffer_check() by adjusting drM_format_num_planes and drm_format_plane_cpp. Would we really need that flag? we can just count the number of buffers and decide from that number whether we're in the 1 or 2 buffer(s) case? We do not know at addfb2 time that it'll be a 3d mode buffer, and drivers might want to know that to internally assign the buffers to left/right buffer handle pointers. Similar to how we already have a flag for when the buffer is interlaced, which atm we don't support (since we only support scan-out of progressive buffers to interlaced modes, not interlaced to interlaced). So imo a flag here makes sense so that the driver can properly check constraints and make sense of the handles. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes
On Tue, Sep 17, 2013 at 1:02 PM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: But the problem is that addfb2 can't supply more than 4. So we need a new ioctl if we want to collect all that information inside a single drm_fb object. If we do add another ioctl then I think we should go at least to 16, since we might also want to have separate buffers for each field in interlaced framebuffers. And then we need to clearly define which handle is which plane/field/eye. Something like this for example: Blergh, I've thought we have gone with at least 6 buffers for the 3d case. Meh :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: WARN is the DP aux read or write is too big
From: Paulo Zanoni paulo.r.zan...@intel.com
So far we control everything and nothing exceeds the current limits,
but (i) we never think about these limits when reviewing patches, (ii)
not all the callers check the return values and (iii) if we ever hit
any of these messages, we'll have to fix the code that added the bad
message.
The current limit for these messages is 20 since we only have 5 data
registers on all the current gens.
The checks inside intel_dp_aux_native_{write,read} are to prevent
buffer overflows. The check inside intel_dp_aux_ch is to prevent
writing past our 5 data registers.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8c70a83..c324a59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -436,6 +436,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
goto out;
}
+ /* Only 5 data registers! */
+ if (WARN_ON(send_bytes 20 || recv_size 20)) {
+ ret = -E2BIG;
+ goto out;
+ }
+
while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
/* Must try at least 3 times according to DP spec */
for (try = 0; try 5; try++) {
@@ -526,9 +532,10 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
int msg_bytes;
uint8_t ack;
+ if (WARN_ON(send_bytes 16))
+ return -E2BIG;
+
intel_dp_check_edp(intel_dp);
- if (send_bytes 16)
- return -1;
msg[0] = AUX_NATIVE_WRITE 4;
msg[1] = address 8;
msg[2] = address 0xff;
@@ -569,6 +576,9 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
uint8_t ack;
int ret;
+ if (WARN_ON(recv_bytes 19))
+ return -E2BIG;
+
intel_dp_check_edp(intel_dp);
msg[0] = AUX_NATIVE_READ 4;
msg[1] = address 8;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/12] drm/edid: Expose mandatory stereo modes for HDMI sinks
On Mon, Sep 16, 2013 at 09:29:31PM +0300, Ville Syrjälä wrote:
+struct stereo_mandatory_mode {
+ int width, height, freq;
[..]
+ unsigned int interlace_flag, layouts;
What's the benefit of separating the two?
Just that we can easily cycle through the layout flags and add a mode
per flag without having to clear the interlace bit. Trade 16 bytes against
one instruction or so, can do I guess.
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: only report hpd connector status change when it actually changed
On Tue, Sep 17, 2013 at 02:26:34PM +0300, Jani Nikula wrote:
This reduces dmesg noise when there's a glitch on the hpd line, or there
are more than one connectors on the same hpd line and only one of them
changes.
While at it, switch to use the friendly status names instead of numbers.
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 13d26cf..a42f30b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device
*dev, int pipe,
crtc);
}
-static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector
*connector)
+static bool intel_hpd_irq_event(struct drm_device *dev,
+ struct drm_connector *connector)
You change the return value to a bool here, but no users. Missing
follow-up patch?
-Daniel
{
enum drm_connector_status old_status;
@@ -673,11 +674,16 @@ static int intel_hpd_irq_event(struct drm_device *dev,
struct drm_connector *con
old_status = connector-status;
connector-status = connector-funcs-detect(connector, false);
- DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %d to %d\n,
+ if (old_status == connector-status)
+ return false;
+
+ DRM_DEBUG_KMS([CONNECTOR:%d:%s] status updated from %s to %s\n,
connector-base.id,
drm_get_connector_name(connector),
- old_status, connector-status);
- return (old_status != connector-status);
+ drm_get_connector_status_name(old_status),
+ drm_get_connector_status_name(connector-status));
+
+ return true;
}
/*
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: WARN is the DP aux read or write is too big
On Tue, 17 Sep 2013, Paulo Zanoni przan...@gmail.com wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
So far we control everything and nothing exceeds the current limits,
but (i) we never think about these limits when reviewing patches, (ii)
not all the callers check the return values and (iii) if we ever hit
any of these messages, we'll have to fix the code that added the bad
message.
The current limit for these messages is 20 since we only have 5 data
registers on all the current gens.
The checks inside intel_dp_aux_native_{write,read} are to prevent
buffer overflows. The check inside intel_dp_aux_ch is to prevent
writing past our 5 data registers.
I wish there were fewer magic values, but it does what it says on the
box.
Reviewed-by: Jani Nikula jani.nik...@intel.com
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8c70a83..c324a59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -436,6 +436,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
goto out;
}
+ /* Only 5 data registers! */
+ if (WARN_ON(send_bytes 20 || recv_size 20)) {
+ ret = -E2BIG;
+ goto out;
+ }
+
while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
/* Must try at least 3 times according to DP spec */
for (try = 0; try 5; try++) {
@@ -526,9 +532,10 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
int msg_bytes;
uint8_t ack;
+ if (WARN_ON(send_bytes 16))
+ return -E2BIG;
+
intel_dp_check_edp(intel_dp);
- if (send_bytes 16)
- return -1;
msg[0] = AUX_NATIVE_WRITE 4;
msg[1] = address 8;
msg[2] = address 0xff;
@@ -569,6 +576,9 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
uint8_t ack;
int ret;
+ if (WARN_ON(recv_bytes 19))
+ return -E2BIG;
+
intel_dp_check_edp(intel_dp);
msg[0] = AUX_NATIVE_READ 4;
msg[1] = address 8;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: WARN is the DP aux read or write is too big
On Tue, Sep 17, 2013 at 06:15:31PM +0300, Jani Nikula wrote:
On Tue, 17 Sep 2013, Paulo Zanoni przan...@gmail.com wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
So far we control everything and nothing exceeds the current limits,
but (i) we never think about these limits when reviewing patches, (ii)
not all the callers check the return values and (iii) if we ever hit
any of these messages, we'll have to fix the code that added the bad
message.
The current limit for these messages is 20 since we only have 5 data
registers on all the current gens.
The checks inside intel_dp_aux_native_{write,read} are to prevent
buffer overflows. The check inside intel_dp_aux_ch is to prevent
writing past our 5 data registers.
I wish there were fewer magic values, but it does what it says on the
box.
Reviewed-by: Jani Nikula jani.nik...@intel.com
Is that for both patches? I've merged the first one for now to dinq ...
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: only report hpd connector status change when it actually changed
On Tue, Sep 17, 2013 at 05:56:08PM +0300, Jani Nikula wrote: On Tue, 17 Sep 2013, Daniel Vetter dan...@ffwll.ch wrote: On Tue, Sep 17, 2013 at 02:26:34PM +0300, Jani Nikula wrote: This reduces dmesg noise when there's a glitch on the hpd line, or there are more than one connectors on the same hpd line and only one of them changes. While at it, switch to use the friendly status names instead of numbers. Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 13d26cf..a42f30b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } -static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector) +static bool intel_hpd_irq_event(struct drm_device *dev, + struct drm_connector *connector) You change the return value to a bool here, but no users. Missing follow-up patch? Nope, the only call site in i915_hotplug_work_func() already treats it as a bool. Somehow I've thought it started out with a void and you switched to bool. Thanks for removing my blinders, patch merged ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: check for more ASLC interrupts
On Tue, Sep 17, 2013 at 06:29:46PM +0300, Jani Nikula wrote: On Tue, 17 Sep 2013, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Sometimes I see the non asle set request?? message on my Haswell machine, so I decided to get the spec and see if some bits are missing from the mask. We do have some bits missing from the mask, so this patch adds them, and the corresponding code to print unsupported messages just like we do with the other bits we don't support. But I still see the non asle set request?? message on my machine :( Also use the proper ASLC name to indicate the registers we're talking about. v2: - Properly set the new FAILED bits - Rename the old FAILED bits - Print everything we don't support Reviewed-by: Jani Nikula jani.nik...@intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Finish enabling rps before use by sysfs or debugfs
On Tue, Sep 17, 2013 at 12:12 AM, Daniel Vetter dan...@ffwll.ch wrote: On Mon, Sep 16, 2013 at 02:56:43PM -0700, Tom.O'rou...@intel.com wrote: From: Tom O'Rourke Tom.O'rou...@intel.com Enabling rps (turbo setup) was put in a work queue because it may take quite awhile. This change flushes the work queue to initialize rps values before use by sysfs or debugfs. Specifically, rps.delayed_resume_work is flushed before using rps.hw_max, rps.max_delay, rps.min_delay, or rps.cur_delay. This change fixes a problem in sysfs where show functions using uninitialized values show incorrect values and store functions using uninitialized values in range checks incorrectly fail to store valid input values. This change also addresses similar use before initialized problems in debugfs. Change-Id: Ib9c4f2066b65013094cb9278fc17958a964836e7 Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com I think we can exercise this by going through a suspend/resume cycle and racing concurrent reads/writes of the sysfs files in a 2nd process. With the igt_fork and igt_system_suspend_autoresume helpers in our kernel testsuite repro this should be a really quick testcase to write ... I'd go for a generic read all sysfs files approach to increase the chances of catching other similar fallout in the future. To clarify: I'd like to see a testcase for this so that we can make sure it keeps working. I guess we unfortunately can't test for functional correctness by just resuming since we won't see uninitialized data. But at least we can test for the flush_delayed_work deadlock implications, which have bitten us badly in the past on numerous occasions. And maybe we can provoke hangs if we adjust the rps values while the setup hasn't completed yet, so might also give us weak coverage there. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe
On Tue, Sep 17, 2013 at 05:24:01PM +0100, Chris Wilson wrote:
On Tue, Sep 17, 2013 at 06:33:44PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
On HSW enabling a plane on a disabled pipe may hang the entire system.
And there's no good reason for doing it ever, so just don't.
v2: Move the crtc active checks to intel_crtc_cursor_{set,move} to
avoid confusing people during modeset
But outside of modeset the existing checks are accurate.
There are no existing checks anymore. The crtc-enabled check ended up
as collateral damage in the cursor visibility patches.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] [v5] drm/i915: Add bind/unbind object functions to VM
From: Ben Widawsky b...@bwidawsk.net
As we plumb the code with more VM information, it has become more
obvious that the easiest way to deal with bind and unbind is to simply
put the function pointers in the vm, and let those choose the correct
way to handle the page table updates. This change allows many places in
the code to simply be vm-bind, and not have to worry about
distinguishing PPGTT vs GGTT.
Notice that this patch has no impact on functionality. I've decided to
save the actual change until the next patch because I think it's easier
to review that way. I'm happy to squash the two, or let Daniel do it on
merge.
v2:
Make ggtt handle the quirky aliasing ppgtt
Add flags to bind object to support above
Don't ever call bind/unbind directly for PPGTT until we have real, full
PPGTT (use NULLs to assert this)
Make sure we rebind the ggtt if there already is a ggtt binding. This
happens on set cache levels.
Use VMA for bind/unbind (Daniel, Ben)
v3: Reorganize ggtt_vma_bind to be more concise and easier to read
(Ville). Change logic in unbind to only unbind ggtt when there is a
global mapping, and to remove a redundant check if the aliasing ppgtt
exists.
v4: Make the bind function a bit smarter about the cache levels to avoid
unnecessary multiple remaps. I accept it is a wart, I think unifying
the pin_vma / bind_vma could be unified later (Chris)
Removed the git notes, and put version info here. (Daniel)
v5: Update the comment to not suck (Chris)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 69 --
drivers/gpu/drm/i915/i915_gem_gtt.c | 112
2 files changed, 151 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 427c537..686a66c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -465,6 +465,36 @@ enum i915_cache_level {
typedef uint32_t gen6_gtt_pte_t;
+/**
+ * A VMA represents a GEM BO that is bound into an address space. Therefore, a
+ * VMA's presence cannot be guaranteed before binding, or after unbinding the
+ * object into/from the address space.
+ *
+ * To make things as simple as possible (ie. no refcounting), a VMA's lifetime
+ * will always be = an objects lifetime. So object refcounting should cover
us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ /** This object's place on the active/inactive lists */
+ struct list_head mm_list;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+
+ /** This vma's place in the batchbuffer or on the eviction list */
+ struct list_head exec_list;
+
+ /**
+* Used for performing relocations during execbuffer insertion.
+*/
+ struct hlist_node exec_node;
+ unsigned long exec_handle;
+ struct drm_i915_gem_exec_object2 *exec_entry;
+
+};
+
struct i915_address_space {
struct drm_mm mm;
struct drm_device *dev;
@@ -503,9 +533,18 @@ struct i915_address_space {
/* FIXME: Need a more generic return type */
gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
enum i915_cache_level level);
+
+ /** Unmap an object from an address space. This usually consists of
+* setting the valid PTE entries to a reserved scratch page. */
+ void (*unbind_vma)(struct i915_vma *vma);
void (*clear_range)(struct i915_address_space *vm,
unsigned int first_entry,
unsigned int num_entries);
+ /* Map an object into an address space with the given cache flags. */
+#define GLOBAL_BIND (10)
+ void (*bind_vma)(struct i915_vma *vma,
+enum i915_cache_level cache_level,
+u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st,
unsigned int first_entry,
@@ -552,36 +591,6 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
-/**
- * A VMA represents a GEM BO that is bound into an address space. Therefore, a
- * VMA's presence cannot be guaranteed before binding, or after unbinding the
- * object into/from the address space.
- *
- * To make things as simple as possible (ie. no refcounting), a VMA's lifetime
- * will always be = an objects lifetime. So object refcounting should cover
us.
- */
-struct i915_vma {
- struct drm_mm_node node;
- struct drm_i915_gem_object *obj;
- struct i915_address_space *vm;
-
- /** This object's place on the active/inactive lists */
- struct list_head mm_list;
-
- struct list_head vma_link; /* Link in the object's VMA list */
-
- /** This vma's place in the batchbuffer or on the eviction list */
- struct list_head
Re: [Intel-gfx] [PATCH 15/19] drm/i915: i915.quirks_set/quirks_mask overrides dmi match
On Wed, 2013-09-11 at 11:21 +0300, Jani Nikula wrote:
On Wed, 11 Sep 2013, Rodrigo Vivi rodrigo.v...@gmail.com wrote:
From: Kamal Mostafa ka...@canonical.com
Boot params quirks_set and quirks_mask allow the user to enable or
inhibit any dmi-matched quirks, overriding the dmi match table. Examples:
i915.quirks_set=0x2 - enables QUIRK_LVDS_SSC_DISABLE
i915.quirks_set=0x8 - enables QUIRK_NO_PCH_PWM_ENABLE
i915.quirks_mask=0x8 - disables QUIRK_NO_PCH_PWM_ENABLE
i915.quirks_mask=0xFF - disables all quirks
Thanks for reviewing this, Jani. I hope you're open to a little more
debate on the topic...
While I admit this can be used to workaround issues with certain
machines, this is a hack that exposes an implementation detail to the
userspace, and carves it into the stone. Any user of it would have to
look at the kernel source to make a smart choice of parameters; more
likely forums would start filling with tips what to set.
All of the above is true of all boot params. Why is that a problem?
Consider that if this quirks_set/mask feature were available:
- Users and developers could trivially check whether any future machine
needs to be added to any of the existing dmi-match quirk tables.
Currently we have to build a test kernel and get the user to install it,
but this would allow us to just ask Does i915.quirks_set=0x2 fix it?.
That feature alone makes me want quirks_set/mask.
- We would unblock some black-screen-on-boot users who currently have
no other solution. And we would unblock future users who face future
dmi/quirk-mismatch issues (of which I am sure there will be more).
- The existing i915.invert_brightness override param would not have
been needed. (It is worth nothing also that my previously proposed
i915.disable_pch_pwm override patch was rejected, even though its
implementation is identical to invert_brightness.)
So NAK from me.
That said, I think we still have a few stones unturned wrt backlight and
what BIOS does in UEFI mode.
I would be thrilled to see the UEFI/BIOS/backlight issue get fixed, but
in my opinion i915.quirks_set/mask would be very useful (a) anyway, and
(b) immediately.
-Kamal
Cheers,
Jani.
Signed-off-by: Kamal Mostafa ka...@canonical.com
Cc: sta...@vger.kernel.org
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/intel_display.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1eabca3..12607f2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10043,8 +10043,19 @@ static struct intel_quirk intel_quirks[] = {
{ 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable },
};
+unsigned long i915_quirks_set __read_mostly = 0;
+module_param_named(quirks_set, i915_quirks_set, ulong, 0600);
+MODULE_PARM_DESC(quirks_set,
+ Enable specified quirks bits.);
+
+unsigned long i915_quirks_mask __read_mostly = 0;
+module_param_named(quirks_mask, i915_quirks_mask, ulong, 0600);
+MODULE_PARM_DESC(quirks_mask,
+ Disable specified quirks bits (override dmi match).);
+
static void intel_init_quirks(struct drm_device *dev)
{
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct pci_dev *d = dev-pdev;
int i;
@@ -10062,6 +10073,10 @@ static void intel_init_quirks(struct drm_device
*dev)
if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
intel_dmi_quirks[i].hook(dev);
}
+
+ /* handle user-specified quirks overrides */
+ dev_priv-quirks |= i915_quirks_set;
+ dev_priv-quirks = ~i915_quirks_mask;
}
/* Disable the VGA plane that we never use */
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] DPF (GPU l3 parity detection) improvements
On Windows, with the hang bit set, we do the cache line replacement after an error occurs, but nothing else [so L3 log registers are definitely responsive, I don't know if other memory is also]. -Original Message- From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, September 17, 2013 12:28 AM To: Widawsky, Benjamin Cc: Bell, Bryan J; intel-gfx@lists.freedesktop.org; Venkatesh, Vishnu Subject: Re: [Intel-gfx] [PATCH 0/8] DPF (GPU l3 parity detection) improvements On Tue, Sep 17, 2013 at 6:15 AM, Ben Widawsky benjamin.widaw...@intel.com wrote: I see. I had thought the hang bit was part of the test injection, when it's actually modifying the behavior or L3 errors. Any opinions on what the default should be (agreed that policy should be controlled by user space, but we can control the default)? What does a hang mean exactly, is the rest of memory still responsive, L3? I guess we want a hang-on-L3-error bit at context creation. But that seems orthogonal to fixing l3 error reporting on hsw and wiring up the test facilities, so I'd wait until someone screams for this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/2] drm/i915: HSW modeset hang fix v2
2013/9/17 ville.syrj...@linux.intel.com: v2 as requested. The first patch just got rebased, the second one I adjusted a bit. I've been testing this for the last 5 minutes and it seems the problem is fixed. Now instead of a hard hang I get a FIFO underrun :) Tested-by: Paulo Zanoni paulo.r.zan...@intel.com I'll keep using these patches (because this machine is almost unusable without them) and I'll report if I see the hang again. I'll also start investigating the underrun. Thanks, Paulo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/8] drm/i915: Add second slice l3 remapping
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, September 17, 2013 12:02 PM
To: Bell, Bryan J
Cc: Ben Widawsky; Widawsky, Benjamin; intel-gfx@lists.freedesktop.org;
Venkatesh, Vishnu
Subject: Re: [Intel-gfx] [PATCH 5/8] drm/i915: Add second slice l3 remapping
On Tue, Sep 17, 2013 at 06:51:31PM +, Bell, Bryan J wrote:
+ spin_lock_irqsave(dev_priv-irq_lock, flags);
+ ilk_enable_gt_irq(dev_priv, GT_PARITY_ERROR);
Is it actually safe to enable the second slice irq when there's no
second slice? This docs say it's just reserved, but no mention
whether it RO or could there be side effects.
Tests on my machine appear to work. But I don't know for certain. Bryan,
could you answer this?
On the Windows driver we enable the IRQ on all HSW skus and haven't seen
any issues.
This code would enable it for IVB too. Any data on that?
No data on IVB, I recommend against enabling it on IVB.
FYI: My understanding is that L3 DPF code and or interrupts should be disabled
on VLV.
VLV does not support dynamic L3 row replacement.
-Original Message-
From: Ben Widawsky [mailto:b...@bwidawsk.net]
Sent: Tuesday, September 17, 2013 11:46 AM
To: Ville Syrjälä; Bell, Bryan J
Cc: Widawsky, Benjamin; intel-gfx@lists.freedesktop.org; Venkatesh,
Vishnu
Subject: Re: [Intel-gfx] [PATCH 5/8] drm/i915: Add second slice l3
remapping
On Fri, Sep 13, 2013 at 12:38:01PM +0300, Ville Syrjälä wrote:
On Thu, Sep 12, 2013 at 10:28:31PM -0700, Ben Widawsky wrote:
Certain HSW SKUs have a second bank of L3. This L3 remapping has a
separate register set, and interrupt from the first slice. A
slice is simply a term to define some subset of the GPU's l3
cache. This patch implements both the interrupt handler, and
ability to communicate with userspace about this second slice.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 9 ++--
drivers/gpu/drm/i915/i915_gem.c | 26 ++
drivers/gpu/drm/i915/i915_irq.c | 84
+
drivers/gpu/drm/i915/i915_reg.h | 6 +++
drivers/gpu/drm/i915/i915_sysfs.c | 34 ++---
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +--
include/uapi/drm/i915_drm.h | 8 ++--
7 files changed, 115 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index 81ba5bb..eb90461 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -918,9 +918,11 @@ struct i915_ums_state {
int mm_suspended;
};
+#define MAX_L3_SLICES 2
struct intel_l3_parity {
- u32 *remap_info;
+ u32 *remap_info[MAX_L3_SLICES];
struct work_struct error_work;
+ int which_slice;
};
struct i915_gem_mm {
@@ -1686,7 +1688,8 @@ struct drm_i915_file_private {
#define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)-has_force_wake)
-#define HAS_L3_GPU_CACHE(dev) (IS_IVYBRIDGE(dev) ||
IS_HASWELL(dev))
+#define HAS_L3_GPU_CACHE(dev) (INTEL_INFO(dev)-gen = 7) #define
+NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_GPU_CACHE(dev))
#define GT_FREQUENCY_MULTIPLIER 50
@@ -1947,7 +1950,7 @@ bool i915_gem_clflush_object(struct
drm_i915_gem_object *obj, bool force); int __must_check
i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj); int
__must_check i915_gem_init(struct drm_device *dev); int
__must_check i915_gem_init_hw(struct drm_device *dev); -void
i915_gem_l3_remap(struct drm_device *dev);
+void i915_gem_l3_remap(struct drm_device *dev, int slice);
void i915_gem_init_swizzling(struct drm_device *dev); void
i915_gem_cleanup_ringbuffer(struct drm_device *dev); int
__must_check i915_gpu_idle(struct drm_device *dev); diff --git
a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c index 5b510a3..b11f7d6c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4256,16 +4256,21 @@ i915_gem_idle(struct drm_device *dev)
return 0;
}
-void i915_gem_l3_remap(struct drm_device *dev)
+void i915_gem_l3_remap(struct drm_device *dev, int slice)
{
drm_i915_private_t *dev_priv = dev-dev_private;
+ u32 reg_base = GEN7_L3LOG_BASE + (slice * 0x200);
+ u32 *remap_info = dev_priv-l3_parity.remap_info[slice];
u32 misccpctl;
int i;
if (!HAS_L3_GPU_CACHE(dev))
return;
- if (!dev_priv-l3_parity.remap_info)
+ if (NUM_L3_SLICES(dev) 2 slice)
+ return;
This check is redundant as we should never populate
Re: [Intel-gfx] [PATCH 2/2] drm/i915: check for more ASLC interrupts
On Tue, 17 Sep 2013, Paulo Zanoni przan...@gmail.com wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
Sometimes I see the non asle set request?? message on my Haswell
machine, so I decided to get the spec and see if some bits are missing
from the mask. We do have some bits missing from the mask, so this
patch adds them, and the corresponding code to print unsupported
messages just like we do with the other bits we don't support.
But I still see the non asle set request?? message on my machine :(
Also use the proper ASLC name to indicate the registers we're talking
about.
v2: - Properly set the new FAILED bits
- Rename the old FAILED bits
- Print everything we don't support
Reviewed-by: Jani Nikula jani.nik...@intel.com
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_opregion.c | 153
+++---
1 file changed, 121 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index c4fb2ae..b627202 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -110,25 +110,38 @@ struct opregion_asle {
u32 epfm; /* enabled panel fitting modes */
u8 plut[74];/* panel LUT and identifier */
u32 pfmb; /* PWM freq and min brightness */
- u8 rsvd[102];
+ u32 cddv; /* color correction default values */
+ u32 pcft; /* power conservation features */
+ u32 srot; /* supported rotation angles */
+ u32 iuer; /* IUER events */
+ u8 rsvd[86];
} __attribute__((packed));
/* Driver readiness indicator */
#define ASLE_ARDY_READY (1 0)
#define ASLE_ARDY_NOT_READY (0 0)
-/* ASLE irq request bits */
-#define ASLE_SET_ALS_ILLUM (1 0)
-#define ASLE_SET_BACKLIGHT (1 1)
-#define ASLE_SET_PFIT (1 2)
-#define ASLE_SET_PWM_FREQ (1 3)
-#define ASLE_REQ_MSK 0xf
-
-/* response bits of ASLE irq request */
-#define ASLE_ALS_ILLUM_FAILED(110)
-#define ASLE_BACKLIGHT_FAILED(112)
-#define ASLE_PFIT_FAILED (114)
-#define ASLE_PWM_FREQ_FAILED (116)
+/* ASLE Interrupt Command (ASLC) bits */
+#define ASLC_SET_ALS_ILLUM (1 0)
+#define ASLC_SET_BACKLIGHT (1 1)
+#define ASLC_SET_PFIT(1 2)
+#define ASLC_SET_PWM_FREQ(1 3)
+#define ASLC_SUPPORTED_ROTATION_ANGLES (1 4)
+#define ASLC_BUTTON_ARRAY(1 5)
+#define ASLC_CONVERTIBLE_INDICATOR (1 6)
+#define ASLC_DOCKING_INDICATOR (1 7)
+#define ASLC_ISCT_STATE_CHANGE (1 8)
+#define ASLC_REQ_MSK 0x1ff
+/* response bits */
+#define ASLC_ALS_ILLUM_FAILED(1 10)
+#define ASLC_BACKLIGHT_FAILED(1 12)
+#define ASLC_PFIT_FAILED (1 14)
+#define ASLC_PWM_FREQ_FAILED (1 16)
+#define ASLC_ROTATION_ANGLES_FAILED (1 18)
+#define ASLC_BUTTON_ARRAY_FAILED (1 20)
+#define ASLC_CONVERTIBLE_FAILED (1 22)
+#define ASLC_DOCKING_FAILED (1 24)
+#define ASLC_ISCT_STATE_FAILED (1 26)
/* Technology enabled indicator */
#define ASLE_TCHE_ALS_EN (1 0)
@@ -154,6 +167,15 @@ struct opregion_asle {
#define ASLE_CBLV_VALID (131)
+/* IUER */
+#define ASLE_IUER_DOCKING(1 7)
+#define ASLE_IUER_CONVERTIBLE(1 6)
+#define ASLE_IUER_ROTATION_LOCK_BTN (1 4)
+#define ASLE_IUER_VOLUME_DOWN_BTN(1 3)
+#define ASLE_IUER_VOLUME_UP_BTN (1 2)
+#define ASLE_IUER_WINDOWS_BTN(1 1)
+#define ASLE_IUER_POWER_BTN (1 0)
+
/* Software System Control Interrupt (SWSCI) */
#define SWSCI_SCIC_INDICATOR (1 0)
#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1
@@ -377,11 +399,11 @@ static u32 asle_set_backlight(struct drm_device *dev,
u32 bclp)
DRM_DEBUG_DRIVER(bclp = 0x%08x\n, bclp);
if (!(bclp ASLE_BCLP_VALID))
- return ASLE_BACKLIGHT_FAILED;
+ return ASLC_BACKLIGHT_FAILED;
bclp = ASLE_BCLP_MSK;
if (bclp 255)
- return ASLE_BACKLIGHT_FAILED;
+ return ASLC_BACKLIGHT_FAILED;
intel_panel_set_backlight(dev, bclp, 255);
iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, asle-cblv);
@@ -394,13 +416,13 @@ static u32 asle_set_als_illum(struct drm_device *dev,
u32 alsi)
/* alsi is the current ALS reading in lux. 0 indicates below sensor
range, 0x indicates above sensor range. 1-0xfffe are valid */
DRM_DEBUG_DRIVER(Illum is not supported\n);
- return ASLE_ALS_ILLUM_FAILED;
+ return ASLC_ALS_ILLUM_FAILED;
}
static u32 asle_set_pwm_freq(struct drm_device *dev, u32 pfmb)
{
DRM_DEBUG_DRIVER(PWM freq is not supported\n);
- return ASLE_PWM_FREQ_FAILED;
+
Re: [Intel-gfx] BUG: sleeping function called from invalid context on 3.10.10-rt7
On Tue, Sep 17, 2013 at 9:50 PM, Peter Hurley pe...@hurleysoftware.com wrote: On 09/11/2013 03:31 PM, Peter Hurley wrote: [+cc dri-devel] On 09/11/2013 11:38 AM, Steven Rostedt wrote: On Wed, 11 Sep 2013 11:16:43 -0400 Peter Hurley pe...@hurleysoftware.com wrote: The funny part is, there's a comment there that shows that this was done even for PREEMPT_RT. Unfortunately, the call to get_scanout_position() can call functions that use the rt-mutex sleeping spin locks and it breaks there. I guess we need to ask the authors of the mainline patch exactly why that preempt_disable() is needed? The drm core associates a timestamp with each vertical blank frame #. Drm drivers can optionally support a 'high resolution' hw timestamp. The vblank frame #/timestamp tuple is user-space visible. The i915 drm driver supports a hw timestamp via this drm helper function which computes the timestamp from the crtc scan position (based on the pixel clock). For mainline, the preempt_disable/_enable() isn't actually necessary because every call tree that leads here already has preemption disabled. For -RT, the maybe i915 register spinlock (uncore.lock) should be raw? No, it should not. Note, any other lock that can be held when it is held would also need to be raw. By that, you mean any other lock that might be claimed would also need to be raw? Hopefully not any other lock already held? And by taking a quick audit of the code, I see this: spin_lock_irqsave(dev_priv-uncore.lock, irqflags); /* Reset the chip */ /* GEN6_GDRST is not in the gt power well, no need to check * for fifo space for the write or forcewake the chip for * the read */ __raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL); /* Spin waiting for the device to ack the reset request */ ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) GEN6_GRDOM_FULL) == 0, 500); That spin is unacceptable in RT with preemption and interrupts disabled. Yep. That would be bad. AFAICT the registers read in i915_get_crtc_scanoutpos() aren't included in the force-wake set, so raw reads of the registers would probably be acceptable (thus obviating the need for claiming the uncore.lock). Except that _ALL_ register access is disabled with the uncore.lock during a gpu reset. Not sure if that's meant to include crtc registers or not, or what other synchronization/serialization issues are being handled/hidden by forcing all register accesses to wait during a gpu reset. Hopefully an i915 expert can weigh in here? Daniel, Can you shed some light on whether the i915+ crtc registers (specifically those in i915_get_crtc_scanoutpos() and i915_/gm45_get_vblank_counter()) read as part of the vblank counter/timestamp handling need to be prevented during gpu reset? The depency here in the locking is a recent addition: commit a7cd1b8fea2f341b626b255d9898a5ca5fabbf0a Author: Chris Wilson ch...@chris-wilson.co.uk Date: Fri Jul 19 20:36:51 2013 +0100 drm/i915: Serialize almost all register access It's a (slightly) oversized hammer to work around a hardware issue - we could break it down to register blocks, which can be accessed concurrently, but that tends to be more fragile. But the chip really dies if you access (even just reads) the same block concurrently :( We could try break the spinlock protected section a bit in the reset handler - register access on a hung gpu tends to be ill-defined anyway. The implied wait with preemption and interrupts disabled is causing grief in -RT, but also a 4ms wait inside an irq handler seems like a bad idea. Oops, the magic code in wait_for which is just there to make the imo totally misguided kgdb support work papered over the aweful long wait in atomic context ever since we've added this in commit b6e45f866465f42b53d803b0c574da0fc508a0e9 Author: Keith Packard kei...@keithp.com Date: Fri Jan 6 11:34:04 2012 -0800 drm/i915: Move reset forcewake processing to gen6_do_reset Reverting this change should be enough (code moved obviously a bit). Cheers, Daniel Regards, Peter Hurley What's the real issue here? That the vblank timestamp needs to be an accurate measurement of a realtime event. Sleeping/servicing interrupts while reading the registers necessary to compute the timestamp would be bad too. (edit: which hopefully Mario Kleiner clarified in his reply) My point earlier was three-fold: 1. Don't need the preempt_disable() for mainline: all callers are already holding interrupt-disabling spinlocks. 2. -RT still needs to prevent scheduling there. 3. the problem is i915-specific. [update: the radeon driver should also BUG like the i915 driver but probably should have mmio_idx_lock spinlock as raw] -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list
Re: [Intel-gfx] [PATCH 5/6] [v3] drm/i915: Use the new vm [un]bind functions
On Tue, Sep 17, 2013 at 10:01:33AM -0700, Ben Widawsky wrote:
@@ -1117,8 +1109,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
* batch bit. Hence we need to pin secure batches into the global gtt.
* hsw should have this fixed, but let's be paranoid and do it
* unconditionally for now. */
- if (flags I915_DISPATCH_SECURE !batch_obj-has_global_gtt_mapping)
- i915_gem_gtt_bind_object(batch_obj, batch_obj-cache_level);
+ if (flags I915_DISPATCH_SECURE
+ !batch_obj-has_global_gtt_mapping) {
+ const struct i915_address_space *ggtt = obj_to_ggtt(batch_obj);
+ struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
+ BUG_ON(!vma);
+ ggtt-bind_vma(vma, batch_obj-cache_level, GLOBAL_BIND);
+ }
The issue here is that if we don't set the USE_PPGTT/USE_SECURE flag in
the dispatch, the CS will use the GGTT (hence our binding) but so we
then need to use the GGTT offset for the dispatch as well.
Is that as concisely as we can write bind_to_ggtt? :(
-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] [RFC][PATCH] drm/i915: Fix VGA handling using stop_machine() or mmio
From: Ville Syrjälä ville.syrj...@linux.intel.com
We have several problems with out VGA handling:
- We try to use the GMCH control VGA disable bit even though it may
be locked
- If we manage to disable VGA throuh GMCH control, we're no longer
able to correctly disable the VGA plane
- Taking part in the VGA arbitration is too expensive for X [1]
So let's treat the GMCH control VGA disable bit as read-only and leave
it for the BIOS to set, as it was intended. To disable VGA we will use
the VGA misc register, and to disable VGA IO we will disable IO space
completely via the PCI command register.
But we still need VGA register access during resume (and possibly during
lid event on insane BIOSen) to disable the VGA plane. Also we need to
re-disable VGA memory decode via the VGA misc register on resume.
Luckily up to gen4, VGA registers can be accessed through MMIO.
Unfortunately from gen5 onwards only the legacy VGA IO port range
works. So on gen5+ we still need IO space to be enabled during those
few special moments when we need to access VGA registers.
We still want to opt out of VGA arbitration on gen5+, so we have keep
IO space disabled most of the time. And when we do need to poke at VGA
registers, we enable IO space briefly while no one is looking. To
guarantee that no one is looking we will use stop_machine().
[1] http://lists.x.org/archives/xorg-devel/2013-September/037763.html
Cc: Alex Williamson alex.william...@redhat.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Dave Airlie airl...@redhat.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 37 +-
drivers/gpu/drm/i915/i915_suspend.c | 4 +-
drivers/gpu/drm/i915/intel_display.c | 247 +--
drivers/gpu/drm/i915/intel_drv.h | 1 -
4 files changed, 215 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index be5120f7..0fd86c2 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1225,19 +1225,6 @@ intel_teardown_mchbar(struct drm_device *dev)
release_resource(dev_priv-mch_res);
}
-/* true = enable decode, false = disable decoder */
-static unsigned int i915_vga_set_decode(void *cookie, bool state)
-{
- struct drm_device *dev = cookie;
-
- intel_modeset_vga_set_state(dev, state);
- if (state)
- return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM |
- VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
- else
- return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
-}
-
static void i915_switcheroo_set_state(struct pci_dev *pdev, enum
vga_switcheroo_state state)
{
struct drm_device *dev = pci_get_drvdata(pdev);
@@ -1283,25 +1270,11 @@ static int i915_load_modeset_init(struct drm_device
*dev)
if (ret)
DRM_INFO(failed to find VBIOS tables\n);
- /* If we have 1 VGA cards, then we need to arbitrate access
-* to the common VGA resources.
-*
-* If we are a secondary display controller (!PCI_DISPLAY_CLASS_VGA),
-* then we do not take part in VGA arbitration and the
-* vga_client_register() fails with -ENODEV.
-*/
- if (!HAS_PCH_SPLIT(dev)) {
- ret = vga_client_register(dev-pdev, dev, NULL,
- i915_vga_set_decode);
- if (ret ret != -ENODEV)
- goto out;
- }
-
intel_register_dsm_handler();
ret = vga_switcheroo_register_client(dev-pdev, i915_switcheroo_ops,
false);
if (ret)
- goto cleanup_vga_client;
+ goto out;
/* Initialise stolen first so that we may reserve preallocated
* objects for the BIOS to KMS transition.
@@ -1352,10 +1325,13 @@ static int i915_load_modeset_init(struct drm_device
*dev)
intel_fbdev_initial_config(dev);
/*
+* Disable VGA IO and memory, and
+* tell the arbiter to ignore us.
+*
* Must do this after fbcon init so that
* vgacon_save_screen() works during the handover.
*/
- i915_disable_vga_mem(dev);
+ intel_modeset_vga_set_state(dev, false);
/* Only enable hotplug handling once the fbdev is fully set up. */
dev_priv-enable_hotplug_processing = true;
@@ -1377,8 +1353,6 @@ cleanup_gem_stolen:
i915_gem_cleanup_stolen(dev);
cleanup_vga_switcheroo:
vga_switcheroo_unregister_client(dev-pdev);
-cleanup_vga_client:
- vga_client_register(dev-pdev, NULL, NULL, NULL);
out:
return ret;
}
@@ -1749,7 +1723,6 @@ int i915_driver_unload(struct drm_device *dev)
}
vga_switcheroo_unregister_client(dev-pdev);
- vga_client_register(dev-pdev, NULL, NULL, NULL);
}
/* Free error state after interrupts are fully disabled. */
diff --git
Re: [Intel-gfx] [PATCH 8/8] drm/i915: Do remaps for all contexts
On Fri, Sep 13, 2013 at 12:17:17PM +0300, Ville Syrjälä wrote:
On Thu, Sep 12, 2013 at 10:28:34PM -0700, Ben Widawsky wrote:
On both Ivybridge and Haswell, row remapping information is saved and
restored with context. This means, we never actually properly supported
the l3 remapping because our sysfs interface is asynchronous (and not
tied to any context), and the known faulty HW would be reused by the
next context to run.
Not that due to the asynchronous nature of the sysfs entry, there is no
point modifying the registers for the existing context. Instead we set a
flag for all contexts to load the correct remapping information on the
next run. Interested clients can use debugfs to determine whether or not
the row has been remapped.
One could propose at this point that we just do the remapping in the
kernel. I guess since we have to maintain the sysfs interface anyway,
I'm not sure how useful it is, and I do like keeping the policy in
userspace; (it wasn't my original decision to make the
interface the way it is, so I'm not attached).
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_debugfs.c | 8 +++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 17 +--
drivers/gpu/drm/i915/i915_sysfs.c | 38
+++--
4 files changed, 36 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index ada0950..80bed69 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -145,6 +145,13 @@ describe_obj(struct seq_file *m, struct
drm_i915_gem_object *obj)
seq_printf(m, (%s), obj-ring-name);
}
+static void describe_ctx(struct seq_file *m, struct i915_hw_context *ctx)
+{
+ seq_putc(m, ctx-is_initialized ? 'I' : 'i');
+ seq_putc(m, ctx-remap_slice ? 'R' : 'r');
+ seq_putc(m, ' ');
+}
+
static int i915_gem_object_list_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m-private;
@@ -1463,6 +1470,7 @@ static int i915_context_status(struct seq_file *m,
void *unused)
list_for_each_entry(ctx, dev_priv-context_list, link) {
seq_puts(m, HW context );
+ describe_ctx(m, ctx);
for_each_ring(ring, dev_priv, i)
if (ring-default_context == ctx)
seq_printf(m, (default context %s) ,
ring-name);
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 68f992b..6ba78cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -602,6 +602,7 @@ struct i915_hw_context {
struct kref ref;
int id;
bool is_initialized;
+ uint8_t remap_slice;
struct drm_i915_file_private *file_priv;
struct intel_ring_buffer *ring;
struct drm_i915_gem_object *obj;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 2bbdce8..72b7202 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -140,7 +140,7 @@ create_hw_context(struct drm_device *dev,
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct i915_hw_context *ctx;
- int ret;
+ int ret, i;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (ctx == NULL)
@@ -181,6 +181,8 @@ create_hw_context(struct drm_device *dev,
ctx-file_priv = file_priv;
ctx-id = ret;
+ for (i = 0; i NUM_L3_SLICES(dev); i++)
+ ctx-remap_slice |= 1 1;
^
i
return ctx;
@@ -396,7 +398,7 @@ static int do_switch(struct i915_hw_context *to)
struct intel_ring_buffer *ring = to-ring;
struct i915_hw_context *from = ring-last_context;
u32 hw_flags = 0;
- int ret;
+ int ret, i;
BUG_ON(from != NULL from-obj != NULL from-obj-pin_count == 0);
@@ -432,6 +434,17 @@ static int do_switch(struct i915_hw_context *to)
return ret;
}
+ for (i = 0; i MAX_L3_SLICES; i++) {
+ if (!(to-remap_slice (1i)))
+ continue;
+
+ ret = i915_gem_l3_remap(ring, i);
+ if (!ret) {
+ to-remap_slice = ~(1i);
+ }
+ /* If it failed, try again next round */
+ }
+
/* The backing object for the context is done after switching to the
* *next* context. Therefore we cannot retire the previous context until
* the next context has already started running. In fact, the below code
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 65a7274..f0d7e1c 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++
Re: [Intel-gfx] [PATCH 5/6] [v3] drm/i915: Use the new vm [un]bind functions
On Tue, Sep 17, 2013 at 09:55:35PM +0100, Chris Wilson wrote:
On Tue, Sep 17, 2013 at 10:01:33AM -0700, Ben Widawsky wrote:
@@ -1117,8 +1109,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
* batch bit. Hence we need to pin secure batches into the global gtt.
* hsw should have this fixed, but let's be paranoid and do it
* unconditionally for now. */
- if (flags I915_DISPATCH_SECURE !batch_obj-has_global_gtt_mapping)
- i915_gem_gtt_bind_object(batch_obj, batch_obj-cache_level);
+ if (flags I915_DISPATCH_SECURE
+ !batch_obj-has_global_gtt_mapping) {
+ const struct i915_address_space *ggtt = obj_to_ggtt(batch_obj);
+ struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
+ BUG_ON(!vma);
+ ggtt-bind_vma(vma, batch_obj-cache_level, GLOBAL_BIND);
+ }
The issue here is that if we don't set the USE_PPGTT/USE_SECURE flag in
the dispatch, the CS will use the GGTT (hence our binding) but so we
then need to use the GGTT offset for the dispatch as well.
Is that as concisely as we can write bind_to_ggtt? :(
-Chris
Resuming the conversation started on irc... what do you want from me?
--
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 5/6] [v3] drm/i915: Use the new vm [un]bind functions
On Tue, Sep 17, 2013 at 04:14:43PM -0700, Ben Widawsky wrote:
On Tue, Sep 17, 2013 at 09:55:35PM +0100, Chris Wilson wrote:
On Tue, Sep 17, 2013 at 10:01:33AM -0700, Ben Widawsky wrote:
@@ -1117,8 +1109,13 @@ i915_gem_do_execbuffer(struct drm_device *dev,
void *data,
* batch bit. Hence we need to pin secure batches into the global gtt.
* hsw should have this fixed, but let's be paranoid and do it
* unconditionally for now. */
- if (flags I915_DISPATCH_SECURE !batch_obj-has_global_gtt_mapping)
- i915_gem_gtt_bind_object(batch_obj, batch_obj-cache_level);
+ if (flags I915_DISPATCH_SECURE
+ !batch_obj-has_global_gtt_mapping) {
+ const struct i915_address_space *ggtt = obj_to_ggtt(batch_obj);
+ struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
+ BUG_ON(!vma);
+ ggtt-bind_vma(vma, batch_obj-cache_level, GLOBAL_BIND);
+ }
The issue here is that if we don't set the USE_PPGTT/USE_SECURE flag in
the dispatch, the CS will use the GGTT (hence our binding) but so we
then need to use the GGTT offset for the dispatch as well.
Is that as concisely as we can write bind_to_ggtt? :(
-Chris
Resuming the conversation started on irc... what do you want from me?
I think we need to pass the ggtt offset to dispatch for
I915_DISPATCH_SECURE -- which offset to use might even depend upon the
implementation and hw generation in intel_ringbuffer.c. But at the very
least, I think SNB/IVB will be executing the wrong address come full
ppgtt.
dev_priv-ggtt.base.bind_vma(i915_gem_obj_to_ggtt(batch_obj),
batch_obj-cache_level,
GLOBAL_BIND);
#define i915_vm_bind(vm__, vma__, cache_level__, flags__) \
(vm__)-bind_vma((vma__), (cache_level__), (flags__))
i915_vm_bind(dev_priv-ggtt.base,
i915_gem_obj_to_ggtt(batch_obj),
batch_obj-cache_level,
GLOBAL_BIND);
-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 10/16] intel_l3_parity: Assert all GEN7+ support
On Mon, Sep 16, 2013 at 06:18:28PM +, Bell, Bryan J wrote:
L3 dynamic parity is not supported on VLV. Please add the check for VLV.
I can send you the email thread, if needed.
--Thanks
Bryan
More importantly, we need to fix this in the kernel too. Thanks for
catching this.
-Original Message-
From: Ben Widawsky [mailto:benjamin.widaw...@intel.com]
Sent: Thursday, September 12, 2013 10:29 PM
To: intel-gfx@lists.freedesktop.org
Cc: Venkatesh, Vishnu; Bell, Bryan J; Widawsky, Benjamin; Ben Widawsky
Subject: [PATCH 10/16] intel_l3_parity: Assert all GEN7+ support
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/intel_l3_parity.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c index
970dcd6..a3d268b 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -120,7 +120,7 @@ int main(int argc, char *argv[])
assert(ret != -1);
fd = open(path, O_RDWR);
- if (fd == -1 IS_IVYBRIDGE(devid)) {
+ if (fd == -1 intel_gen(devid) 6) {
perror(Opening sysfs);
exit(EXIT_FAILURE);
} else if (fd == -1)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 5/6] [v3] drm/i915: Use the new vm [un]bind functions
On Wed, Sep 18, 2013 at 12:57:20AM +0100, Chris Wilson wrote:
On Tue, Sep 17, 2013 at 04:48:50PM -0700, Ben Widawsky wrote:
On Wed, Sep 18, 2013 at 12:33:32AM +0100, Chris Wilson wrote:
On Tue, Sep 17, 2013 at 04:14:43PM -0700, Ben Widawsky wrote:
On Tue, Sep 17, 2013 at 09:55:35PM +0100, Chris Wilson wrote:
On Tue, Sep 17, 2013 at 10:01:33AM -0700, Ben Widawsky wrote:
@@ -1117,8 +1109,13 @@ i915_gem_do_execbuffer(struct drm_device
*dev, void *data,
* batch bit. Hence we need to pin secure batches into the
global gtt.
* hsw should have this fixed, but let's be paranoid and do it
* unconditionally for now. */
- if (flags I915_DISPATCH_SECURE
!batch_obj-has_global_gtt_mapping)
- i915_gem_gtt_bind_object(batch_obj,
batch_obj-cache_level);
+ if (flags I915_DISPATCH_SECURE
+ !batch_obj-has_global_gtt_mapping) {
+ const struct i915_address_space *ggtt =
obj_to_ggtt(batch_obj);
+ struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
+ BUG_ON(!vma);
+ ggtt-bind_vma(vma, batch_obj-cache_level,
GLOBAL_BIND);
+ }
The issue here is that if we don't set the USE_PPGTT/USE_SECURE flag
in
the dispatch, the CS will use the GGTT (hence our binding) but so we
then need to use the GGTT offset for the dispatch as well.
Is that as concisely as we can write bind_to_ggtt? :(
-Chris
Resuming the conversation started on irc... what do you want from me?
I think we need to pass the ggtt offset to dispatch for
I915_DISPATCH_SECURE -- which offset to use might even depend upon the
implementation and hw generation in intel_ringbuffer.c. But at the very
least, I think SNB/IVB will be executing the wrong address come full
ppgtt.
dev_priv-ggtt.base.bind_vma(i915_gem_obj_to_ggtt(batch_obj),
batch_obj-cache_level,
GLOBAL_BIND);
#define i915_vm_bind(vm__, vma__, cache_level__, flags__) \
(vm__)-bind_vma((vma__), (cache_level__), (flags__))
i915_vm_bind(dev_priv-ggtt.base,
i915_gem_obj_to_ggtt(batch_obj),
batch_obj-cache_level,
GLOBAL_BIND);
-Chris
I915_DISPATCH_SECURE is a special case. If we see the flag, we look up
the GGTT offset as opposed to the offset in the VM being used at
execbuf. We can either bind the batchbuffer into both the PPGTT, and
GGTT, or it's only even in the GGTT - in either case, we'll have to have
done the bind (and found space in the drm_mm). It just seems like this
is duplicating the already existing i915_gem_object_bind_to_vm code
that's in place.
Sorry if I am not following what you're asking. I'm just failing to see
a problem, or maybe you're just trying to solve problems that I haven't
yet conceived; or solved in a different way. It's pretty darn hard to
discuss this given the piecemeal nature of the thing.
The code does
exec_start = i915_gem_obj_offset(batch_obj, vm) +
args-batch_start_offset;
exec_len = args-batch_len;
...
ret = ring-dispatch_execbuffer(ring,
exec_start, exec_len,
flags);
if (ret)
goto err;
So we lookup the address of the batch buffer in the wrong vm for
I915_DISPATCH_SECURE.
-Chris
But this is very easily solved, no?
http://cgit.freedesktop.org/~bwidawsk/drm-intel/tree/drivers/gpu/drm/i915/i915_gem_execbuffer.c?h=ppgtt#n1083
--
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 v2 0/3] Fix Win8 backlight issue
On 09/17/2013 09:34 PM, Igor Gnatenko wrote: On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Apply on top of v3.12-rc1. Aaron Lu (3): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists drivers/acpi/internal.h | 5 +- drivers/acpi/video.c| 442 drivers/acpi/video_detect.c | 14 +- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h| 2 + include/linux/backlight.h | 4 + 6 files changed, 300 insertions(+), 198 deletions(-) Aaron, how about fix indicator on ThinkPads ? Can you please describe the problem in detail, is it that when you adjust brightness level through hotkey, there is no GUI indication? Thanks. -Aaron ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in
drivers/gpu/drm/i915/i915_gem.c between commit 7dc19d5affd7 (drivers:
convert shrinkers to new count/scan API) from the tree and commit
e656a6cba0fe (drm/i915: inline vma_create into lookup_or_create_vma)
from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/i915_gem.c
index df9253d,d00d24f..000
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@@ -4886,61 -4912,3 +4940,37 @@@ unsigned long i915_gem_obj_size(struct
return 0;
}
+
+static unsigned long
+i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(shrinker,
+ struct drm_i915_private,
+ mm.inactive_shrinker);
+ struct drm_device *dev = dev_priv-dev;
+ int nr_to_scan = sc-nr_to_scan;
+ unsigned long freed;
+ bool unlock = true;
+
+ if (!mutex_trylock(dev-struct_mutex)) {
+ if (!mutex_is_locked_by(dev-struct_mutex, current))
+ return 0;
+
+ if (dev_priv-mm.shrinker_no_lock_stealing)
+ return 0;
+
+ unlock = false;
+ }
+
+ freed = i915_gem_purge(dev_priv, nr_to_scan);
+ if (freed nr_to_scan)
+ freed += __i915_gem_shrink(dev_priv, nr_to_scan,
+ false);
+ if (freed nr_to_scan)
+ freed += i915_gem_shrink_all(dev_priv);
+
+ if (unlock)
+ mutex_unlock(dev-struct_mutex);
+ return freed;
+}
-
- struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm)
- {
- struct i915_vma *vma;
- list_for_each_entry(vma, obj-vma_list, vma_link)
- if (vma-vm == vm)
- return vma;
-
- return NULL;
- }
-
- struct i915_vma *
- i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm)
- {
- struct i915_vma *vma;
-
- vma = i915_gem_obj_to_vma(obj, vm);
- if (!vma)
- vma = i915_gem_vma_create(obj, vm);
-
- return vma;
- }
pgpBQDmKDKrxr.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
Hi all, Today's linux-next merge of the drm-intel tree got a conflict in drivers/gpu/drm/i915/intel_drv.h between commit 6e1b4fdad515 (drm/i915: Delay disabling of VGA memory until vgacon-fbcon handoff is done) from Linus' tree and commit eb14cb747bc5 (drm/i915: Add state readout and checking for has_dp_encoder and dp_m_n) from the drm-intel tree. I fixed it up (see below) and can carry the fix as necessary (no action is required). -- Cheers, Stephen Rothwells...@canb.auug.org.au diff --cc drivers/gpu/drm/i915/intel_drv.h index 28cae80,b85354f..000 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@@ -793,6 -814,13 +815,14 @@@ extern void hsw_pc8_disable_interrupts( extern void hsw_pc8_restore_interrupts(struct drm_device *dev); extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); +extern void i915_disable_vga_mem(struct drm_device *dev); + extern void intel_dp_get_m_n(struct intel_crtc *crtc, +struct intel_crtc_config *pipe_config); + extern int intel_dotclock_calculate(int link_freq, + const struct intel_link_m_n *m_n); + extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config, + int dotclock); + + extern bool intel_crtc_active(struct drm_crtc *crtc); #endif /* __INTEL_DRV_H__ */ pgptaTfkGoWdd.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: Make l3 remapping use the ring
Using LRI for setting the remapping registers allows us to stream l3
remapping information. This is necessary to handle per context remaps as
we'll see implemented in an upcoming patch.
Using the ring also means we don't need to frob the DOP clock gating
bits.
v2: Add comment about lack of worry for concurrent register access
(Daniel)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 38 ++
drivers/gpu/drm/i915/i915_sysfs.c | 3 ++-
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6e8df7..0c39805 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1949,7 +1949,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object
*obj, bool force);
int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
int __must_check i915_gem_init(struct drm_device *dev);
int __must_check i915_gem_init_hw(struct drm_device *dev);
-void i915_gem_l3_remap(struct drm_device *dev, int slice);
+int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice);
void i915_gem_init_swizzling(struct drm_device *dev);
void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
int __must_check i915_gpu_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 66bf75d..2538138 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4252,35 +4252,33 @@ i915_gem_idle(struct drm_device *dev)
return 0;
}
-void i915_gem_l3_remap(struct drm_device *dev, int slice)
+int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice)
{
+ struct drm_device *dev = ring-dev;
drm_i915_private_t *dev_priv = dev-dev_private;
u32 reg_base = GEN7_L3LOG_BASE + (slice * 0x200);
u32 *remap_info = dev_priv-l3_parity.remap_info[slice];
- u32 misccpctl;
- int i;
+ int i, ret;
if (!HAS_L3_GPU_CACHE(dev) || !remap_info)
- return;
+ return 0;
- misccpctl = I915_READ(GEN7_MISCCPCTL);
- I915_WRITE(GEN7_MISCCPCTL, misccpctl ~GEN7_DOP_CLOCK_GATE_ENABLE);
- POSTING_READ(GEN7_MISCCPCTL);
+ ret = intel_ring_begin(ring, GEN7_L3LOG_SIZE / 4 * 3);
+ if (ret)
+ return ret;
+ /* XXX: We do not worry about the concurrent register cacheline hang
+* here because no other code should access these registers other than
+* at initialization time. */
for (i = 0; i GEN7_L3LOG_SIZE; i += 4) {
- u32 remap = I915_READ(reg_base + i);
- if (remap remap != remap_info[i/4])
- DRM_DEBUG(0x%x was already programmed to %x\n,
- reg_base + i, remap);
- if (remap !remap_info[i/4])
- DRM_DEBUG_DRIVER(Clearing remapped register\n);
- I915_WRITE(reg_base + i, remap_info[i/4]);
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, reg_base + i);
+ intel_ring_emit(ring, remap_info[i/4]);
}
- /* Make sure all the writes land before disabling dop clock gating */
- POSTING_READ(reg_base);
+ intel_ring_advance(ring);
- I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+ return ret;
}
void i915_gem_init_swizzling(struct drm_device *dev)
@@ -4391,15 +4389,15 @@ i915_gem_init_hw(struct drm_device *dev)
I915_WRITE(GEN7_MSG_CTL, temp);
}
- for (i = 0; i NUM_L3_SLICES(dev); i++)
- i915_gem_l3_remap(dev, i);
-
i915_gem_init_swizzling(dev);
ret = i915_gem_init_rings(dev);
if (ret)
return ret;
+ for (i = 0; i NUM_L3_SLICES(dev); i++)
+ i915_gem_l3_remap(dev_priv-ring[RCS], i);
+
/*
* XXX: There was some w/a described somewhere suggesting loading
* contexts before PPGTT.
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 3a8bf0c..b07bdfb 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -204,7 +204,8 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
memcpy(dev_priv-l3_parity.remap_info[slice] + (offset/4), buf, count);
- i915_gem_l3_remap(drm_dev, slice);
+ if (i915_gem_l3_remap(dev_priv-ring[RCS], slice))
+ count = 0;
mutex_unlock(drm_dev-struct_mutex);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Add second slice l3 remapping
Certain HSW SKUs have a second bank of L3. This L3 remapping has a
separate register set, and interrupt from the first slice. A slice is
simply a term to define some subset of the GPU's l3 cache. This patch
implements both the interrupt handler, and ability to communicate with
userspace about this second slice.
v2: Remove redundant check about non-existent slice.
Change warning about interrupts of unknown slices to WARN_ON_ONCE
Handle the case where we get 2 slice interrupts concurrently, and switch
the tracking of interrupts to be non-destructive (all Ville)
Don't enable/mask the second slice parity interrupt for ivb/vlv (even
though all docs I can find claim it's rsvd) (Ville + Bryan)
Keep BYT excluded from L3 parity
CC: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 7 ++-
drivers/gpu/drm/i915/i915_gem.c | 26 +-
drivers/gpu/drm/i915/i915_irq.c | 88 +
drivers/gpu/drm/i915/i915_reg.h | 7 +++
drivers/gpu/drm/i915/i915_sysfs.c | 34 ++---
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++-
include/uapi/drm/i915_drm.h | 8 +--
7 files changed, 116 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8b16d47..c6e8df7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -917,9 +917,11 @@ struct i915_ums_state {
int mm_suspended;
};
+#define MAX_L3_SLICES 2
struct intel_l3_parity {
- u32 *remap_info;
+ u32 *remap_info[MAX_L3_SLICES];
struct work_struct error_work;
+ int which_slice;
};
struct i915_gem_mm {
@@ -1686,6 +1688,7 @@ struct drm_i915_file_private {
#define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)-has_force_wake)
#define HAS_L3_GPU_CACHE(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
+#define NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_GPU_CACHE(dev))
#define GT_FREQUENCY_MULTIPLIER 50
@@ -1946,7 +1949,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object
*obj, bool force);
int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
int __must_check i915_gem_init(struct drm_device *dev);
int __must_check i915_gem_init_hw(struct drm_device *dev);
-void i915_gem_l3_remap(struct drm_device *dev);
+void i915_gem_l3_remap(struct drm_device *dev, int slice);
void i915_gem_init_swizzling(struct drm_device *dev);
void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
int __must_check i915_gpu_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3d3de6e..66bf75d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4252,16 +4252,15 @@ i915_gem_idle(struct drm_device *dev)
return 0;
}
-void i915_gem_l3_remap(struct drm_device *dev)
+void i915_gem_l3_remap(struct drm_device *dev, int slice)
{
drm_i915_private_t *dev_priv = dev-dev_private;
+ u32 reg_base = GEN7_L3LOG_BASE + (slice * 0x200);
+ u32 *remap_info = dev_priv-l3_parity.remap_info[slice];
u32 misccpctl;
int i;
- if (!HAS_L3_GPU_CACHE(dev))
- return;
-
- if (!dev_priv-l3_parity.remap_info)
+ if (!HAS_L3_GPU_CACHE(dev) || !remap_info)
return;
misccpctl = I915_READ(GEN7_MISCCPCTL);
@@ -4269,17 +4268,17 @@ void i915_gem_l3_remap(struct drm_device *dev)
POSTING_READ(GEN7_MISCCPCTL);
for (i = 0; i GEN7_L3LOG_SIZE; i += 4) {
- u32 remap = I915_READ(GEN7_L3LOG_BASE + i);
- if (remap remap != dev_priv-l3_parity.remap_info[i/4])
+ u32 remap = I915_READ(reg_base + i);
+ if (remap remap != remap_info[i/4])
DRM_DEBUG(0x%x was already programmed to %x\n,
- GEN7_L3LOG_BASE + i, remap);
- if (remap !dev_priv-l3_parity.remap_info[i/4])
+ reg_base + i, remap);
+ if (remap !remap_info[i/4])
DRM_DEBUG_DRIVER(Clearing remapped register\n);
- I915_WRITE(GEN7_L3LOG_BASE + i,
dev_priv-l3_parity.remap_info[i/4]);
+ I915_WRITE(reg_base + i, remap_info[i/4]);
}
/* Make sure all the writes land before disabling dop clock gating */
- POSTING_READ(GEN7_L3LOG_BASE);
+ POSTING_READ(reg_base);
I915_WRITE(GEN7_MISCCPCTL, misccpctl);
}
@@ -4373,7 +4372,7 @@ int
i915_gem_init_hw(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev-dev_private;
- int ret;
+ int ret, i;
if (INTEL_INFO(dev)-gen 6 !intel_enable_gtt())
return -EIO;
@@ -4392,7 +4391,8 @@ i915_gem_init_hw(struct drm_device *dev)
I915_WRITE(GEN7_MSG_CTL, temp);
}
- i915_gem_l3_remap(dev);
+
[Intel-gfx] [PATCH 4/6] drm/i915: Keep a list of all contexts
I have implemented this patch before without creating a separate list
(I'm having trouble finding the links, but the messages ids are:
1364942743-6041-2-git-send-email-...@bwidawsk.net
1365118914-15753-9-git-send-email-...@bwidawsk.net)
However, the code is much simpler to just use a list and it makes the
code from the next patch a lot more pretty.
As you'll see in the next patch, the reason for this is to be able to
specify when a context needs to get L3 remapping. More details there.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_debugfs.c | 15 +--
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_gem.c | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 1d77624..ada0950 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1442,6 +1442,7 @@ static int i915_context_status(struct seq_file *m, void
*unused)
struct drm_device *dev = node-minor-dev;
drm_i915_private_t *dev_priv = dev-dev_private;
struct intel_ring_buffer *ring;
+ struct i915_hw_context *ctx;
int ret, i;
ret = mutex_lock_interruptible(dev-mode_config.mutex);
@@ -1460,12 +1461,14 @@ static int i915_context_status(struct seq_file *m, void
*unused)
seq_putc(m, '\n');
}
- for_each_ring(ring, dev_priv, i) {
- if (ring-default_context) {
- seq_printf(m, HW default context %s , ring-name);
- describe_obj(m, ring-default_context-obj);
- seq_putc(m, '\n');
- }
+ list_for_each_entry(ctx, dev_priv-context_list, link) {
+ seq_puts(m, HW context );
+ for_each_ring(ring, dev_priv, i)
+ if (ring-default_context == ctx)
+ seq_printf(m, (default context %s) ,
ring-name);
+
+ describe_obj(m, ctx-obj);
+ seq_putc(m, '\n');
}
mutex_unlock(dev-mode_config.mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0c39805..1795927 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -605,6 +605,8 @@ struct i915_hw_context {
struct intel_ring_buffer *ring;
struct drm_i915_gem_object *obj;
struct i915_ctx_hang_stats hang_stats;
+
+ struct list_head link;
};
struct i915_fbc {
@@ -1343,6 +1345,7 @@ typedef struct drm_i915_private {
bool hw_contexts_disabled;
uint32_t hw_context_size;
+ struct list_head context_list;
u32 fdi_rx_config;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2538138..18d07d7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4569,6 +4569,7 @@ i915_gem_load(struct drm_device *dev)
INIT_LIST_HEAD(dev_priv-vm_list);
i915_init_vm(dev_priv, dev_priv-gtt.base);
+ INIT_LIST_HEAD(dev_priv-context_list);
INIT_LIST_HEAD(dev_priv-mm.unbound_list);
INIT_LIST_HEAD(dev_priv-mm.bound_list);
INIT_LIST_HEAD(dev_priv-mm.fence_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 26c3fcc..2bbdce8 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -129,6 +129,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
struct i915_hw_context *ctx = container_of(ctx_ref,
typeof(*ctx), ref);
+ list_del(ctx-link);
drm_gem_object_unreference(ctx-obj-base);
kfree(ctx);
}
@@ -147,6 +148,7 @@ create_hw_context(struct drm_device *dev,
kref_init(ctx-ref);
ctx-obj = i915_gem_alloc_object(dev, dev_priv-hw_context_size);
+ INIT_LIST_HEAD(ctx-link);
if (ctx-obj == NULL) {
kfree(ctx);
DRM_DEBUG_DRIVER(Context object allocated failed\n);
@@ -166,6 +168,7 @@ create_hw_context(struct drm_device *dev,
* assertion in the context switch code.
*/
ctx-ring = dev_priv-ring[RCS];
+ list_add_tail(ctx-link, dev_priv-context_list);
/* Default context will never have a file_priv */
if (file_priv == NULL)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915: Do remaps for all contexts
On both Ivybridge and Haswell, row remapping information is saved and
restored with context. This means, we never actually properly supported
the l3 remapping because our sysfs interface is asynchronous (and not
tied to any context), and the known faulty HW would be reused by the
next context to run.
Not that due to the asynchronous nature of the sysfs entry, there is no
point modifying the registers for the existing context. Instead we set a
flag for all contexts to load the correct remapping information on the
next run. Interested clients can use debugfs to determine whether or not
the row has been remapped.
One could propose at this point that we just do the remapping in the
kernel. I guess since we have to maintain the sysfs interface anyway,
I'm not sure how useful it is, and I do like keeping the policy in
userspace; (it wasn't my original decision to make the
interface the way it is, so I'm not attached).
v2: Force a context switch when we have a remap on the next switch.
(Ville)
Don't let userspace use the interface with disabled contexts.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_debugfs.c | 8 +++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 22
drivers/gpu/drm/i915/i915_sysfs.c | 37 +
4 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index ada0950..80bed69 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -145,6 +145,13 @@ describe_obj(struct seq_file *m, struct
drm_i915_gem_object *obj)
seq_printf(m, (%s), obj-ring-name);
}
+static void describe_ctx(struct seq_file *m, struct i915_hw_context *ctx)
+{
+ seq_putc(m, ctx-is_initialized ? 'I' : 'i');
+ seq_putc(m, ctx-remap_slice ? 'R' : 'r');
+ seq_putc(m, ' ');
+}
+
static int i915_gem_object_list_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m-private;
@@ -1463,6 +1470,7 @@ static int i915_context_status(struct seq_file *m, void
*unused)
list_for_each_entry(ctx, dev_priv-context_list, link) {
seq_puts(m, HW context );
+ describe_ctx(m, ctx);
for_each_ring(ring, dev_priv, i)
if (ring-default_context == ctx)
seq_printf(m, (default context %s) ,
ring-name);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1795927..015df52 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -601,6 +601,7 @@ struct i915_hw_context {
struct kref ref;
int id;
bool is_initialized;
+ uint8_t remap_slice;
struct drm_i915_file_private *file_priv;
struct intel_ring_buffer *ring;
struct drm_i915_gem_object *obj;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 2bbdce8..7e138cc 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -140,7 +140,7 @@ create_hw_context(struct drm_device *dev,
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct i915_hw_context *ctx;
- int ret;
+ int ret, i;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (ctx == NULL)
@@ -181,6 +181,8 @@ create_hw_context(struct drm_device *dev,
ctx-file_priv = file_priv;
ctx-id = ret;
+ for (i = 0; i NUM_L3_SLICES(dev); i++)
+ ctx-remap_slice |= 1 1;
return ctx;
@@ -396,11 +398,11 @@ static int do_switch(struct i915_hw_context *to)
struct intel_ring_buffer *ring = to-ring;
struct i915_hw_context *from = ring-last_context;
u32 hw_flags = 0;
- int ret;
+ int ret, i;
BUG_ON(from != NULL from-obj != NULL from-obj-pin_count == 0);
- if (from == to)
+ if (from == to !to-remap_slice)
return 0;
ret = i915_gem_obj_ggtt_pin(to-obj, CONTEXT_ALIGN, false, false);
@@ -423,7 +425,7 @@ static int do_switch(struct i915_hw_context *to)
if (!to-is_initialized || is_default_context(to))
hw_flags |= MI_RESTORE_INHIBIT;
- else if (WARN_ON_ONCE(from == to)) /* not yet expected */
+ else if (from == to)
hw_flags |= MI_FORCE_RESTORE;
ret = mi_set_context(ring, to, hw_flags);
@@ -432,6 +434,18 @@ static int do_switch(struct i915_hw_context *to)
return ret;
}
+ for (i = 0; i MAX_L3_SLICES; i++) {
+ if (!(to-remap_slice (1i)))
+ continue;
+
+ ret = i915_gem_l3_remap(ring, i);
+ if (!ret) {
+ to-remap_slice = ~(1i);
+ /* If it failed, try
[Intel-gfx] [PATCH 07/14] intel_l3_parity: Fix indentation
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/intel_l3_parity.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index ad027ac..970dcd6 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -58,12 +58,12 @@ static void dumpit(void)
for (j = 0; j NUM_SUBBANKS; j++) {
struct l3_log_register *reg = l3log[i][j];
- if (reg-row0_enable)
- printf(Row %d, Bank %d, Subbank %d is disabled\n,
- reg-row0, i, j);
- if (reg-row1_enable)
- printf(Row %d, Bank %d, Subbank %d is disabled\n,
- reg-row1, i, j);
+ if (reg-row0_enable)
+ printf(Row %d, Bank %d, Subbank %d is
disabled\n,
+ reg-row0, i, j);
+ if (reg-row1_enable)
+ printf(Row %d, Bank %d, Subbank %d is
disabled\n,
+ reg-row1, i, j);
}
}
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/14] intel_l3_parity: Assert all GEN7+ support
v2: Don't assert for Valleyview (Bryan)
Rework code to be a bit more readable.
CC: Bell, Bryan J bryan.j.b...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/intel_l3_parity.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index 970dcd6..715d095 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -116,15 +116,14 @@ int main(int argc, char *argv[])
drm_fd = drm_open_any();
devid = intel_get_drm_devid(drm_fd);
+ if (intel_gen(devid) 7 || IS_VALLEYVIEW(devid))
+ exit(EXIT_SUCCESS);
+
ret = asprintf(path, /sys/class/drm/card%d/l3_parity, device);
assert(ret != -1);
fd = open(path, O_RDWR);
- if (fd == -1 IS_IVYBRIDGE(devid)) {
- perror(Opening sysfs);
- exit(EXIT_FAILURE);
- } else if (fd == -1)
- exit(EXIT_SUCCESS);
+ assert(fd != -1);
ret = read(fd, l3log, NUM_REGS * sizeof(uint32_t));
if (ret == -1) {
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/14] intel_l3_parity: Use getopt for the l3 parity tool
Add new command line arguments in addition to supporting the old
features. This patch only introduces one feature, the -e argument to
enable a specific row/bank/subbank. Previously you could only enable
all. Otherwise, it has what you expect (we prefer -r -b -s for
specifying the row/bank/subbank).
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tests/sysfs_l3_parity | 12 ++---
tools/intel_l3_parity.c | 122
2 files changed, 109 insertions(+), 25 deletions(-)
diff --git a/tests/sysfs_l3_parity b/tests/sysfs_l3_parity
index 6f814a1..a0dfad9 100755
--- a/tests/sysfs_l3_parity
+++ b/tests/sysfs_l3_parity
@@ -8,20 +8,20 @@ fi
SOURCE_DIR=$( dirname ${BASH_SOURCE[0]} )
. $SOURCE_DIR/drm_lib.sh
-$SOURCE_DIR/../tools/intel_l3_parity -c
+$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
#Check that we can remap a row
-$SOURCE_DIR/../tools/intel_l3_parity 0,0,0
-disabled=`$SOURCE_DIR/../tools/intel_l3_parity | grep -c 'Row 0, Bank 0,
Subbank 0 is disabled'`
+$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -d
+disabled=`$SOURCE_DIR/../tools/intel_l3_parity -l | grep -c 'Row 0, Bank 0,
Subbank 0 is disabled'`
if [ $disabled != 1 ] ; then
echo Fail
exit 1
fi
-$SOURCE_DIR/../tools/intel_l3_parity -c
+$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
#Check that we can clear remaps
-if [ `$SOURCE_DIR/../tools/intel_l3_parity | wc -c` != 0 ] ; then
- echo Fail
+if [ `$SOURCE_DIR/../tools/intel_l3_parity -l | wc -c` != 0 ] ; then
+ echo Fail 2
exit 1
fi
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index 715d095..507a522 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -33,12 +33,14 @@
#include stdlib.h
#include string.h
#include unistd.h
+#include getopt.h
#include intel_chipset.h
#include intel_gpu_tools.h
#include drmtest.h
#define NUM_BANKS 4
#define NUM_SUBBANKS 8
+#define MAX_ROW (112)
#define NUM_REGS (NUM_BANKS * NUM_SUBBANKS)
struct __attribute__ ((__packed__)) l3_log_register {
@@ -93,17 +95,34 @@ static int disable_rbs(int row, int bank, int sbank)
return 0;
}
-static int do_parse(int argc, char *argv[])
+static void enables_rbs(int row, int bank, int sbank)
{
- int row, bank, sbank, i, ret;
+ struct l3_log_register *reg = l3log[bank][sbank];
- for (i = 1; i argc; i++) {
- ret = sscanf(argv[i], %d,%d,%d, row, bank, sbank);
- if (ret != 3)
- return i;
- assert(disable_rbs(row, bank, sbank) == 0);
- }
- return 0;
+ if (!reg-row0_enable !reg-row1_enable)
+ return;
+
+ if (reg-row1_enable reg-row1 == row)
+ reg-row1_enable = 0;
+ else if (reg-row0_enable reg-row0 == row)
+ reg-row0_enable = 0;
+}
+
+static void usage(const char *name)
+{
+ printf(usage: %s [OPTIONS] [ACTION]\n
+ Operate on the i915 L3 GPU cache (should be run as root)\n\n
+OPTIONS:\n
+ -r, --row=[row] The row to act upon
(default 0)\n
+ -b, --bank=[bank]The bank to act upon
(default 0)\n
+ -s, --subbank=[subbank] The subbank to act upon
(default 0)\n
+ACTIONS (only 1 may be specified at a time):\n
+ -h, --help Display this help\n
+ -l, --list List the current L3
logs\n
+ -a, --clear-all Clear all disabled
rows\n
+ -e, --enable Enable row, bank,
subbank (undo -d)\n
+ -d, --disable=row,bank,subbank Disable row, bank,
subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n,
+ name);
}
int main(int argc, char *argv[])
@@ -111,7 +130,9 @@ int main(int argc, char *argv[])
const int device = drm_get_card();
char *path;
unsigned int devid;
+ int row = 0, bank = 0, sbank = 0;
int drm_fd, fd, ret;
+ int action = '0';
drm_fd = drm_open_any();
devid = intel_get_drm_devid(drm_fd);
@@ -133,19 +154,82 @@ int main(int argc, char *argv[])
assert(lseek(fd, 0, SEEK_SET) == 0);
- if (argc == 1) {
- dumpit();
- exit(EXIT_SUCCESS);
- } else if (!strncmp(-c, argv[1], 2)) {
- memset(l3log, 0, sizeof(l3log));
- } else {
- ret = do_parse(argc, argv);
- if (ret != 0) {
- fprintf(stderr, Malformed command line at %s\n,
argv[ret]);
- exit(EXIT_FAILURE);
+ while (1) {
+ int c, option_index = 0;
+ static struct option long_options[] = {
+ { help, no_argument, 0, 'h' },
+ { list,
[Intel-gfx] [PATCH 11/14] intel_l3_parity: slice support
Haswell GT3 adds a new slice which is kept distinct from the old
register interface. Plumb it into the code, though it's only 1 slice
still.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/intel_l3_parity.c | 106 +---
1 file changed, 65 insertions(+), 41 deletions(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index 28c47eb..c98eb80 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -52,6 +52,8 @@ static unsigned int devid;
#define MAX_ROW (112)
#define L3_SIZE ((MAX_ROW * 4) * NUM_SUBBANKS * NUM_BANKS)
#define NUM_REGS (NUM_BANKS * NUM_SUBBANKS)
+#define MAX_SLICES 1
+#define REAL_MAX_SLICES 1
struct __attribute__ ((__packed__)) l3_log_register {
uint32_t row0_enable: 1;
@@ -60,15 +62,21 @@ struct __attribute__ ((__packed__)) l3_log_register {
uint32_t row1_enable: 1;
uint32_t rsvd1 : 4;
uint32_t row1 : 11;
-} l3log[MAX_BANKS][NUM_SUBBANKS];
+} l3logs[REAL_MAX_SLICES][MAX_BANKS][NUM_SUBBANKS];
-static void dumpit(void)
+static int which_slice = -1;
+#define for_each_slice(__i) \
+ for ((__i) = (which_slice == -1) ? 0 : which_slice; \
+ (__i) ((which_slice == -1) ? MAX_SLICES :
(which_slice + 1)); \
+ (__i)++)
+
+static void dumpit(int slice)
{
int i, j;
for (i = 0; i NUM_BANKS; i++) {
for (j = 0; j NUM_SUBBANKS; j++) {
- struct l3_log_register *reg = l3log[i][j];
+ struct l3_log_register *reg = l3logs[slice][i][j];
if (reg-row0_enable)
printf(Row %d, Bank %d, Subbank %d is
disabled\n,
@@ -80,9 +88,9 @@ static void dumpit(void)
}
}
-static int disable_rbs(int row, int bank, int sbank)
+static int disable_rbs(int row, int bank, int sbank, int slice)
{
- struct l3_log_register *reg = l3log[bank][sbank];
+ struct l3_log_register *reg = l3logs[slice][bank][sbank];
// can't map more than 2 rows
if (reg-row0_enable reg-row1_enable)
@@ -105,9 +113,9 @@ static int disable_rbs(int row, int bank, int sbank)
return 0;
}
-static void enables_rbs(int row, int bank, int sbank)
+static void enables_rbs(int row, int bank, int sbank, int slice)
{
- struct l3_log_register *reg = l3log[bank][sbank];
+ struct l3_log_register *reg = l3logs[slice][bank][sbank];
if (!reg-row0_enable !reg-row1_enable)
return;
@@ -126,6 +134,7 @@ static void usage(const char *name)
-r, --row=[row] The row to act upon
(default 0)\n
-b, --bank=[bank]The bank to act upon
(default 0)\n
-s, --subbank=[subbank] The subbank to act upon
(default 0)\n
+ -w, --slice=[slice] Which slice to act on
(default: -1 [all])
ACTIONS (only 1 may be specified at a time):\n
-h, --help Display this help\n
-H, --hw-infoDisplay the
current L3 properties\n
@@ -139,30 +148,30 @@ static void usage(const char *name)
int main(int argc, char *argv[])
{
const int device = drm_get_card();
- char *path;
+ char *path[REAL_MAX_SLICES];
int row = 0, bank = 0, sbank = 0;
- int drm_fd, fd, ret;
+ int fd[REAL_MAX_SLICES] = {0}, ret, i;
int action = '0';
-
- drm_fd = drm_open_any();
+ int drm_fd = drm_open_any();
devid = intel_get_drm_devid(drm_fd);
if (intel_gen(devid) 7 || IS_VALLEYVIEW(devid))
exit(EXIT_SUCCESS);
- ret = asprintf(path, /sys/class/drm/card%d/l3_parity, device);
+ ret = asprintf(path[0], /sys/class/drm/card%d/l3_parity, device);
assert(ret != -1);
- fd = open(path, O_RDWR);
- assert(fd != -1);
-
- ret = read(fd, l3log, NUM_REGS * sizeof(uint32_t));
- if (ret == -1) {
- perror(Reading sysfs);
- exit(EXIT_FAILURE);
+ for_each_slice(i) {
+ fd[i] = open(path[i], O_RDWR);
+ assert(fd[i]);
+ ret = read(fd[i], l3logs[i], NUM_REGS * sizeof(uint32_t));
+ if (ret == -1) {
+ perror(Reading sysfs);
+ exit(EXIT_FAILURE);
+ }
+ assert(lseek(fd[i], 0, SEEK_SET) == 0);
}
- assert(lseek(fd, 0, SEEK_SET) == 0);
while (1) {
int c, option_index = 0;
@@ -176,10 +185,11 @@ int main(int argc, char *argv[])
{ row, required_argument, 0, 'r' },
{ bank, required_argument, 0, 'b' },
{ subbank, required_argument, 0, 's' },
+ { slice, required_argument, 0, 'w' },
[Intel-gfx] [PATCH 10/14] intel_l3_parity: Hardware info argument
Add a new command line argument to the tool which will spit out various
parameters for the giving hardware. As a result of this, some new
defines are added to help with the various info.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/intel_l3_parity.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index 507a522..28c47eb 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -38,9 +38,19 @@
#include intel_gpu_tools.h
#include drmtest.h
-#define NUM_BANKS 4
+static unsigned int devid;
+/* L3 size is always a function of banks. The number of banks cannot be
+ * determined by number of slices however */
+#define MAX_BANKS 4
+#define NUM_BANKS \
+ ((devid == PCI_CHIP_IVYBRIDGE_GT1 || devid == PCI_CHIP_IVYBRIDGE_M_GT1)
? 2 : 4)
#define NUM_SUBBANKS 8
+#define BYTES_PER_BANK (128 10)
+/* Each row addresses [up to] 4b. This multiplied by the number of subbanks
+ * will give the L3 size per bank.
+ * TODO: Row size is fixed on IVB, and variable on HSW.*/
#define MAX_ROW (112)
+#define L3_SIZE ((MAX_ROW * 4) * NUM_SUBBANKS * NUM_BANKS)
#define NUM_REGS (NUM_BANKS * NUM_SUBBANKS)
struct __attribute__ ((__packed__)) l3_log_register {
@@ -50,7 +60,7 @@ struct __attribute__ ((__packed__)) l3_log_register {
uint32_t row1_enable: 1;
uint32_t rsvd1 : 4;
uint32_t row1 : 11;
-} l3log[NUM_BANKS][NUM_SUBBANKS];
+} l3log[MAX_BANKS][NUM_SUBBANKS];
static void dumpit(void)
{
@@ -118,6 +128,7 @@ static void usage(const char *name)
-s, --subbank=[subbank] The subbank to act upon
(default 0)\n
ACTIONS (only 1 may be specified at a time):\n
-h, --help Display this help\n
+ -H, --hw-infoDisplay the
current L3 properties\n
-l, --list List the current L3
logs\n
-a, --clear-all Clear all disabled
rows\n
-e, --enable Enable row, bank,
subbank (undo -d)\n
@@ -129,7 +140,6 @@ int main(int argc, char *argv[])
{
const int device = drm_get_card();
char *path;
- unsigned int devid;
int row = 0, bank = 0, sbank = 0;
int drm_fd, fd, ret;
int action = '0';
@@ -162,13 +172,14 @@ int main(int argc, char *argv[])
{ clear-all, no_argument, 0, 'a' },
{ enable, no_argument, 0, 'e' },
{ disable, optional_argument, 0, 'd' },
+ { hw-info, no_argument, 0, 'H' },
{ row, required_argument, 0, 'r' },
{ bank, required_argument, 0, 'b' },
{ subbank, required_argument, 0, 's' },
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, hr:b:s:aled::, long_options,
+ c = getopt_long(argc, argv, hHr:b:s:aled::, long_options,
option_index);
if (c == -1)
break;
@@ -178,6 +189,11 @@ int main(int argc, char *argv[])
case 'h':
usage(argv[0]);
exit(EXIT_SUCCESS);
+ case 'H':
+ printf(Number of banks: %d\n, NUM_BANKS);
+ printf(Subbanks per bank: %d\n, NUM_SUBBANKS);
+ printf(L3 size: %dK\n, L3_SIZE 10);
+ exit(EXIT_SUCCESS);
case 'r':
row = atoi(optarg);
if (row = MAX_ROW)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/14] intel_l3_parity: Actually support multiple slices
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/intel_l3_parity.c | 45 -
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index c98eb80..ed7034a 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -41,19 +41,28 @@
static unsigned int devid;
/* L3 size is always a function of banks. The number of banks cannot be
* determined by number of slices however */
-#define MAX_BANKS 4
-#define NUM_BANKS \
- ((devid == PCI_CHIP_IVYBRIDGE_GT1 || devid == PCI_CHIP_IVYBRIDGE_M_GT1)
? 2 : 4)
+static inline int num_banks(void) {
+ if (IS_HSW_GT3(devid))
+ return 8; /* 4 per each slice */
+ else if (IS_HSW_GT1(devid) ||
+ devid == PCI_CHIP_IVYBRIDGE_GT1 ||
+ devid == PCI_CHIP_IVYBRIDGE_M_GT1)
+ return 2;
+ else
+ return 4;
+}
#define NUM_SUBBANKS 8
#define BYTES_PER_BANK (128 10)
/* Each row addresses [up to] 4b. This multiplied by the number of subbanks
* will give the L3 size per bank.
* TODO: Row size is fixed on IVB, and variable on HSW.*/
#define MAX_ROW (112)
-#define L3_SIZE ((MAX_ROW * 4) * NUM_SUBBANKS * NUM_BANKS)
-#define NUM_REGS (NUM_BANKS * NUM_SUBBANKS)
-#define MAX_SLICES 1
-#define REAL_MAX_SLICES 1
+#define MAX_BANKS_PER_SLICE 4
+#define NUM_REGS (MAX_BANKS_PER_SLICE * NUM_SUBBANKS)
+#define MAX_SLICES (IS_HSW_GT3(devid) ? 2 : 1)
+#define REAL_MAX_SLICES 2
+/* TODO support SLM config */
+#define L3_SIZE ((MAX_ROW * 4) * NUM_SUBBANKS * num_banks())
struct __attribute__ ((__packed__)) l3_log_register {
uint32_t row0_enable: 1;
@@ -62,7 +71,7 @@ struct __attribute__ ((__packed__)) l3_log_register {
uint32_t row1_enable: 1;
uint32_t rsvd1 : 4;
uint32_t row1 : 11;
-} l3logs[REAL_MAX_SLICES][MAX_BANKS][NUM_SUBBANKS];
+} l3logs[REAL_MAX_SLICES][MAX_BANKS_PER_SLICE][NUM_SUBBANKS];
static int which_slice = -1;
#define for_each_slice(__i) \
@@ -74,16 +83,16 @@ static void dumpit(int slice)
{
int i, j;
- for (i = 0; i NUM_BANKS; i++) {
+ for (i = 0; i MAX_BANKS_PER_SLICE; i++) {
for (j = 0; j NUM_SUBBANKS; j++) {
struct l3_log_register *reg = l3logs[slice][i][j];
if (reg-row0_enable)
- printf(Row %d, Bank %d, Subbank %d is
disabled\n,
- reg-row0, i, j);
+ printf(Slice %d, Row %d, Bank %d, Subbank %d
is disabled\n,
+ slice, reg-row0, i, j);
if (reg-row1_enable)
- printf(Row %d, Bank %d, Subbank %d is
disabled\n,
- reg-row1, i, j);
+ printf(Slice %d, Row %d, Bank %d, Subbank %d
is disabled\n,
+ slice, reg-row1, i, j);
}
}
}
@@ -160,6 +169,8 @@ int main(int argc, char *argv[])
ret = asprintf(path[0], /sys/class/drm/card%d/l3_parity, device);
assert(ret != -1);
+ ret = asprintf(path[1], /sys/class/drm/card%d/l3_parity_slice_1,
device);
+ assert(ret != -1);
for_each_slice(i) {
fd[i] = open(path[i], O_RDWR);
@@ -201,9 +212,9 @@ int main(int argc, char *argv[])
exit(EXIT_SUCCESS);
case 'H':
printf(Number of slices: %d\n, MAX_SLICES);
- printf(Number of banks: %d\n, NUM_BANKS);
+ printf(Number of banks: %d\n, num_banks());
printf(Subbanks per bank: %d\n, NUM_SUBBANKS);
- printf(L3 size: %dK\n, L3_SIZE 10);
+ printf(Max L3 size: %dK\n, L3_SIZE 10);
exit(EXIT_SUCCESS);
case 'r':
row = atoi(optarg);
@@ -212,7 +223,7 @@ int main(int argc, char *argv[])
break;
case 'b':
bank = atoi(optarg);
- if (bank = NUM_BANKS)
+ if (bank = num_banks() || bank =
MAX_BANKS_PER_SLICE)
exit(EXIT_FAILURE);
break;
case 's':
@@ -222,7 +233,7 @@ int main(int argc, char *argv[])
break;
case 'w':
which_slice = atoi(optarg);
- if (which_slice 1)
+ if (which_slice = MAX_SLICES)
exit(EXIT_FAILURE);
[Intel-gfx] [PATCH 14/14] intel_l3_parity: Support a daemonic mode
v2: Add a comment explaining the dangers of directly accessing the DFT
register (Daniel)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/Makefile.am | 6 ++-
tools/intel_l3_parity.c| 46 --
tools/intel_l3_parity.h| 31
tools/intel_l3_udev_listener.c | 108 +
4 files changed, 186 insertions(+), 5 deletions(-)
create mode 100644 tools/intel_l3_parity.h
create mode 100644 tools/intel_l3_udev_listener.c
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 47bd5b3..19810cf 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -39,7 +39,7 @@ dist_bin_SCRIPTS = intel_gpu_abrt
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
-LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS)
+LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS) $(LIBUDEV_LIBS)
intel_dump_decode_SOURCES =\
intel_dump_decode.c
@@ -50,3 +50,7 @@ intel_error_decode_SOURCES = \
intel_bios_reader_SOURCES =\
intel_bios_reader.c \
intel_bios.h
+
+intel_l3_parity_SOURCES = \
+ intel_l3_parity.c \
+ intel_l3_udev_listener.c
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index d2ad3c9..ead8fb5 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -37,6 +37,14 @@
#include intel_chipset.h
#include intel_gpu_tools.h
#include drmtest.h
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+#if HAVE_UDEV
+#include libudev.h
+#include syslog.h
+#endif
+#include intel_l3_parity.h
static unsigned int devid;
/* L3 size is always a function of banks. The number of banks cannot be
@@ -157,7 +165,8 @@ static void usage(const char *name)
-r, --row=[row] The row to act upon
(default 0)\n
-b, --bank=[bank]The bank to act upon
(default 0)\n
-s, --subbank=[subbank] The subbank to act upon
(default 0)\n
- -w, --slice=[slice] Which slice to act on
(default: -1 [all])
+ -w, --slice=[slice] Which slice to act on
(default: -1 [all])\n
+ , --daemon Run the listener (-L)
as a daemon\n
ACTIONS (only 1 may be specified at a time):\n
-h, --help Display this help\n
-H, --hw-infoDisplay the
current L3 properties\n
@@ -166,7 +175,8 @@ static void usage(const char *name)
-e, --enable Enable row, bank,
subbank (undo -d)\n
-d, --disable=row,bank,subbank Disable row, bank,
subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n
-i, --inject [HSW only] Cause
hardware to inject a row errors\n
- -u, --uninject [HSW only] Turn off
hardware error injectection (undo -i)\n,
+ -u, --uninject [HSW only] Turn off
hardware error injectection (undo -i)\n
+ -L, --listen Listen for uevent
errors\n,
name);
}
@@ -179,6 +189,7 @@ int main(int argc, char *argv[])
int fd[REAL_MAX_SLICES] = {0}, ret, i;
int action = '0';
int drm_fd = drm_open_any();
+ int daemonize = 0;
devid = intel_get_drm_devid(drm_fd);
if (intel_gen(devid) 7 || IS_VALLEYVIEW(devid))
@@ -202,11 +213,18 @@ int main(int argc, char *argv[])
assert(lseek(fd[i], 0, SEEK_SET) == 0);
}
+ /* NB: It is potentially unsafe to read this register if the kernel is
+* actively using this register range, or we're running multiple
+* instances of this tool. Since neither of those cases should occur
+* (and the tool should be root only) we can safely ignore this for
+* now. Just be aware of this if for some reason a hang is reported
+* when using this tool.
+*/
dft = intel_register_read(0xb038);
while (1) {
int c, option_index = 0;
- static struct option long_options[] = {
+ struct option long_options[] = {
{ help, no_argument, 0, 'h' },
{ list, no_argument, 0, 'l' },
{ clear-all, no_argument, 0, 'a' },
@@ -215,18 +233,23 @@ int main(int argc, char *argv[])
{ inject, no_argument, 0, 'i' },
{ uninject, no_argument, 0, 'u' },
{ hw-info, no_argument, 0, 'H' },
+ { listen, no_argument, 0, 'L' },
{ row, required_argument, 0, 'r'
[Intel-gfx] [PATCH 13/14] intel_l3_parity: Support error injection
Haswell added the ability to inject errors which is extremely useful for
testing. Add two arguments to the tool to inject, and uninject.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tests/sysfs_l3_parity | 2 +-
tools/intel_l3_parity.c | 69 +++--
2 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/tests/sysfs_l3_parity b/tests/sysfs_l3_parity
index a0dfad9..e9d4411 100755
--- a/tests/sysfs_l3_parity
+++ b/tests/sysfs_l3_parity
@@ -21,7 +21,7 @@ fi
$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
#Check that we can clear remaps
-if [ `$SOURCE_DIR/../tools/intel_l3_parity -l | wc -c` != 0 ] ; then
+if [ `$SOURCE_DIR/../tools/intel_l3_parity -l | wc -l` != 1 ] ; then
echo Fail 2
exit 1
fi
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index ed7034a..d2ad3c9 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -79,6 +79,20 @@ static int which_slice = -1;
(__i) ((which_slice == -1) ? MAX_SLICES :
(which_slice + 1)); \
(__i)++)
+static void decode_dft(uint32_t dft)
+{
+ if (IS_IVYBRIDGE(devid) || !(dft 1)) {
+ printf(Error injection disabled\n);
+ return;
+ }
+ printf(Error injection enabled\n);
+ printf( Hang = %s\n, (dft 28) 0x1 ? yes : no);
+ printf( Row = %d\n, (dft 7) 0x7ff);
+ printf( Bank = %d\n, (dft 2) 0x3);
+ printf( Subbank = %d\n, (dft 4) 0x7);
+ printf( Slice = %d\n, (dft 1) 0x1);
+}
+
static void dumpit(int slice)
{
int i, j;
@@ -150,7 +164,9 @@ static void usage(const char *name)
-l, --list List the current L3
logs\n
-a, --clear-all Clear all disabled
rows\n
-e, --enable Enable row, bank,
subbank (undo -d)\n
- -d, --disable=row,bank,subbank Disable row, bank,
subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n,
+ -d, --disable=row,bank,subbank Disable row, bank,
subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n
+ -i, --inject [HSW only] Cause
hardware to inject a row errors\n
+ -u, --uninject [HSW only] Turn off
hardware error injectection (undo -i)\n,
name);
}
@@ -158,6 +174,7 @@ int main(int argc, char *argv[])
{
const int device = drm_get_card();
char *path[REAL_MAX_SLICES];
+ uint32_t dft;
int row = 0, bank = 0, sbank = 0;
int fd[REAL_MAX_SLICES] = {0}, ret, i;
int action = '0';
@@ -167,6 +184,8 @@ int main(int argc, char *argv[])
if (intel_gen(devid) 7 || IS_VALLEYVIEW(devid))
exit(EXIT_SUCCESS);
+ assert(intel_register_access_init(intel_get_pci_device(), 0) == 0);
+
ret = asprintf(path[0], /sys/class/drm/card%d/l3_parity, device);
assert(ret != -1);
ret = asprintf(path[1], /sys/class/drm/card%d/l3_parity_slice_1,
device);
@@ -183,6 +202,7 @@ int main(int argc, char *argv[])
assert(lseek(fd[i], 0, SEEK_SET) == 0);
}
+ dft = intel_register_read(0xb038);
while (1) {
int c, option_index = 0;
@@ -192,6 +212,8 @@ int main(int argc, char *argv[])
{ clear-all, no_argument, 0, 'a' },
{ enable, no_argument, 0, 'e' },
{ disable, optional_argument, 0, 'd' },
+ { inject, no_argument, 0, 'i' },
+ { uninject, no_argument, 0, 'u' },
{ hw-info, no_argument, 0, 'H' },
{ row, required_argument, 0, 'r' },
{ bank, required_argument, 0, 'b' },
@@ -200,7 +222,7 @@ int main(int argc, char *argv[])
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, hHr:b:s:w:aled::, long_options,
+ c = getopt_long(argc, argv, hHr:b:s:w:aled::iu, long_options,
option_index);
if (c == -1)
break;
@@ -215,6 +237,7 @@ int main(int argc, char *argv[])
printf(Number of banks: %d\n, num_banks());
printf(Subbanks per bank: %d\n, NUM_SUBBANKS);
printf(Max L3 size: %dK\n, L3_SIZE 10);
+ printf(Has error injection: %s\n,
IS_HASWELL(devid) ? yes : no);
exit(EXIT_SUCCESS);
case 'r':
row = atoi(optarg);
@@ -236,6 +259,12 @@ int main(int argc, char *argv[])
if (which_slice = MAX_SLICES)
