[Intel-gfx] [PATCH] drm/i915/dp: constify link_status
Follow-up to
commit 0aec288130713cf7bcf97c929ac5fab6a8e00e44
Author: Jani Nikula jani.nik...@intel.com
Date: Fri Sep 27 19:01:01 2013 +0300
drm/dp: constify DP DPCD helpers
Requested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c392ad2..e4fdedc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2095,7 +2095,8 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp
*intel_dp)
}
static void
-intel_get_adjust_train(struct intel_dp *intel_dp, uint8_t
link_status[DP_LINK_STATUS_SIZE])
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const uint8_t link_status[DP_LINK_STATUS_SIZE])
{
uint8_t v = 0;
uint8_t p = 0;
@@ -2396,7 +2397,7 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp,
uint32_t *DP,
static bool
intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
- uint8_t link_status[DP_LINK_STATUS_SIZE])
+ const uint8_t link_status[DP_LINK_STATUS_SIZE])
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port-base.base.dev;
--
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] drm/i915/dp: constify link_status
On Tue, Oct 15, 2013 at 09:36:08AM +0300, Jani Nikula wrote:
Follow-up to
commit 0aec288130713cf7bcf97c929ac5fab6a8e00e44
Author: Jani Nikula jani.nik...@intel.com
Date: Fri Sep 27 19:01:01 2013 +0300
drm/dp: constify DP DPCD helpers
Requested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Jani Nikula jani.nik...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c392ad2..e4fdedc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2095,7 +2095,8 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp
*intel_dp)
}
static void
-intel_get_adjust_train(struct intel_dp *intel_dp, uint8_t
link_status[DP_LINK_STATUS_SIZE])
+intel_get_adjust_train(struct intel_dp *intel_dp,
+const uint8_t link_status[DP_LINK_STATUS_SIZE])
{
uint8_t v = 0;
uint8_t p = 0;
@@ -2396,7 +2397,7 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp,
uint32_t *DP,
static bool
intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
-uint8_t link_status[DP_LINK_STATUS_SIZE])
+const uint8_t link_status[DP_LINK_STATUS_SIZE])
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port-base.base.dev;
--
1.7.9.5
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: constify link_status
On Tue, Oct 15, 2013 at 09:36:08AM +0300, Jani Nikula wrote: Follow-up to commit 0aec288130713cf7bcf97c929ac5fab6a8e00e44 Author: Jani Nikula jani.nik...@intel.com Date: Fri Sep 27 19:01:01 2013 +0300 drm/dp: constify DP DPCD helpers Requested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-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] [RFC] Runtime display PM for VLV/BYT
On Mon, Oct 14, 2013 at 04:07:44PM -0700, Jesse Barnes wrote: This set adds bits needed for runtime power support, currently only lightly tested on VLV/BYT: 1) suspend/resume callbacks for different platforms 2) save/restore of display state across a power well toggle 3) get/put of display power well in critical places The TODO list still has a few items on it, and I'm looking for feedback: 1) sprinkle around some power well WARNs so we can catch things easily 2) add some tests using DPMS and NULL mode sets and comparing power well state 3) better debugfs support for multiple wells 4) refcount of power well in debugfs (with ref holders?) 5) more testing - I think the load time ref is still busted here and on HSW 6) convert HSW as well so DPMS will shut things down, not just mode sets Thoughts or comments? I'd also like to see what Imre cooked up, and then come up with some grand unified design. Based on our discussions I think his power well abstraction sounded somewhat nicer and more general. Also your locking seems to be fubar in places (frobbing with sideband while holding a spinlock). I think Imre converted the power wells to use a mutex everywhere. Or perhaps we just start with your stuff and Imre rebases his stuff on top? -- 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 09/16] drm/i915: Store current watermark state in dev_priv-wm
On Fri, Oct 11, 2013 at 11:21:04AM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: [snip] + previous.enable_fbc_wm = !(I915_READ(DISP_ARB_CTL) DISP_FBC_WM_DIS); + + if (memcmp(results, previous, sizeof(*results)) == 0) This may cause problems since we're also comparing the structure paddings. It seems results is already zero-initialized, so if you also zero-initialize previous we'll probably be fine with the memcmp(). But my fear is that future code changes will break this, so if you stick with the new memcmp please add a comment remembering us that we rely on zero-initializing stuff. Or maybe keep the old-big-ugly code. I've added a comment about this and then smashed your presumed r-b onto the patch. Please scream if the patch as merged isn't good enough. -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 for crashing intel server
On Tue, Oct 15, 2013 at 03:46:08AM +0200, Bas Wijnen wrote: On Sun, Oct 13, 2013 at 10:43:49AM +0100, Chris Wilson wrote: My X server was crashing when playing video, and I wrote a patch to fix it. Please find the background and the patch at http://bugs.debian.org/724944 . Ok, I can see the allocation failure that leads to the crash: commit f9a18c9f38d09c145eb513ca989966dc135c1e9b Author: Chris Wilson ch...@chris-wilson.co.uk Date: Sun Oct 13 10:36:35 2013 +0100 This does indeed stop the server from crashing, but actually makes the problem worse: it used to play video for a few minutes and then crash when trying. With my patch it would play video for a few minutes and then present black screens when trying. With your patch, it presents black screens from the start. Start of video, or beginning of X? I made two changes. The first to check for a failed GPU pixmap allocation during video playback and the second to check for a failed malloc during Screen initialisation. Neither should be likely. I must say I'm not entirely sure if the backtrace I sent you is a typical case; I managed to crash it sooner than usual, so perhaps it wasn't the bug that I triggered before. It did stop the crashing however. However, that still leaveas the question as to how you ended up being unable to allocate bo... You can watch /sys/kernel/debug/dri/0/i915_gem_objects (or just use intel-gpu-overlay) and see if there is an object leak. I don't have enough knowledge about the internals to know how that works. I can see the file if I mount the debugfs, but what am I looking for? An increase in the number of total objects and allocated bytes. I don't seem to have intel-gpu-overlay on my system; does it make sense to install it? If so, where do I get it? It just presents the same information, so not really important if you are happy with catting the debugfs file. While looking for it I did find and try intel-gpu-time, and noticed that it always reports the gpu 100% busy, even when running intel-gpu-time sleep 5 from a linux virtual terminal (so not even X is displayed). Is that normal? Hmm, looks like it should report correctly on i915. -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 14/16] drm/i915: Add watermark tracepoints
On Fri, Oct 11, 2013 at 04:40:24PM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com We may want to know what kind of watermarks got computed and programmed into the hardware. Using tracepoints is much leaner than debug prints. Also add trace call for the watermark state we read out of the hardware during init, though I;m not sure there's any way to see that trace as the events aren't available until the module is loaded. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I never worked with these things before, but on a quick look it all sounds sane. Acked-by: Paulo Zanoni paulo.r.zan...@intel.com I'm not sold on tracepoints being the right tool here. DRM_DEBUG_KMS probably isn't it, since that would needlessly spam dmesg since it's way too coarse. But the kernel has this neat dynamic debug subsystem, which has the upshot that it's all nicely inline with the other modeset debug noise in dmesg. I'll punt on this for now. -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: Replace has_bsd/blt/vebox with a mask
On Mon, Oct 14, 2013 at 08:46:22PM -0700, Ben Widawsky wrote:
-cleanup_vebox_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[VECS]);
-cleanup_blt_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[BCS]);
-cleanup_bsd_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[VCS]);
-cleanup_render_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[RCS]);
+cleanup:
+ for_each_ring(ring, dev_priv, i) {
+ if (!(INTEL_INFO(dev)-ring_mask (1i)) ||
+ !ring-name)
+ continue;
This looks dubious. You don't need to check ring_mask here as that will
be implicit in whatever we test for completeness. ring-name is set at
the start of initialisation and is not cleaned upon error. A better
choice is ring-obj, which we already check in
intel_cleanup_ring_buffer.
So this becomes:
cleanup:
for_each_ring(ring, dev_priv, i)
+ intel_cleanup_ring_buffer(ring);
-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 16/16] drm/i915: Rename ilk_check_wm to ilk_validate_wm_level
On Fri, Oct 11, 2013 at 02:08:07PM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com Makes the behaviour of the function more clear. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Thanks :) Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com With the exception of the tracepoint patch I've merged the entire series, thanks for patchesreview. Now all these watermark changes start to freak me out since we seem to fully rely on Paulo's sharp eyes to check them. I really think it's time to blow through a few cycles to independently check all this stuff. Some ideas: - Enable the fifo underrun stuff and make it really load. Maybe only on haswell for a start. If this starts to hit issues in the wild we might need some form of display error state which captures all the sprites/cursor/planes/crtc/wm/... state. Maybe we could do this as part of the error state stuff we already have, but with the GT side of things not enabled (since presumably the GT is really busy and we shouldn't unduly poke it). - The hw state readout needs cross-checking. We now rely on the read out wm state (for the first modeset at least, but there's always fastboot). Experienc says that without cross checks this will get broken eventually and lead to fun-to-debug bugs. - I'm not sure whether there's a sane way to dump out the wm settings and check them in userspace. Duplicating the entire calculation is pointless and we can't really integrate the excel spreadsheet from the hw guys into igt. And using a set of interesting corner-cases to test all the basic modes (one pipe, sprite splits, ...) is probably too inflexible. But if we can get stable watermark settings by e.g. injecting an special edid somewhere so that we know the exact dotclocks this might be interesting. - At least exercising some of the special cases (and then relying on the state cross-checker and fifo underrun reporting to catch fallout) from userspace would be good. I'm running a bit low on good stuff here, so better ideas highly welcome. It's not really an area I've wreak much havoc in at all ... One other thing I've noticed is that we still have calls to intel_crtc_active sprinkled throughout the hsw wm functions. Should we be able to ditch those and replace them with a plain crtc-active check, now that we have wm state readout? 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] [RFC] Runtime display PM for VLV/BYT
On Mon, Oct 14, 2013 at 04:07:44PM -0700, Jesse Barnes wrote: 5) more testing - I think the load time ref is still busted here and on HSW I've chatted quite a bit yesterday with Paulo about how we can test runtime pm better, since he wanted to get started with testing D3 while vpg is cooking up the kernel support for this. For general tests that stuff works I've suggested to pimp the existing pc8.c testcase and duplicate all the subtests we have there over all the fancy runtime pm features we'll get. So for (pm_method in pc8, vlv_power_well, D3, ..) for (subtest) do_subtest(pm_method) The pm_method would encapsulate stuff like getting in/out of that state and checking that we indeed have residency. All the functional tests could then be shared (i2c, gpu workloads, ...). Even more important once we stumble over bugs and need new test scenarios. The other thing is static register setup. Atm we set up registers after boot, on resume and (in parts at least) after gpu reset, and we already have bug reports to prove that this is too complicated for us to get right. Adding D3 and tons other things will make it even more fun. The problem isn't really the static set of w/as in the -init_clock_gating vfuncs, but all the bits and pieces splattered all over the driver: - w/a which are someplace else due to ordering constraints (e.g. the ring specific w/a can't be done in init_clock_gating since the ring enabling will clear them again) - static register setup which is someplace else for better code structure (e.g. the ddi buffer translation table setup) Paulo's idea is to convert the w/a list in init_clock_gating into a table and also add all the other registers in there but marked with a special bit saying that those workarounds/settings shouldn't be applied in in the init_clock_gating code. Then we could scan this table at the usual places and check the hw state (e.g. after each modeset with all the other modeset state). The upshot compared to doing this in userspace (Eric started such a tool in i-g-t/tools/intel_reg_checker) is that we won't have a synchronization problem between two codebases. Imo the more dynamic state is already sufficiently locked down with all our asserts and state cross-checks plus the functional checks from pc8.c 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 14/16] drm/i915: Add watermark tracepoints
On Tue, Oct 15, 2013 at 10:43:31AM +0200, Daniel Vetter wrote: On Fri, Oct 11, 2013 at 04:40:24PM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com We may want to know what kind of watermarks got computed and programmed into the hardware. Using tracepoints is much leaner than debug prints. Also add trace call for the watermark state we read out of the hardware during init, though I;m not sure there's any way to see that trace as the events aren't available until the module is loaded. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I never worked with these things before, but on a quick look it all sounds sane. Acked-by: Paulo Zanoni paulo.r.zan...@intel.com I'm not sold on tracepoints being the right tool here. DRM_DEBUG_KMS probably isn't it, since that would needlessly spam dmesg since it's way too coarse. But the kernel has this neat dynamic debug subsystem, which has the upshot that it's all nicely inline with the other modeset debug noise in dmesg. I need to trace the watermark updates in relation to plane updates and vblanks. Tracepoints seem like a good tool to me, though it does make it a bit less useful for bug reports and such. I'm just worried that regular printks add too much overhead to be all that useful for such timing sensitive work with potentially quite a bit of trace data. -- 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/16] drm/i915: Add watermark tracepoints
On Tue, Oct 15, 2013 at 01:11:49PM +0300, Ville Syrjälä wrote: On Tue, Oct 15, 2013 at 10:43:31AM +0200, Daniel Vetter wrote: On Fri, Oct 11, 2013 at 04:40:24PM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com We may want to know what kind of watermarks got computed and programmed into the hardware. Using tracepoints is much leaner than debug prints. Also add trace call for the watermark state we read out of the hardware during init, though I;m not sure there's any way to see that trace as the events aren't available until the module is loaded. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I never worked with these things before, but on a quick look it all sounds sane. Acked-by: Paulo Zanoni paulo.r.zan...@intel.com I'm not sold on tracepoints being the right tool here. DRM_DEBUG_KMS probably isn't it, since that would needlessly spam dmesg since it's way too coarse. But the kernel has this neat dynamic debug subsystem, which has the upshot that it's all nicely inline with the other modeset debug noise in dmesg. I need to trace the watermark updates in relation to plane updates and vblanks. Tracepoints seem like a good tool to me, though it does make it a bit less useful for bug reports and such. I'm just worried that regular printks add too much overhead to be all that useful for such timing sensitive work with potentially quite a bit of trace data. Hm, I've thought we'd only dump the wm state once it's computed before we bash it into the hw. That part doesn't really feel timing critical, at least as long as we have the printks disabled by default. Otherwise syslogd will bring the system down ;-) Now I haven't really played around with it yet, but the dynamic printk stuff seemed to fit that bill. -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/3] drm/i915: Allow GT Slices Shutdown on Boot.
Slices shutdown is a power savings feature present on Haswell GT3 whereby
parts of HW i.e. slice is shut off on boot or dynamically to save power.
This patch only introduces a way to disable half of Haswell GT3 slices on boot.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.c | 5 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 5 +
drivers/gpu/drm/i915/i915_reg.h | 8
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 16 +++-
6 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 59649c0..e3207a2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -154,6 +154,11 @@ module_param_named(prefault_disable,
i915_prefault_disable, bool, 0600);
MODULE_PARM_DESC(prefault_disable,
Disable page prefaulting for pread/pwrite/reloc
(default:false). For developers only.);
+int i915_gt_slice_config __read_mostly = 1;
+module_param_named(gt_slice_config, i915_gt_slice_config, int, 0600);
+MODULE_PARM_DESC(gt_slice_config,
+Haswell GT3 has multiple slices. Use Full (1) for better
performance or Half (0) for better power savings. (default:1));
+
static struct drm_driver driver;
extern int intel_agp_enabled;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6106d3d..02d82d8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1794,6 +1794,7 @@ extern bool i915_fastboot __read_mostly;
extern int i915_enable_pc8 __read_mostly;
extern int i915_pc8_timeout __read_mostly;
extern bool i915_prefault_disable __read_mostly;
+extern int i915_gt_slice_config __read_mostly;
extern int i915_suspend(struct drm_device *dev, pm_message_t state);
extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 71dd030..b52808e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4421,10 +4421,7 @@ i915_gem_init_hw(struct drm_device *dev)
if (dev_priv-ellc_size)
I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
- if (IS_HSW_GT3(dev))
- I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_ENABLED);
- else
- I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_DISABLED);
+ intel_init_gt_slices(dev);
if (HAS_PCH_NOP(dev)) {
u32 temp = I915_READ(GEN7_MSG_CTL);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 13153c3..497c441 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -269,6 +269,14 @@
#define LOWER_SLICE_ENABLED (10)
#define LOWER_SLICE_DISABLED (00)
+#define HSW_GT_SLICE_INFO 0x138064
+#define SLICE_SEL_BOTH (13)
+#define SLICE_AUTOWAKE (12)
+#define SLICE_STATUS_MASK0x3
+#define SLICE_STATUS_GT_OFF (00)
+#define SLICE_STATUS_MAIN_ON (20)
+#define SLICE_STATUS_BOTH_ON (30)
+
/*
* 3D instructions used by the kernel
*/
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e33f387..f21f3fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -834,6 +834,7 @@ void intel_set_power_well(struct drm_device *dev, bool
enable);
void intel_enable_gt_powersave(struct drm_device *dev);
void intel_disable_gt_powersave(struct drm_device *dev);
void ironlake_teardown_rc6(struct drm_device *dev);
+void intel_init_gt_slices(struct drm_device *dev);
void gen6_update_ring_freq(struct drm_device *dev);
void gen6_rps_idle(struct drm_i915_private *dev_priv);
void gen6_rps_boost(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6cd5c8f..a1a2588 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3866,6 +3866,21 @@ static void gen6_enable_rps(struct drm_device *dev)
gen6_gt_force_wake_put(dev_priv);
}
+void intel_init_gt_slices(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!IS_HSW_GT3(dev))
+ return;
+
+ I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_ENABLED);
+
+ if (!i915_gt_slice_config) {
+ I915_WRITE(HSW_GT_SLICE_INFO, ~SLICE_SEL_BOTH);
+ I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_DISABLED);
+ }
+}
+
void gen6_update_ring_freq(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -6022,4 +6037,3 @@ void intel_pm_init(struct drm_device *dev)
INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work,
intel_gen6_powersave_work);
}
-
--
1.7.11.7
___
Intel-gfx mailing list
[Intel-gfx] [PATCH] i915_drm: add exec flag for request forcing Intel GT full.
Rendering buffers that need full GT power can request full power through
this I915_EXEC_GT_FULL flag. If the default is the power saving with half
slices off it executes LRIs to immediately enable all slices.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
include/drm/i915_drm.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index aa983f3..7213ea8 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -670,6 +670,9 @@ struct drm_i915_gem_execbuffer2 {
/** Resets the SO write offset registers for transform feedback on gen7. */
#define I915_EXEC_GEN7_SOL_RESET (18)
+/* Use all available GT Slisces */
+#define I915_EXEC_GT_FULL (113)
+
#define I915_EXEC_CONTEXT_ID_MASK (0x)
#define i915_execbuffer2_set_context_id(eb2, context) \
(eb2).rsvd1 = context I915_EXEC_CONTEXT_ID_MASK
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] testcases: Slice Shutdown sysfs switch and gt force full exec buffer flag.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
tests/Makefile.am | 1 +
tests/gt_slice_config.c | 227
2 files changed, 228 insertions(+)
create mode 100644 tests/gt_slice_config.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0290ae0..cf105e4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -50,6 +50,7 @@ TESTS_progs_M = \
gem_tiled_blits \
gem_tiled_partial_pwrite_pread \
gem_write_read_ring_switch \
+ gt_slice_config \
kms_flip \
kms_render \
kms_setmode \
diff --git a/tests/gt_slice_config.c b/tests/gt_slice_config.c
new file mode 100644
index 000..635535e
--- /dev/null
+++ b/tests/gt_slice_config.c
@@ -0,0 +1,227 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Rodrigo Vivi rodrigo.v...@intel.com
+ *
+ */
+
+/*
+ * Testcase: Test GT slice shutdown feature
+ *
+ * Test both:
+ * 1. sysfs slice config half/full switching
+ * 2. exec buffer flag requesting gt full.
+ */
+
+#define _GNU_SOURCE
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include unistd.h
+#include drmtest.h
+
+static void exec(int fd, uint32_t handle)
+{
+struct drm_i915_gem_execbuffer2 execbuf;
+struct drm_i915_gem_exec_object2 gem_exec[1];
+uint32_t b[2] = {MI_BATCH_BUFFER_END};
+int loops = 200;
+int ret = 0;
+
+gem_write(fd, handle, 0, b, sizeof(b));
+
+gem_exec[0].handle = handle;
+gem_exec[0].relocation_count = 0;
+gem_exec[0].relocs_ptr = 0;
+gem_exec[0].alignment = 0;
+gem_exec[0].offset = 0;
+gem_exec[0].flags = 0;
+gem_exec[0].rsvd1 = 0;
+gem_exec[0].rsvd2 = 0;
+
+execbuf.buffers_ptr = (uintptr_t)gem_exec;
+execbuf.buffer_count = 1;
+execbuf.batch_start_offset = 0;
+execbuf.batch_len = 8;
+execbuf.cliprects_ptr = 0;
+execbuf.num_cliprects = 0;
+execbuf.DR1 = 0;
+execbuf.DR4 = 0;
+execbuf.flags = I915_EXEC_RENDER | I915_EXEC_GT_FULL;
+i915_execbuffer2_set_context_id(execbuf, 0);
+execbuf.rsvd2 = 0;
+
+while (loops-- ret == 0) {
+ret = drmIoctl(fd,
+ DRM_IOCTL_I915_GEM_EXECBUFFER2,
+ execbuf);
+}
+ gem_sync(fd, handle);
+}
+
+static bool is_full(const int device)
+{
+ char *path;
+ FILE *file;
+ int ret;
+ char str[5];
+
+ ret = asprintf(path, /sys/class/drm/card%d/power/gt_slice_config,
+ device);
+ igt_assert(ret != -1);
+
+ file = fopen(path, r);
+ igt_require(file);
+
+ ret = fscanf(file, %s, str);
+ igt_assert(ret != 0);
+
+ fclose(file);
+ return strcmp(str, half);
+}
+
+static int set_status(const int device, bool full)
+{
+ char *path;
+ FILE *file;
+ int ret;
+ char str[5];
+ int attempts = 10;
+
+ ret = asprintf(path, /sys/class/drm/card%d/power/gt_slice_config,
+ device);
+ igt_assert(ret != -1);
+
+ file = fopen(path, r);
+ igt_require(file);
+
+ ret = fscanf(file, %s, str);
+ igt_assert(ret != 0);
+
+ fclose(file);
+
+ if (full == strcmp(str, half))
+ return 0;
+
+ while (attempts-- ret != 0) {
+ file = fopen(path, w);
+ igt_require(file);
+ ret = fprintf(file, %s\n, full ? full: half);
+ igt_assert(ret != -1);
+ ret = fclose(file);
+ sleep(1);
+ }
+ return ret;
+}
+
+int main(int argc, char **argv)
+{
+ char *path;
+ FILE *file;
+ unsigned int rc6_enabled;
+ int ret;
+ uint32_t handle;
+ const int
[Intel-gfx] [PATCH 2/3] drm/i915: Slice Shutdown: Allow setting slice config (full x half) through sysfs
This patch introduces a sysfs interface to easily allow dynamically switch
slice config default behaviour between full and half slices.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_sysfs.c | 54
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 58 +--
4 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 02d82d8..685fb1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1728,6 +1728,7 @@ struct drm_i915_file_private {
#define HAS_POWER_WELL(dev)(IS_HASWELL(dev))
#define HAS_FPGA_DBG_UNCLAIMED(dev)(INTEL_INFO(dev)-has_fpga_dbg)
#define HAS_PSR(dev) (IS_HASWELL(dev))
+#define HAS_SLICE_SHUTDOWN(dev)(IS_HSW_GT3(dev) i915_enable_rc6)
#define INTEL_PCH_DEVICE_ID_MASK 0xff00
#define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index bb31ff3..86ccd52 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -117,6 +117,52 @@ static struct attribute_group rc6_attr_group = {
.name = power_group_name,
.attrs = rc6_attrs
};
+
+static ssize_t gt_slice_config_show(struct device *kdev,
+ struct device_attribute *attr, char *buf)
+{
+ struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+ struct drm_device *dev = minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ return sprintf(buf, %s\n, I915_READ(MI_PREDICATE_RESULT_2) ==
+ LOWER_SLICE_ENABLED ? full : half);
+}
+
+static ssize_t gt_slice_config_store(struct device *kdev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+ struct drm_device *dev = minor-dev;
+ int ret;
+
+ if (!strncmp(buf, full, sizeof(full) - 1)) {
+ ret = intel_set_gt_full(dev);
+ if (ret)
+ return ret;
+ } else if (!strncmp(buf, half, sizeof(half) - 1)) {
+ ret = intel_set_gt_half(dev);
+ if (ret)
+ return ret;
+ } else
+ return -EINVAL;
+ return count;
+}
+
+static DEVICE_ATTR(gt_slice_config, S_IRUGO | S_IWUSR, gt_slice_config_show,
+ gt_slice_config_store);
+
+static struct attribute *gt_slice_config_attrs[] = {
+ dev_attr_gt_slice_config.attr,
+ NULL
+};
+
+static struct attribute_group gt_slice_config_attr_group = {
+ .name = power_group_name,
+ .attrs = gt_slice_config_attrs
+};
+
#endif
static int l3_access_valid(struct drm_device *dev, loff_t offset)
@@ -558,6 +604,12 @@ void i915_setup_sysfs(struct drm_device *dev)
if (ret)
DRM_ERROR(RC6 residency sysfs setup failed\n);
}
+ if (HAS_SLICE_SHUTDOWN(dev)) {
+ ret = sysfs_merge_group(dev-primary-kdev.kobj,
+ gt_slice_config_attr_group);
+ if (ret)
+ DRM_ERROR(GT slice config sysfs setup failed\n);
+ }
#endif
if (HAS_L3_DPF(dev)) {
ret = device_create_bin_file(dev-primary-kdev, dpf_attrs);
@@ -597,5 +649,7 @@ void i915_teardown_sysfs(struct drm_device *dev)
device_remove_bin_file(dev-primary-kdev, dpf_attrs);
#ifdef CONFIG_PM
sysfs_unmerge_group(dev-primary-kdev.kobj, rc6_attr_group);
+ sysfs_unmerge_group(dev-primary-kdev.kobj,
+ gt_slice_config_attr_group);
#endif
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f21f3fa..a9abbb5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -834,6 +834,8 @@ void intel_set_power_well(struct drm_device *dev, bool
enable);
void intel_enable_gt_powersave(struct drm_device *dev);
void intel_disable_gt_powersave(struct drm_device *dev);
void ironlake_teardown_rc6(struct drm_device *dev);
+int intel_set_gt_full(struct drm_device *dev);
+int intel_set_gt_half(struct drm_device *dev);
void intel_init_gt_slices(struct drm_device *dev);
void gen6_update_ring_freq(struct drm_device *dev);
void gen6_rps_idle(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a1a2588..63af075 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3866,14 +3866,66 @@ static void gen6_enable_rps(struct drm_device *dev)
gen6_gt_force_wake_put(dev_priv);
}
-void
[Intel-gfx] [PATCH 3/3] drm/i915: HSW GT3 Slices: Userspace can dynamically forcibly enable all slices
Rendering buffers that need full GT power can request full power through
this I915_EXEC_GT_FULL flag. If the default is the power saving with half
slices off it executes LRIs to immediately enable all slices.
CC: Eric Anholt e...@anholt.net
CC: Kenneth Graunke kenn...@whitecape.org
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.h| 8
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 67 ++
drivers/gpu/drm/i915/i915_reg.h| 11 +
drivers/gpu/drm/i915/i915_sysfs.c | 11 -
drivers/gpu/drm/i915/intel_display.c | 6 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c| 41 --
include/uapi/drm/i915_drm.h| 5 ++-
8 files changed, 144 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 685fb1d..bd4774e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1219,6 +1219,12 @@ struct i915_package_c8 {
} regsave;
};
+struct i915_gt_slices {
+ int state_default;
+ int forcing_full;
+ struct mutex m_lock;
+};
+
typedef struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1418,6 +1424,8 @@ typedef struct drm_i915_private {
struct i915_package_c8 pc8;
+ struct i915_gt_slices gt_slices;
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
* here! */
struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 0ce0d47..16dc86e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -923,6 +923,66 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
}
static int
+i915_gt_full_immediate(struct drm_device *dev, struct intel_ring_buffer *ring)
+{
+ drm_i915_private_t *dev_priv = dev-dev_private;
+ int ret;
+
+ if (!HAS_SLICE_SHUTDOWN(dev) || ring == dev_priv-ring[BCS])
+ return 0;
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, HSW_GT_SLICE_INFO);
+ intel_ring_emit(ring, SLICE_SEL_BOTH);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, MI_PREDICATE_RESULT_2);
+ intel_ring_emit(ring, LOWER_SLICE_ENABLED);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, HSW_SLICESHUTDOWN);
+ intel_ring_emit(ring, ~SLICE_SHUTDOWN);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, RC_IDLE_MAX_COUNT);
+ intel_ring_emit(ring, CS_IDLE_COUNT_1US);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, WAIT_FOR_RC6_EXIT);
+ intel_ring_emit(ring, WAIT_RC6_EXIT | MASK_WAIT_RC6_EXIT);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, RC_IDLE_MAX_COUNT);
+ intel_ring_emit(ring, CS_IDLE_COUNT_5US);
+ intel_ring_advance(ring);
+
+ return 0;
+}
+
+static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
struct drm_i915_gem_execbuffer2 *args,
@@ -999,6 +1059,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
return -EINVAL;
}
+ if (args-flags I915_EXEC_GT_FULL
+ dev_priv-gt_slices.state_default == 0
+ !dev_priv-gt_slices.forcing_full) {
+ dev_priv-gt_slices.forcing_full = true;
+ i915_gt_full_immediate(dev, ring);
+ }
+
mode = args-flags I915_EXEC_CONSTANTS_MASK;
mask = I915_EXEC_CONSTANTS_MASK;
switch (mode) {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 497c441..0146bef 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -277,6 +277,17 @@
#define SLICE_STATUS_MAIN_ON (20)
#define SLICE_STATUS_BOTH_ON (30)
+#define HSW_SLICESHUTDOWN 0xA190
+#define SLICE_SHUTDOWN (10)
+
+#define RC_IDLE_MAX_COUNT 0x2054
+#define
Re: [Intel-gfx] [PATCH] drm/i915: Replace has_bsd/blt/vebox with a mask
On Tue, Oct 15, 2013 at 09:50:39AM +0100, Chris Wilson wrote:
On Mon, Oct 14, 2013 at 08:46:22PM -0700, Ben Widawsky wrote:
-cleanup_vebox_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[VECS]);
-cleanup_blt_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[BCS]);
-cleanup_bsd_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[VCS]);
-cleanup_render_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[RCS]);
+cleanup:
+ for_each_ring(ring, dev_priv, i) {
+ if (!(INTEL_INFO(dev)-ring_mask (1i)) ||
+ !ring-name)
+ continue;
This looks dubious. You don't need to check ring_mask here as that will
be implicit in whatever we test for completeness. ring-name is set at
the start of initialisation and is not cleaned upon error. A better
choice is ring-obj, which we already check in
intel_cleanup_ring_buffer.
So this becomes:
cleanup:
for_each_ring(ring, dev_priv, i)
+ intel_cleanup_ring_buffer(ring);
-Chris
Actually, I just plopped this code in at the last minute because I
wanted to provide a sample usage in the patch too. I do have some issues
in the future of using for_each_ring(), hopefully you remember those...
In any event, I'll gladly drop this hunk. Can you review the rest?
P.S. if you want my acked-by on this above mentioned cleanup, have it.
--
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: Replace has_bsd/blt/vebox with a mask
On Tue, Oct 15, 2013 at 08:03:25AM -0700, Ben Widawsky wrote:
On Tue, Oct 15, 2013 at 09:50:39AM +0100, Chris Wilson wrote:
On Mon, Oct 14, 2013 at 08:46:22PM -0700, Ben Widawsky wrote:
-cleanup_vebox_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[VECS]);
-cleanup_blt_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[BCS]);
-cleanup_bsd_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[VCS]);
-cleanup_render_ring:
- intel_cleanup_ring_buffer(dev_priv-ring[RCS]);
+cleanup:
+ for_each_ring(ring, dev_priv, i) {
+ if (!(INTEL_INFO(dev)-ring_mask (1i)) ||
+ !ring-name)
+ continue;
This looks dubious. You don't need to check ring_mask here as that will
be implicit in whatever we test for completeness. ring-name is set at
the start of initialisation and is not cleaned upon error. A better
choice is ring-obj, which we already check in
intel_cleanup_ring_buffer.
So this becomes:
cleanup:
for_each_ring(ring, dev_priv, i)
+ intel_cleanup_ring_buffer(ring);
-Chris
Actually, I just plopped this code in at the last minute because I
wanted to provide a sample usage in the patch too. I do have some issues
in the future of using for_each_ring(), hopefully you remember those...
In any event, I'll gladly drop this hunk. Can you review the rest?
No comments on the rest and lgtm, so
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Runtime display PM for VLV/BYT
On Tue, 15 Oct 2013 15:16:11 +0300 Imre Deak imre.d...@intel.com wrote: On Tue, 2013-10-15 at 11:06 +0300, Ville Syrjälä wrote: On Mon, Oct 14, 2013 at 04:07:44PM -0700, Jesse Barnes wrote: This set adds bits needed for runtime power support, currently only lightly tested on VLV/BYT: 1) suspend/resume callbacks for different platforms 2) save/restore of display state across a power well toggle 3) get/put of display power well in critical places The TODO list still has a few items on it, and I'm looking for feedback: 1) sprinkle around some power well WARNs so we can catch things easily 2) add some tests using DPMS and NULL mode sets and comparing power well state 3) better debugfs support for multiple wells 4) refcount of power well in debugfs (with ref holders?) 5) more testing - I think the load time ref is still busted here and on HSW 6) convert HSW as well so DPMS will shut things down, not just mode sets Thoughts or comments? I'd also like to see what Imre cooked up, and then come up with some grand unified design. Based on our discussions I think his power well abstraction sounded somewhat nicer and more general. I've pushed what I have so far to: https://github.com/ideak/linux/commits/powerwells I've tested this on VLV with VGA output so far and somewhat on HSW. I'd still have to check the need to do any HW state save/restore and the GFX clock forcing, afaics Jesse has already code for these in his patchset. I think a few can be pushed upstream now: drm/i915: change power_well-lock to be mutex drm/i915: factor out is_always_on_domain drm/i915: factor out reset_vblank_counter drm/i915: fix HAS_POWER_WELL-IS_HASWELL - pushed this but it doesn't look like Daniel has picked it up yet drm/i915: check powerwell in get_pipe_config drm/i915: enable only the needed power domains during modeset Looks good too, and I guess there's no harm in pushing the earlier patch to move the get of the power wells out of the HSW global res function, even if we do move to get/put later. For drm/i915: support for multiple power wells, I think I prefer a single uncore function taking a power well, rather than an array of power_well structs. If we went that direction, we could probably rename the power_well variable to audio_power_wells or something and just track the ones we need to expose to the audio driver there, rather than everything. That might be a little clearer than what we have today, but I guess I'd have to see it coded to be sure. For drm/i915: add intel_encoder_get_hw_state, do you think it would be clearer to take refs in the caller of the encoder-get_hw_state instead? I like drm/i915: get port power domains for connector detect and drm/i915: add output power domains too, we'll need those if we want to do fine grained output control on BYT and future chips. But they'll probably need to be rebased on top of the above once it goes upstream. Overall I don't think there's a ton of conflict between our patchsets, they seem mostly complimentary and let me remove hacks in mine. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: HSW GT3 Slices: Userspace can dynamically forcibly enable all slices
Rodrigo Vivi rodrigo.v...@gmail.com writes: Rendering buffers that need full GT power can request full power through this I915_EXEC_GT_FULL flag. If the default is the power saving with half slices off it executes LRIs to immediately enable all slices. How is userspace supposed to decide that the GPU needs to be at full power? Power management is a system problem which wants awareness of the total load on the GPU from all clients, not just 1. The kernel can get that information, but userspace can't. Basically, if you give userspace this knob, userspace will just have to set it on every batchbuffer. At which point, why give them the knob at all? pgpHDG9khQBuH.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/16] drm/i915: Store current watermark state in dev_priv-wm
2013/10/15 Daniel Vetter dan...@ffwll.ch: On Fri, Oct 11, 2013 at 11:21:04AM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: [snip] + previous.enable_fbc_wm = !(I915_READ(DISP_ARB_CTL) DISP_FBC_WM_DIS); + + if (memcmp(results, previous, sizeof(*results)) == 0) This may cause problems since we're also comparing the structure paddings. It seems results is already zero-initialized, so if you also zero-initialize previous we'll probably be fine with the memcmp(). But my fear is that future code changes will break this, so if you stick with the new memcmp please add a comment remembering us that we rely on zero-initializing stuff. Or maybe keep the old-big-ugly code. I've added a comment about this and then smashed your presumed r-b onto the patch. Please scream if the patch as merged isn't good enough. Patch 10 removes the memcmp, so the problem is fixed. You should then remove the comment on that patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/16] drm/i915: Rename ilk_check_wm to ilk_validate_wm_level
2013/10/15 Daniel Vetter dan...@ffwll.ch: On Fri, Oct 11, 2013 at 02:08:07PM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com Makes the behaviour of the function more clear. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Thanks :) Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com With the exception of the tracepoint patch I've merged the entire series, thanks for patchesreview. Now all these watermark changes start to freak me out since we seem to fully rely on Paulo's sharp eyes to check them. I really think it's time to blow through a few cycles to independently check all this stuff. Some ideas: Before reviewing each of Ville's series I usually dump the current WM configurations of eDP-only, eDP+DP, nothing, DP+something with intel-reg-dumper and then apply his patches and compare the results. So far we're good, the only change I have noticed was already discussed here. Also, all this code only runs on Haswell right now (even though the goal is to run it on ILK+), so checking regressions is not really that hard today. Of course, I don't do full corner-case checking. I always thought about writing some IGT test to check watermarks, but the problem is that we'd have to reimplement the WM code on user space if we want to validate the Kernel code, so not really a feasible solution. - Enable the fifo underrun stuff and make it really load. Maybe only on haswell for a start. If this starts to hit issues in the wild we might need some form of display error state which captures all the sprites/cursor/planes/crtc/wm/... state. Maybe we could do this as part of the error state stuff we already have, but with the GT side of things not enabled (since presumably the GT is really busy and we shouldn't unduly poke it). That was already suggested, but since Ville seems to have the code to properly set watermarks on ILK+ already written, I think we should just wait for it. - The hw state readout needs cross-checking. We now rely on the read out wm state (for the first modeset at least, but there's always fastboot). Experienc says that without cross checks this will get broken eventually and lead to fun-to-debug bugs. The nice thing of this series is that it adds the infrastructure to do the HW state readout + check. I even suggested this already. Maybe it's already on Ville's TODO list :) - I'm not sure whether there's a sane way to dump out the wm settings and check them in userspace. Duplicating the entire calculation is pointless and we can't really integrate the excel spreadsheet from the hw guys into igt. And using a set of interesting corner-cases to test all the basic modes (one pipe, sprite splits, ...) is probably too inflexible. But if we can get stable watermark settings by e.g. injecting an special edid somewhere so that we know the exact dotclocks this might be interesting. Watermarks also depend on the machine memory configuration (SSKPD) so that's not really easy... The intel-reg-dumper tools dumps all the relevant registers and can be easily be used to compare against the spreadsheed. OTOH, we could hardcode the common SSKPD values (at least from QA's machines) and the values for some common modes (1024x768, 1920x1080, etc) and check the state set by the Kernel against our hardcoded state... It's not the best solution, but at least it's something. - At least exercising some of the special cases (and then relying on the state cross-checker and fifo underrun reporting to catch fallout) from userspace would be good. I'm running a bit low on good stuff here, so better ideas highly welcome. It's not really an area I've wreak much havoc in at all ... One other thing I've noticed is that we still have calls to intel_crtc_active sprinkled throughout the hsw wm functions. Should we be able to ditch those and replace them with a plain crtc-active check, now that we have wm state readout? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Replace has_bsd/blt/vebox with a mask
From: Ben Widawsky b...@bwidawsk.net
I've sent this patch several times for various reasons. It essentially
cleans up a lot of code where we need to do something per ring, and want
to query whether or not the ring exists on that hardware.
It has various uses coming up, but for now it shouldn't be too
offensive.
v2: Big conflict resolution on Damien's DEV_INFO_FOR_EACH stuff
v3: Resolved vebox addition
v4: Rebased after months of disuse. Also made failed ringbuffer init
cleaner.
v5: Remove the init cleaner from v4. There is a better way to do it.
(Chris)
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.c | 32
drivers/gpu/drm/i915/i915_drv.h | 14 --
2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index db84e24..e9dfadc 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -160,49 +160,58 @@ extern int intel_agp_enabled;
static const struct intel_device_info intel_i830_info = {
.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
.has_overlay = 1, .overlay_needs_physical = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_845g_info = {
.gen = 2, .num_pipes = 1,
.has_overlay = 1, .overlay_needs_physical = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i85x_info = {
.gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 2,
.cursor_needs_physical = 1,
.has_overlay = 1, .overlay_needs_physical = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i865g_info = {
.gen = 2, .num_pipes = 1,
.has_overlay = 1, .overlay_needs_physical = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i915g_info = {
.gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 2,
.has_overlay = 1, .overlay_needs_physical = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i915gm_info = {
.gen = 3, .is_mobile = 1, .num_pipes = 2,
.cursor_needs_physical = 1,
.has_overlay = 1, .overlay_needs_physical = 1,
.supports_tv = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i945g_info = {
.gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 2,
.has_overlay = 1, .overlay_needs_physical = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i945gm_info = {
.gen = 3, .is_i945gm = 1, .is_mobile = 1, .num_pipes = 2,
.has_hotplug = 1, .cursor_needs_physical = 1,
.has_overlay = 1, .overlay_needs_physical = 1,
.supports_tv = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i965g_info = {
.gen = 4, .is_broadwater = 1, .num_pipes = 2,
.has_hotplug = 1,
.has_overlay = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_i965gm_info = {
@@ -210,18 +219,20 @@ static const struct intel_device_info intel_i965gm_info =
{
.is_mobile = 1, .has_fbc = 1, .has_hotplug = 1,
.has_overlay = 1,
.supports_tv = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_g33_info = {
.gen = 3, .is_g33 = 1, .num_pipes = 2,
.need_gfx_hws = 1, .has_hotplug = 1,
.has_overlay = 1,
+ .ring_mask = RENDER_RING,
};
static const struct intel_device_info intel_g45_info = {
.gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 2,
.has_pipe_cxsr = 1, .has_hotplug = 1,
- .has_bsd_ring = 1,
+ .ring_mask = RENDER_RING | BSD_RING,
};
static const struct intel_device_info intel_gm45_info = {
@@ -229,7 +240,7 @@ static const struct intel_device_info intel_gm45_info = {
.is_mobile = 1, .need_gfx_hws = 1, .has_fbc = 1,
.has_pipe_cxsr = 1, .has_hotplug = 1,
.supports_tv = 1,
- .has_bsd_ring = 1,
+ .ring_mask = RENDER_RING | BSD_RING,
};
static const struct intel_device_info intel_pineview_info = {
@@ -241,21 +252,20 @@ static const struct intel_device_info intel_pineview_info
= {
static const struct intel_device_info intel_ironlake_d_info = {
.gen = 5, .num_pipes = 2,
.need_gfx_hws = 1, .has_hotplug = 1,
- .has_bsd_ring = 1,
+ .ring_mask = RENDER_RING | BSD_RING,
};
static const struct intel_device_info intel_ironlake_m_info = {
.gen = 5, .is_mobile = 1, .num_pipes = 2,
.need_gfx_hws = 1, .has_hotplug = 1,
.has_fbc = 1,
- .has_bsd_ring = 1,
+ .ring_mask = RENDER_RING | BSD_RING,
};
static const struct intel_device_info intel_sandybridge_d_info = {
.gen = 6,
Re: [Intel-gfx] [PATCH 3/3] drm/i915: HSW GT3 Slices: Userspace can dynamically forcibly enable all slices
please ignore this one and other two (libdrm and igt) with exec flag... will do a v2 and resend soon.. On Tue, Oct 15, 2013 at 1:14 PM, Eric Anholt e...@anholt.net wrote: Rodrigo Vivi rodrigo.v...@gmail.com writes: Rendering buffers that need full GT power can request full power through this I915_EXEC_GT_FULL flag. If the default is the power saving with half slices off it executes LRIs to immediately enable all slices. How is userspace supposed to decide that the GPU needs to be at full power? Power management is a system problem which wants awareness of the total load on the GPU from all clients, not just 1. The kernel can get that information, but userspace can't. Basically, if you give userspace this knob, userspace will just have to set it on every batchbuffer. At which point, why give them the knob at all? -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: add support for additional stereo 3D modes
On 11 October 2013 12:12, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Thu, Oct 10, 2013 at 02:19:15PM +0100, Thomas Wood wrote:
Parse the 3D_Structure_ALL and 3D_MASK fields of the HDMI Vendor
Specific Data Block to expose more stereo 3D modes.
Signed-off-by: Thomas Wood thomas.w...@intel.com
---
drivers/gpu/drm/drm_edid.c | 93
++
1 file changed, 85 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e81609..b3949f9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2652,6 +2652,50 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
return 1;
}
+static int add_3d_struct_modes(struct drm_connector *connector, u16
structure,
+const u8 *video_db, u8 video_len, u8
video_index)
+{
+ struct drm_device *dev = connector-dev;
+ struct drm_display_mode *newmode;
+ int modes = 0;
+ u8 cea_mode;
+
+ if (video_db == NULL || video_index video_len)
+ return 0;
+
+ /* CEA modes are numbered 1..127 */
+ cea_mode = (video_db[video_index] 127) - 1;
+ if (cea_mode = ARRAY_SIZE(edid_cea_modes))
+ return 0;
+
+ if (structure 1) {
Could use (1 0) for consistency.
I've updated this for v2.
I'm also wondering if some displays might include some of the mandatory
modes in 3D_Structure_ALL, and if so should we filter out the
duplicates?
As Damien mentioned, duplicates are filtered out later on.
+ if ((multi_present == 1 || multi_present == 2)
+ len = (9 + offset)) {
If multi_present==2 and len is too small for the mask, I think we should
skip adding the modes since the block is clearly incorrect/corrupted.
So maybe just something like this:
if ((multi_present == 1 len (9 + offset)) ||
(multi_present == 2 len (11 + offset)))
goto out;
I've added this check to v2 and also made sure multi_present is either 1 or 2
before continuing.
I would also add a similar check for HDMI_3D_LEN since that is
supposed to include the space required by 3D_Structure_ALL and
3D_MASK. Or you could just check HDMI_3D_LEN against 'len' and bail
out if that doesn't fit. And then you could just check
3D_Structure_ALL and 3D_MASK against HDMI_3D_LEN.
I've added a check to ensure the value of HDMI_3D_LEN is large enough to
include 3D_Structure_ALL and 3D_MASK, if they are present.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm: add support for additional stereo 3D modes
Parse the 3D_Structure_ALL and 3D_MASK fields of the HDMI Vendor
Specific Data Block to expose more stereo 3D modes.
Signed-off-by: Thomas Wood thomas.w...@intel.com
---
drivers/gpu/drm/drm_edid.c | 105 +
1 file changed, 96 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e81609..456a694 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2652,6 +2652,50 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
return 1;
}
+static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
+ const u8 *video_db, u8 video_len, u8 video_index)
+{
+ struct drm_device *dev = connector-dev;
+ struct drm_display_mode *newmode;
+ int modes = 0;
+ u8 cea_mode;
+
+ if (video_db == NULL || video_index video_len)
+ return 0;
+
+ /* CEA modes are numbered 1..127 */
+ cea_mode = (video_db[video_index] 127) - 1;
+ if (cea_mode = ARRAY_SIZE(edid_cea_modes))
+ return 0;
+
+ if (structure (1 0)) {
+ newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+ if (newmode) {
+ newmode-flags |= DRM_MODE_FLAG_3D_FRAME_PACKING;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+ if (structure (1 6)) {
+ newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+ if (newmode) {
+ newmode-flags |= DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+ if (structure (1 8)) {
+ newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+ if (newmode) {
+ newmode-flags = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+
+ return modes;
+}
+
/*
* do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
* @connector: connector corresponding to the HDMI sink
@@ -2662,10 +2706,13 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
* also adds the stereo 3d modes when applicable.
*/
static int
-do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
+ const u8 *video_db, u8 video_len)
{
- int modes = 0, offset = 0, i;
- u8 vic_len;
+ int modes = 0, offset = 0, i, multi_present = 0;
+ u8 vic_len, hdmi_3d_len;
+ u16 mask;
+ u16 structure_all;
if (len 8)
goto out;
@@ -2689,9 +2736,13 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len)
/* 3D_Present */
offset++;
- if (db[8 + offset] (1 7))
+ if (db[8 + offset] (1 7)) {
modes += add_hdmi_mandatory_stereo_modes(connector);
+ /* 3D_Multi_present */
+ multi_present = (db[8 + offset] 0x60) 5;
+ }
+
offset++;
vic_len = db[8 + offset] 5;
@@ -2702,6 +2753,38 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len)
modes += add_hdmi_mode(connector, vic);
}
+ if (!(multi_present == 1 || multi_present == 2))
+ goto out;
+
+ if ((multi_present == 1 len (9 + offset)) ||
+ (multi_present == 2 len (11 + offset)))
+ goto out;
+
+ hdmi_3d_len = db[8 + offset] 0x1f;
+
+ if ((multi_present == 1 hdmi_3d_len 2) ||
+ (multi_present == 2 hdmi_3d_len 4))
+ goto out;
+
+ offset += 1 + vic_len;
+
+ /* 3D_Structure_ALL */
+ structure_all = (db[8 + offset] 8) | db[9 + offset];
+
+ /* check if 3D_MASK is present */
+ if (multi_present == 2)
+ mask = (db[10 + offset] 8) | db[11 + offset];
+ else
+ mask = 0x;
+
+ for (i = 0; i 16; i++) {
+ if (mask (1 i))
+ modes += add_3d_struct_modes(connector,
+structure_all,
+video_db,
+video_len, i);
+ }
+
out:
return modes;
}
@@ -2759,8 +2842,8 @@ static int
add_cea_modes(struct drm_connector *connector, struct edid *edid)
{
const u8 *cea = drm_find_cea_extension(edid);
- const u8 *db, *hdmi = NULL;
- u8 dbl, hdmi_len;
+ const u8 *db, *hdmi = NULL, *video = NULL;
+ u8 dbl, hdmi_len, video_len = 0;
int modes = 0;
if (cea
Re: [Intel-gfx] [PATCH 16/16] drm/i915: Rename ilk_check_wm to ilk_validate_wm_level
On Tue, Oct 15, 2013 at 02:01:39PM -0300, Paulo Zanoni wrote: 2013/10/15 Daniel Vetter dan...@ffwll.ch: On Fri, Oct 11, 2013 at 02:08:07PM -0300, Paulo Zanoni wrote: 2013/10/9 ville.syrj...@linux.intel.com: From: Ville Syrjälä ville.syrj...@linux.intel.com Makes the behaviour of the function more clear. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Thanks :) Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com With the exception of the tracepoint patch I've merged the entire series, thanks for patchesreview. Now all these watermark changes start to freak me out since we seem to fully rely on Paulo's sharp eyes to check them. I really think it's time to blow through a few cycles to independently check all this stuff. Some ideas: Before reviewing each of Ville's series I usually dump the current WM configurations of eDP-only, eDP+DP, nothing, DP+something with intel-reg-dumper and then apply his patches and compare the results. So far we're good, the only change I have noticed was already discussed here. Also, all this code only runs on Haswell right now (even though the goal is to run it on ILK+), so checking regressions is not really that hard today. Of course, I don't do full corner-case checking. I always thought about writing some IGT test to check watermarks, but the problem is that we'd have to reimplement the WM code on user space if we want to validate the Kernel code, so not really a feasible solution. - Enable the fifo underrun stuff and make it really load. Maybe only on haswell for a start. If this starts to hit issues in the wild we might need some form of display error state which captures all the sprites/cursor/planes/crtc/wm/... state. Maybe we could do this as part of the error state stuff we already have, but with the GT side of things not enabled (since presumably the GT is really busy and we shouldn't unduly poke it). That was already suggested, but since Ville seems to have the code to properly set watermarks on ILK+ already written, I think we should just wait for it. The current code is still unsafe wrt. underruns, even on HSW. So if we extend underrun reporting at this point, we're sure to get some extra noise. I'm working on the safe update mechnism, and it's starting to look fairly decent, although it does make the whole thing a notch more complex again. I still had some sprite underrun issues on ILK, but I think I just discovered the magic sequence to fix that. Now I have the watermarks for all planes set up dynamically depending on the current need. Eg. if you disable the cursor even the cursor watermarks get zeroed out. Also it seems I've accidentally fixed the 5/6 DDB split on IVB. Previously I had issues with it, but that may have been a simple bug in my earlier code since I'm not seeing it currently. But I do need to run some more tests on all affected machines to make sure I haven't missed something. I could post the basic ILK/SNB/IVB enabling patches already. I don't think they can severly regress the current situation, so in that sense they should be fairly OK to push in. I just figured I'd give you a few days to catch your breath :) - The hw state readout needs cross-checking. We now rely on the read out wm state (for the first modeset at least, but there's always fastboot). Experienc says that without cross checks this will get broken eventually and lead to fun-to-debug bugs. The nice thing of this series is that it adds the infrastructure to do the HW state readout + check. I even suggested this already. Maybe it's already on Ville's TODO list :) - I'm not sure whether there's a sane way to dump out the wm settings and check them in userspace. Duplicating the entire calculation is pointless and we can't really integrate the excel spreadsheet from the hw guys into igt. And using a set of interesting corner-cases to test all the basic modes (one pipe, sprite splits, ...) is probably too inflexible. But if we can get stable watermark settings by e.g. injecting an special edid somewhere so that we know the exact dotclocks this might be interesting. Watermarks also depend on the machine memory configuration (SSKPD) so that's not really easy... The intel-reg-dumper tools dumps all the relevant registers and can be easily be used to compare against the spreadsheed. OTOH, we could hardcode the common SSKPD values (at least from QA's machines) and the values for some common modes (1024x768, 1920x1080, etc) and check the state set by the Kernel against our hardcoded state... It's not the best solution, but at least it's something. We'd need to add a debugfs file to override the latency values since I changed the code to use the copies stored in dev_priv. But that could be a generally useful underrun debug mechanism anyway. I suppose the file contents could just reflect the MLTR/SSKPD register value, or
[Intel-gfx] Pipe CRCs v1
This series exposes the pipe CRCs on ivybridge through debugfs. It's based on the initial work from Shuang He, with some improvements to have a nice user space API. There are several points in the display pipeline where CRCs can be computed on the bits flowing there. For instance, it's usually possible to compute the CRCs of the primary plane, the sprite plane or the CRCs of the bits after the panel fitter (collectively called pipe CRCs). An intel-gpu-tools series will follow with helpers to use the feature from tests and basic testing. Further work items: * make it work on other platforms * expose other CRCs than just the pipe CRCs (transcoders, ddi, ...) * implement poll() for the result files -- Damien drivers/gpu/drm/i915/i915_debugfs.c | 503 ++-- drivers/gpu/drm/i915/i915_dma.c | 2 + drivers/gpu/drm/i915/i915_drv.h | 31 ++ drivers/gpu/drm/i915/i915_irq.c | 49 +++ drivers/gpu/drm/i915/i915_reg.h | 36 +- 5 files changed, 594 insertions(+), 27 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/16] drm/i915: Expose latest 200 CRC value for pipe through debugfs
From: Shuang He shuang...@intel.com
There are several points in the display pipeline where CRCs can be
computed on the bits flowing there. For instance, it's usually possible
to compute the CRCs of the primary plane, the sprite plane or the CRCs
of the bits after the panel fitter (collectively called pipe CRCs).
v2: Quite a bit of rework here and there (Damien)
Signed-off-by: Shuang He shuang...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 33 +
drivers/gpu/drm/i915/i915_drv.h | 16
drivers/gpu/drm/i915/i915_irq.c | 35 +++
drivers/gpu/drm/i915/i915_reg.h | 36 +++-
4 files changed, 119 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 72d0458..e1d45aa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1732,6 +1732,36 @@ static int i915_pc8_status(struct seq_file *m, void
*unused)
return 0;
}
+static int i915_pipe_crc(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ enum pipe pipe = (enum pipe)node-info_ent-data;
+ const struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
+ int i;
+ int start;
+
+ if (!IS_IVYBRIDGE(dev)) {
+ seq_puts(m, unsupported\n);
+ return 0;
+ }
+
+ start = atomic_read(pipe_crc-slot) + 1;
+ seq_puts(m, timestamp CRC1 CRC2 CRC3 CRC4
CRC5\n);
+ for (i = 0; i INTEL_PIPE_CRC_ENTRIES_NR; i++) {
+ const struct intel_pipe_crc_entry *entry =
+ pipe_crc-entries[(start + i) %
+ INTEL_PIPE_CRC_ENTRIES_NR];
+
+ seq_printf(m, %12u %8x %8x %8x %8x %8x\n, entry-timestamp,
+ entry-crc[0], entry-crc[1], entry-crc[2],
+ entry-crc[3], entry-crc[4]);
+ }
+
+ return 0;
+}
+
static int
i915_wedged_get(void *data, u64 *val)
{
@@ -2247,6 +2277,9 @@ static struct drm_info_list i915_debugfs_list[] = {
{i915_edp_psr_status, i915_edp_psr_status, 0},
{i915_energy_uJ, i915_energy_uJ, 0},
{i915_pc8_status, i915_pc8_status, 0},
+ {i915_pipe_A_crc, i915_pipe_crc, 0, (void *)PIPE_A},
+ {i915_pipe_B_crc, i915_pipe_crc, 0, (void *)PIPE_B},
+ {i915_pipe_C_crc, i915_pipe_crc, 0, (void *)PIPE_C},
};
#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6106d3d..6855d91 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1219,6 +1219,18 @@ struct i915_package_c8 {
} regsave;
};
+struct intel_pipe_crc_entry {
+ uint32_t timestamp;
+ uint32_t crc[5];
+};
+
+#define INTEL_PIPE_CRC_ENTRIES_NR 200
+struct intel_pipe_crc {
+ struct intel_pipe_crc_entry entries[INTEL_PIPE_CRC_ENTRIES_NR];
+ enum intel_pipe_crc_source source;
+ atomic_t slot;
+};
+
typedef struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1423,6 +1435,10 @@ typedef struct drm_i915_private {
struct i915_dri1_state dri1;
/* Old ums support infrastructure, same warning applies. */
struct i915_ums_state ums;
+
+#ifdef CONFIG_DEBUG_FS
+ struct intel_pipe_crc pipe_crc[I915_MAX_PIPES];
+#endif
} drm_i915_private_t;
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 26753b6..d2074f1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1188,6 +1188,32 @@ static void dp_aux_irq_handler(struct drm_device *dev)
wake_up_all(dev_priv-gmbus_wait_queue);
}
+#if defined(CONFIG_DEBUG_FS)
+static void ivb_pipe_crc_update(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
+ struct intel_pipe_crc_entry *entry;
+ ktime_t now;
+ int ts, slot;
+
+ now = ktime_get();
+ ts = ktime_to_us(now);
+
+ slot = (atomic_read(pipe_crc-slot) + 1) % INTEL_PIPE_CRC_ENTRIES_NR;
+ entry = pipe_crc-entries[slot];
+ entry-timestamp = ts;
+ entry-crc[0] = I915_READ(PIPE_CRC_RES_1_IVB(pipe));
+ entry-crc[1] = I915_READ(PIPE_CRC_RES_2_IVB(pipe));
+ entry-crc[2] = I915_READ(PIPE_CRC_RES_3_IVB(pipe));
+ entry-crc[3] = I915_READ(PIPE_CRC_RES_4_IVB(pipe));
+ entry-crc[4] = I915_READ(PIPE_CRC_RES_5_IVB(pipe));
+
[Intel-gfx] [PATCH 02/16] drm/i915: Add a control file for pipe CRCs
Note the return -ENODEV; in pipe_crc_set_source(). The ctl file is
disabled until the end of the series to be able to do incremental
improvements.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 217 +++-
drivers/gpu/drm/i915/i915_drv.h | 8 ++
2 files changed, 223 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index e1d45aa..0d8a9a3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -27,6 +27,7 @@
*/
#include linux/seq_file.h
+#include linux/ctype.h
#include linux/debugfs.h
#include linux/slab.h
#include linux/export.h
@@ -1742,8 +1743,8 @@ static int i915_pipe_crc(struct seq_file *m, void *data)
int i;
int start;
- if (!IS_IVYBRIDGE(dev)) {
- seq_puts(m, unsupported\n);
+ if (dev_priv-pipe_crc[pipe].source == INTEL_PIPE_CRC_SOURCE_NONE) {
+ seq_puts(m, none\n);
return 0;
}
@@ -1762,6 +1763,217 @@ static int i915_pipe_crc(struct seq_file *m, void *data)
return 0;
}
+static const char *pipe_crc_sources[] = {
+ none,
+ plane1,
+ plane2,
+ pf,
+};
+
+static const char *pipe_crc_source_name(enum intel_pipe_crc_source source)
+{
+ BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX);
+ return pipe_crc_sources[source];
+}
+
+static int pipe_crc_ctl_show(struct seq_file *m, void *data)
+{
+ struct drm_device *dev = m-private;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int i;
+
+ for (i = 0; i I915_MAX_PIPES; i++)
+ seq_printf(m, %c %s\n, pipe_name(i),
+ pipe_crc_source_name(dev_priv-pipe_crc[i].source));
+
+ return 0;
+}
+
+static int pipe_crc_ctl_open(struct inode *inode, struct file *file)
+{
+ struct drm_device *dev = inode-i_private;
+
+ return single_open(file, pipe_crc_ctl_show, dev);
+}
+
+static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
+ enum intel_pipe_crc_source source)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 val;
+
+
+ return -ENODEV;
+
+ if (!IS_IVYBRIDGE(dev))
+ return -ENODEV;
+
+ dev_priv-pipe_crc[pipe].source = source;
+
+ switch (source) {
+ case INTEL_PIPE_CRC_SOURCE_PLANE1:
+ val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB;
+ break;
+ case INTEL_PIPE_CRC_SOURCE_PLANE2:
+ val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB;
+ break;
+ case INTEL_PIPE_CRC_SOURCE_PF:
+ val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB;
+ break;
+ case INTEL_PIPE_CRC_SOURCE_NONE:
+ default:
+ val = 0;
+ break;
+ }
+
+ I915_WRITE(PIPE_CRC_CTL(pipe), val);
+ POSTING_READ(PIPE_CRC_CTL(pipe));
+
+ return 0;
+}
+
+/*
+ * Parse pipe CRC command strings:
+ * command: wsp* pipe wsp+ source wsp*
+ * pipe: (A | B | C)
+ * source: (none | plane1 | plane2 | pf)
+ * wsp: (#0x20 | #0x9 | #0xA)+
+ *
+ * eg.:
+ * A plane1 - Start CRC computations on plane1 of pipe A
+ * A none- Stop CRC
+ */
+static int pipe_crc_ctl_tokenize(char *buf, char *words[], int max_words)
+{
+ int n_words = 0;
+
+ while (*buf) {
+ char *end;
+
+ /* skip leading white space */
+ buf = skip_spaces(buf);
+ if (!*buf)
+ break; /* end of buffer */
+
+ /* find end of word */
+ for (end = buf; *end !isspace(*end); end++)
+ ;
+
+ if (n_words == max_words) {
+ DRM_DEBUG_DRIVER(too many words, allowed = %d\n,
+max_words);
+ return -EINVAL; /* ran out of words[] before bytes */
+ }
+
+ if (*end)
+ *end++ = '\0';
+ words[n_words++] = buf;
+ buf = end;
+ }
+
+ return n_words;
+}
+
+static int pipe_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe)
+{
+ const char name = buf[0];
+
+ if (name 'A' || name = pipe_name(I915_MAX_PIPES))
+ return -EINVAL;
+
+ *pipe = name - 'A';
+
+ return 0;
+}
+
+static int
+pipe_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *source)
+{
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(pipe_crc_sources); i++)
+ if (!strcmp(buf, pipe_crc_sources[i])) {
+ *source = i;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int pipe_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len)
+{
+#define MAX_WORDS 2
+ int
[Intel-gfx] [PATCH 04/16] drm/i915: Sample the frame counter instead of a timestamp for CRCs
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 8 ++--
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 991abff..58c6fd4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1748,14 +1748,14 @@ static int i915_pipe_crc(struct seq_file *m, void *data)
return 0;
}
- seq_puts(m, timestamp CRC1 CRC2 CRC3 CRC4
CRC5\n);
+ seq_puts(m, frameCRC1 CRC2 CRC3 CRC4 CRC5\n);
head = atomic_read(pipe_crc-head);
tail = atomic_read(pipe_crc-tail);
while (CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) = 1) {
struct intel_pipe_crc_entry *entry = pipe_crc-entries[tail];
- seq_printf(m, %12u %8x %8x %8x %8x %8x\n, entry-timestamp,
+ seq_printf(m, %8u %8x %8x %8x %8x %8x\n, entry-frame,
entry-crc[0], entry-crc[1], entry-crc[2],
entry-crc[3], entry-crc[4]);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7a1ed3a..cd87919 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1228,7 +1228,7 @@ enum intel_pipe_crc_source {
};
struct intel_pipe_crc_entry {
- uint32_t timestamp;
+ uint32_t frame;
uint32_t crc[5];
};
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 73d76af..0b21828 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1195,8 +1195,7 @@ static void ivb_pipe_crc_update(struct drm_device *dev,
enum pipe pipe)
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
- ktime_t now;
- int ts, head, tail;
+ int head, tail;
head = atomic_read(pipe_crc-head);
tail = atomic_read(pipe_crc-tail);
@@ -1208,10 +1207,7 @@ static void ivb_pipe_crc_update(struct drm_device *dev,
enum pipe pipe)
entry = pipe_crc-entries[head];
- now = ktime_get();
- ts = ktime_to_us(now);
-
- entry-timestamp = ts;
+ entry-frame = I915_READ(PIPEFRAME(pipe));
entry-crc[0] = I915_READ(PIPE_CRC_RES_1_IVB(pipe));
entry-crc[1] = I915_READ(PIPE_CRC_RES_2_IVB(pipe));
entry-crc[2] = I915_READ(PIPE_CRC_RES_3_IVB(pipe));
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/16] drm/i915: Keep the CRC values into a circular buffer
There are a few good properties to a circular buffer, for instance it
has a number of entries (before we were always dumping the full buffer).
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 20
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_irq.c | 19 +++
3 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0d8a9a3..991abff 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -27,6 +27,7 @@
*/
#include linux/seq_file.h
+#include linux/circ_buf.h
#include linux/ctype.h
#include linux/debugfs.h
#include linux/slab.h
@@ -1739,25 +1740,28 @@ static int i915_pipe_crc(struct seq_file *m, void *data)
struct drm_device *dev = node-minor-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
enum pipe pipe = (enum pipe)node-info_ent-data;
- const struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
- int i;
- int start;
+ struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
+ int head, tail;
if (dev_priv-pipe_crc[pipe].source == INTEL_PIPE_CRC_SOURCE_NONE) {
seq_puts(m, none\n);
return 0;
}
- start = atomic_read(pipe_crc-slot) + 1;
seq_puts(m, timestamp CRC1 CRC2 CRC3 CRC4
CRC5\n);
- for (i = 0; i INTEL_PIPE_CRC_ENTRIES_NR; i++) {
- const struct intel_pipe_crc_entry *entry =
- pipe_crc-entries[(start + i) %
- INTEL_PIPE_CRC_ENTRIES_NR];
+ head = atomic_read(pipe_crc-head);
+ tail = atomic_read(pipe_crc-tail);
+
+ while (CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) = 1) {
+ struct intel_pipe_crc_entry *entry = pipe_crc-entries[tail];
seq_printf(m, %12u %8x %8x %8x %8x %8x\n, entry-timestamp,
entry-crc[0], entry-crc[1], entry-crc[2],
entry-crc[3], entry-crc[4]);
+
+ BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
+ tail = (tail + 1) (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+ atomic_set(pipe_crc-tail, tail);
}
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0d564eb..7a1ed3a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1232,11 +1232,11 @@ struct intel_pipe_crc_entry {
uint32_t crc[5];
};
-#define INTEL_PIPE_CRC_ENTRIES_NR 200
+#define INTEL_PIPE_CRC_ENTRIES_NR 128
struct intel_pipe_crc {
struct intel_pipe_crc_entry entries[INTEL_PIPE_CRC_ENTRIES_NR];
enum intel_pipe_crc_source source;
- atomic_t slot;
+ atomic_t head, tail;
};
typedef struct drm_i915_private {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d2074f1..73d76af 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -30,6 +30,7 @@
#include linux/sysrq.h
#include linux/slab.h
+#include linux/circ_buf.h
#include drm/drmP.h
#include drm/i915_drm.h
#include i915_drv.h
@@ -1195,20 +1196,30 @@ static void ivb_pipe_crc_update(struct drm_device *dev,
enum pipe pipe)
struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
ktime_t now;
- int ts, slot;
+ int ts, head, tail;
+
+ head = atomic_read(pipe_crc-head);
+ tail = atomic_read(pipe_crc-tail);
+
+ if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) 1) {
+ DRM_ERROR(CRC buffer overflowing\n);
+ return;
+ }
+
+ entry = pipe_crc-entries[head];
now = ktime_get();
ts = ktime_to_us(now);
- slot = (atomic_read(pipe_crc-slot) + 1) % INTEL_PIPE_CRC_ENTRIES_NR;
- entry = pipe_crc-entries[slot];
entry-timestamp = ts;
entry-crc[0] = I915_READ(PIPE_CRC_RES_1_IVB(pipe));
entry-crc[1] = I915_READ(PIPE_CRC_RES_2_IVB(pipe));
entry-crc[2] = I915_READ(PIPE_CRC_RES_3_IVB(pipe));
entry-crc[3] = I915_READ(PIPE_CRC_RES_4_IVB(pipe));
entry-crc[4] = I915_READ(PIPE_CRC_RES_5_IVB(pipe));
- atomic_set(dev_priv-pipe_crc[pipe].slot, slot);
+
+ head = (head + 1) (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+ atomic_set(pipe_crc-head, head);
}
#else
static void ivb_pipe_crc_update(struct drm_device *dev, int pipe) {}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/16] drm/i915: Make switching to the same CRC source a no-op
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 58c6fd4..8c750d5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1804,6 +1804,7 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
enum intel_pipe_crc_source source)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
u32 val;
@@ -1812,7 +1813,10 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
if (!IS_IVYBRIDGE(dev))
return -ENODEV;
- dev_priv-pipe_crc[pipe].source = source;
+ if (pipe_crc-source == source)
+ return 0;
+
+ pipe_crc-source = source;
switch (source) {
case INTEL_PIPE_CRC_SOURCE_PLANE1:
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/16] drm/i915: Empty the circular buffer when asked for a new source
So we don't read out stale CRCs from a previous run left in the buffer.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 787c50d..ec9151a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1820,6 +1820,12 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
if (pipe_crc-source source)
return -EINVAL;
+ /* none - real source transition */
+ if (source) {
+ atomic_set(pipe_crc-head, 0);
+ atomic_set(pipe_crc-tail, 0);
+ }
+
pipe_crc-source = source;
switch (source) {
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/16] drm/i915: Generalize the CRC command format for future work
Let's move from writing 'A plane1' to 'pipe A plane1' to
i915_pipe_crc_ctl. This will allow us to extend the interface to
transcoders or DDIs in the future.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 56 -
1 file changed, 43 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 53a3f22..c609783 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1864,14 +1864,15 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
/*
* Parse pipe CRC command strings:
- * command: wsp* pipe wsp+ source wsp*
- * pipe: (A | B | C)
+ * command: wsp* object wsp+ name wsp+ source wsp*
+ * object: 'pipe'
+ * name: (A | B | C)
* source: (none | plane1 | plane2 | pf)
* wsp: (#0x20 | #0x9 | #0xA)+
*
* eg.:
- * A plane1 - Start CRC computations on plane1 of pipe A
- * A none- Stop CRC
+ * pipe A plane1 - Start CRC computations on plane1 of pipe A
+ * pipe A none- Stop CRC
*/
static int pipe_crc_ctl_tokenize(char *buf, char *words[], int max_words)
{
@@ -1904,6 +1905,28 @@ static int pipe_crc_ctl_tokenize(char *buf, char
*words[], int max_words)
return n_words;
}
+enum intel_pipe_crc_object {
+ PIPE_CRC_OBJECT_PIPE,
+};
+
+static const char *pipe_crc_objects[] = {
+ pipe,
+};
+
+static int
+pipe_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *object)
+{
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(pipe_crc_objects); i++)
+ if (!strcmp(buf, pipe_crc_objects[i])) {
+ *object = i;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
static int pipe_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe)
{
const char name = buf[0];
@@ -1932,25 +1955,32 @@ pipe_crc_ctl_parse_source(const char *buf, enum
intel_pipe_crc_source *source)
static int pipe_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len)
{
-#define MAX_WORDS 2
+#define N_WORDS 3
int n_words;
- char *words[MAX_WORDS];
+ char *words[N_WORDS];
enum pipe pipe;
+ enum intel_pipe_crc_object object;
enum intel_pipe_crc_source source;
- n_words = pipe_crc_ctl_tokenize(buf, words, MAX_WORDS);
- if (n_words != 2) {
- DRM_DEBUG_DRIVER(tokenize failed, a command is 2 words\n);
+ n_words = pipe_crc_ctl_tokenize(buf, words, N_WORDS);
+ if (n_words != N_WORDS) {
+ DRM_DEBUG_DRIVER(tokenize failed, a command is %d words\n,
+N_WORDS);
+ return -EINVAL;
+ }
+
+ if (pipe_crc_ctl_parse_object(words[0], object) 0) {
+ DRM_DEBUG_DRIVER(unknown object %s\n, words[0]);
return -EINVAL;
}
- if (pipe_crc_ctl_parse_pipe(words[0], pipe) 0) {
- DRM_DEBUG_DRIVER(unknown pipe %s\n, words[0]);
+ if (pipe_crc_ctl_parse_pipe(words[1], pipe) 0) {
+ DRM_DEBUG_DRIVER(unknown pipe %s\n, words[1]);
return -EINVAL;
}
- if (pipe_crc_ctl_parse_source(words[1], source) 0) {
- DRM_DEBUG_DRIVER(unknown source %s\n, words[1]);
+ if (pipe_crc_ctl_parse_source(words[2], source) 0) {
+ DRM_DEBUG_DRIVER(unknown source %s\n, words[2]);
return -EINVAL;
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/16] drm/i915: Enforce going back to none before changing CRC source
This way we can have some init/fini code on those transitions.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 8c750d5..787c50d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1816,6 +1816,10 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
if (pipe_crc-source == source)
return 0;
+ /* forbid changing the source without going back to 'none' */
+ if (pipe_crc-source source)
+ return -EINVAL;
+
pipe_crc-source = source;
switch (source) {
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/16] drm/i915: Dynamically allocate the CRC circular buffer
So we don't eat that memory when not needed.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 12
drivers/gpu/drm/i915/i915_drv.h | 2 +-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index ec9151a..53a3f22 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1822,6 +1822,12 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
/* none - real source transition */
if (source) {
+ pipe_crc-entries = kzalloc(sizeof(*pipe_crc-entries) *
+ INTEL_PIPE_CRC_ENTRIES_NR,
+ GFP_KERNEL);
+ if (!pipe_crc-entries)
+ return -ENOMEM;
+
atomic_set(pipe_crc-head, 0);
atomic_set(pipe_crc-tail, 0);
}
@@ -1847,6 +1853,12 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
I915_WRITE(PIPE_CRC_CTL(pipe), val);
POSTING_READ(PIPE_CRC_CTL(pipe));
+ /* real source - none transition */
+ if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
+ kfree(pipe_crc-entries);
+ pipe_crc-entries = NULL;
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cd87919..5171a69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1234,7 +1234,7 @@ struct intel_pipe_crc_entry {
#define INTEL_PIPE_CRC_ENTRIES_NR 128
struct intel_pipe_crc {
- struct intel_pipe_crc_entry entries[INTEL_PIPE_CRC_ENTRIES_NR];
+ struct intel_pipe_crc_entry *entries;
enum intel_pipe_crc_source source;
atomic_t head, tail;
};
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/16] drm/i915: Warn if we receive an interrupt after freeing the buffer
This shouldn't happen as the buffer is freed after disable pipe CRCs,
but better be safe than sorry.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0b21828..b201a21 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1197,6 +1197,11 @@ static void ivb_pipe_crc_update(struct drm_device *dev,
enum pipe pipe)
struct intel_pipe_crc_entry *entry;
int head, tail;
+ if (!pipe_crc-entries) {
+ DRM_ERROR(spurious interrupt\n);
+ return;
+ }
+
head = atomic_read(pipe_crc-head);
tail = atomic_read(pipe_crc-tail);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/16] drm/i915: Rename i915_pipe_crc_ctl to i915_display_crc_ctl
In the same spirit than:
drm/i915: Generalize the CRC command format for future work
Let's move from writing 'A plane1' to 'pipe A plane1' to
i915_pipe_crc_ctl. This will allow us to extend the interface to
transcoders or DDIs in the future.
Let's rename the CRC control file to be more generic.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 42 ++---
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index c609783..471c258 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1780,7 +1780,7 @@ static const char *pipe_crc_source_name(enum
intel_pipe_crc_source source)
return pipe_crc_sources[source];
}
-static int pipe_crc_ctl_show(struct seq_file *m, void *data)
+static int display_crc_ctl_show(struct seq_file *m, void *data)
{
struct drm_device *dev = m-private;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1793,11 +1793,11 @@ static int pipe_crc_ctl_show(struct seq_file *m, void
*data)
return 0;
}
-static int pipe_crc_ctl_open(struct inode *inode, struct file *file)
+static int display_crc_ctl_open(struct inode *inode, struct file *file)
{
struct drm_device *dev = inode-i_private;
- return single_open(file, pipe_crc_ctl_show, dev);
+ return single_open(file, display_crc_ctl_show, dev);
}
static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
@@ -1874,7 +1874,7 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
* pipe A plane1 - Start CRC computations on plane1 of pipe A
* pipe A none- Stop CRC
*/
-static int pipe_crc_ctl_tokenize(char *buf, char *words[], int max_words)
+static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words)
{
int n_words = 0;
@@ -1914,20 +1914,20 @@ static const char *pipe_crc_objects[] = {
};
static int
-pipe_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *object)
+display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o)
{
int i;
for (i = 0; i ARRAY_SIZE(pipe_crc_objects); i++)
if (!strcmp(buf, pipe_crc_objects[i])) {
- *object = i;
+ *o = i;
return 0;
}
return -EINVAL;
}
-static int pipe_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe)
+static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe)
{
const char name = buf[0];
@@ -1940,20 +1940,20 @@ static int pipe_crc_ctl_parse_pipe(const char *buf,
enum pipe *pipe)
}
static int
-pipe_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *source)
+display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
{
int i;
for (i = 0; i ARRAY_SIZE(pipe_crc_sources); i++)
if (!strcmp(buf, pipe_crc_sources[i])) {
- *source = i;
+ *s = i;
return 0;
}
return -EINVAL;
}
-static int pipe_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len)
+static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len)
{
#define N_WORDS 3
int n_words;
@@ -1962,24 +1962,24 @@ static int pipe_crc_ctl_parse(struct drm_device *dev,
char *buf, size_t len)
enum intel_pipe_crc_object object;
enum intel_pipe_crc_source source;
- n_words = pipe_crc_ctl_tokenize(buf, words, N_WORDS);
+ n_words = display_crc_ctl_tokenize(buf, words, N_WORDS);
if (n_words != N_WORDS) {
DRM_DEBUG_DRIVER(tokenize failed, a command is %d words\n,
N_WORDS);
return -EINVAL;
}
- if (pipe_crc_ctl_parse_object(words[0], object) 0) {
+ if (display_crc_ctl_parse_object(words[0], object) 0) {
DRM_DEBUG_DRIVER(unknown object %s\n, words[0]);
return -EINVAL;
}
- if (pipe_crc_ctl_parse_pipe(words[1], pipe) 0) {
+ if (display_crc_ctl_parse_pipe(words[1], pipe) 0) {
DRM_DEBUG_DRIVER(unknown pipe %s\n, words[1]);
return -EINVAL;
}
- if (pipe_crc_ctl_parse_source(words[2], source) 0) {
+ if (display_crc_ctl_parse_source(words[2], source) 0) {
DRM_DEBUG_DRIVER(unknown source %s\n, words[2]);
return -EINVAL;
}
@@ -1987,8 +1987,8 @@ static int pipe_crc_ctl_parse(struct drm_device *dev,
char *buf, size_t len)
return pipe_crc_set_source(dev, pipe, source);
}
-static ssize_t pipe_crc_ctl_write(struct file *file, const char __user *ubuf,
- size_t len, loff_t *offp)
+static ssize_t
[Intel-gfx] [PATCH 12/16] drm/i915: Add log messages when CRCs collection is started/stopped
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 471c258..dee85d7 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1822,6 +1822,9 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
/* none - real source transition */
if (source) {
+ DRM_DEBUG_DRIVER(collecting CRCs for pipe %c, %s\n,
+pipe_name(pipe), pipe_crc_source_name(source));
+
pipe_crc-entries = kzalloc(sizeof(*pipe_crc-entries) *
INTEL_PIPE_CRC_ENTRIES_NR,
GFP_KERNEL);
@@ -1855,6 +1858,9 @@ static int pipe_crc_set_source(struct drm_device *dev,
enum pipe pipe,
/* real source - none transition */
if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
+ DRM_DEBUG_DRIVER(stopping CRCs for pipe %c\n,
+pipe_name(pipe));
+
kfree(pipe_crc-entries);
pipe_crc-entries = NULL;
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/16] drm/i915: Move drm_add_fake_info_node() higher in the file
Following commit needs drm_add_fake_info_node() higher in the file to
avoid having a forward declaration. Move this helper near the top of the
file.
This also makes the next commit diff a bit easier to review.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 52 ++---
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index dee85d7..baa2e43 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -53,6 +53,32 @@ static const char *yesno(int v)
return v ? yes : no;
}
+/* As the drm_debugfs_init() routines are called before dev-dev_private is
+ * allocated we need to hook into the minor for release. */
+static int
+drm_add_fake_info_node(struct drm_minor *minor,
+ struct dentry *ent,
+ const void *key)
+{
+ struct drm_info_node *node;
+
+ node = kmalloc(sizeof(*node), GFP_KERNEL);
+ if (node == NULL) {
+ debugfs_remove(ent);
+ return -ENOMEM;
+ }
+
+ node-minor = minor;
+ node-dent = ent;
+ node-info_ent = (void *) key;
+
+ mutex_lock(minor-debugfs_lock);
+ list_add(node-list, minor-debugfs_list);
+ mutex_unlock(minor-debugfs_lock);
+
+ return 0;
+}
+
static int i915_capabilities(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m-private;
@@ -2425,32 +2451,6 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
i915_cache_sharing_get, i915_cache_sharing_set,
%llu\n);
-/* As the drm_debugfs_init() routines are called before dev-dev_private is
- * allocated we need to hook into the minor for release. */
-static int
-drm_add_fake_info_node(struct drm_minor *minor,
- struct dentry *ent,
- const void *key)
-{
- struct drm_info_node *node;
-
- node = kmalloc(sizeof(*node), GFP_KERNEL);
- if (node == NULL) {
- debugfs_remove(ent);
- return -ENOMEM;
- }
-
- node-minor = minor;
- node-dent = ent;
- node-info_ent = (void *) key;
-
- mutex_lock(minor-debugfs_lock);
- list_add(node-list, minor-debugfs_list);
- mutex_unlock(minor-debugfs_lock);
-
- return 0;
-}
-
static int i915_forcewake_open(struct inode *inode, struct file *file)
{
struct drm_device *dev = inode-i_private;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/16] drm/i915: Implement blocking read for pipe CRC files
seq_file is not quite the right interface for these ones. We have a
circular buffer with a new entry per vblank on one side and a process
wanting to dequeue the CRC with a read().
It's quite racy to wait for vblank in user land and then try to read a
pipe_crc file, sometimes the CRC interrupt hasn't been fired and we end
up with an EOF.
So, let's have the read on the pipe_crc file block until the interrupt
gives us a new entry. At that point we can wake the reading process.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 164 +++-
drivers/gpu/drm/i915/i915_dma.c | 2 +
drivers/gpu/drm/i915/i915_drv.h | 6 ++
drivers/gpu/drm/i915/i915_irq.c | 2 +
4 files changed, 155 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index baa2e43..5137f8f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1760,37 +1760,138 @@ static int i915_pc8_status(struct seq_file *m, void
*unused)
return 0;
}
-static int i915_pipe_crc(struct seq_file *m, void *data)
+struct pipe_crc_info {
+ const char *name;
+ struct drm_device *dev;
+ enum pipe pipe;
+};
+
+static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
+{
+ filep-private_data = inode-i_private;
+
+ return 0;
+}
+
+static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
+{
+ return 0;
+}
+
+/* (6 fields, 8 chars each, space separated (5) + '\n') */
+#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
+/* account for \'0' */
+#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1)
+
+static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
{
- struct drm_info_node *node = (struct drm_info_node *) m-private;
- struct drm_device *dev = node-minor-dev;
- struct drm_i915_private *dev_priv = dev-dev_private;
- enum pipe pipe = (enum pipe)node-info_ent-data;
- struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
int head, tail;
- if (dev_priv-pipe_crc[pipe].source == INTEL_PIPE_CRC_SOURCE_NONE) {
- seq_puts(m, none\n);
+ head = atomic_read(pipe_crc-head);
+ tail = atomic_read(pipe_crc-tail);
+
+ return CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR);
+}
+
+static ssize_t
+i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
+ loff_t *pos)
+{
+ struct pipe_crc_info *info = filep-private_data;
+ struct drm_device *dev = info-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[info-pipe];
+ char buf[PIPE_CRC_BUFFER_LEN];
+ int head, tail, n_entries, n;
+ ssize_t bytes_read;
+
+ /*
+* Don't allow user space to provide buffers not big enough to hold
+* a line of data.
+*/
+ if (count PIPE_CRC_LINE_LEN)
+ return -EINVAL;
+
+ if (pipe_crc-source == INTEL_PIPE_CRC_SOURCE_NONE)
return 0;
+
+ /* nothing to read */
+ while (pipe_crc_data_count(pipe_crc) == 0) {
+ if (filep-f_flags O_NONBLOCK)
+ return -EAGAIN;
+
+ if (wait_event_interruptible(pipe_crc-wq,
+pipe_crc_data_count(pipe_crc)))
+return -ERESTARTSYS;
}
- seq_puts(m, frameCRC1 CRC2 CRC3 CRC4 CRC5\n);
+ /* We now have one or more entries to read */
head = atomic_read(pipe_crc-head);
tail = atomic_read(pipe_crc-tail);
-
- while (CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) = 1) {
+ n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR),
+ count / PIPE_CRC_LINE_LEN);
+ bytes_read = 0;
+ n = 0;
+ do {
struct intel_pipe_crc_entry *entry = pipe_crc-entries[tail];
+ int ret;
- seq_printf(m, %8u %8x %8x %8x %8x %8x\n, entry-frame,
- entry-crc[0], entry-crc[1], entry-crc[2],
- entry-crc[3], entry-crc[4]);
+ bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN,
+ %8u %8x %8x %8x %8x %8x\n,
+ entry-frame, entry-crc[0],
+ entry-crc[1], entry-crc[2],
+ entry-crc[3], entry-crc[4]);
+
+ ret = copy_to_user(user_buf + n * PIPE_CRC_LINE_LEN,
+ buf, PIPE_CRC_LINE_LEN);
+ if (ret == PIPE_CRC_LINE_LEN)
+ return -EFAULT;
BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
tail = (tail + 1) (INTEL_PIPE_CRC_ENTRIES_NR - 1);
[Intel-gfx] [PATCH 16/16] drm/i915: Enable pipe CRCs
It's time to declare them ready. Unleash the beast. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 826ebce..d1674b6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1949,9 +1949,6 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe]; u32 val; - - return -ENODEV; - if (!IS_IVYBRIDGE(dev)) return -ENODEV; -- 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 v2] drm: add support for additional stereo 3D modes
On Tue, Oct 15, 2013 at 06:25:27PM +0100, Thomas Wood wrote:
Parse the 3D_Structure_ALL and 3D_MASK fields of the HDMI Vendor
Specific Data Block to expose more stereo 3D modes.
Daniel likes to have the v2,v3,etc. changes listed here in the commit
msg.
Signed-off-by: Thomas Wood thomas.w...@intel.com
---
drivers/gpu/drm/drm_edid.c | 105
+
1 file changed, 96 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e81609..456a694 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2652,6 +2652,50 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
return 1;
}
+static int add_3d_struct_modes(struct drm_connector *connector, u16
structure,
+const u8 *video_db, u8 video_len, u8 video_index)
+{
+ struct drm_device *dev = connector-dev;
+ struct drm_display_mode *newmode;
+ int modes = 0;
+ u8 cea_mode;
+
+ if (video_db == NULL || video_index video_len)
+ return 0;
+
+ /* CEA modes are numbered 1..127 */
+ cea_mode = (video_db[video_index] 127) - 1;
+ if (cea_mode = ARRAY_SIZE(edid_cea_modes))
+ return 0;
+
+ if (structure (1 0)) {
+ newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+ if (newmode) {
+ newmode-flags |= DRM_MODE_FLAG_3D_FRAME_PACKING;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+ if (structure (1 6)) {
+ newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+ if (newmode) {
+ newmode-flags |= DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+ if (structure (1 8)) {
+ newmode = drm_mode_duplicate(dev, edid_cea_modes[cea_mode]);
+ if (newmode) {
+ newmode-flags = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+ }
+
+ return modes;
+}
+
/*
* do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
* @connector: connector corresponding to the HDMI sink
@@ -2662,10 +2706,13 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
* also adds the stereo 3d modes when applicable.
*/
static int
-do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
+const u8 *video_db, u8 video_len)
{
- int modes = 0, offset = 0, i;
- u8 vic_len;
+ int modes = 0, offset = 0, i, multi_present = 0;
+ u8 vic_len, hdmi_3d_len;
+ u16 mask;
+ u16 structure_all;
if (len 8)
goto out;
@@ -2689,9 +2736,13 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len)
/* 3D_Present */
offset++;
- if (db[8 + offset] (1 7))
+ if (db[8 + offset] (1 7)) {
modes += add_hdmi_mandatory_stereo_modes(connector);
+ /* 3D_Multi_present */
+ multi_present = (db[8 + offset] 0x60) 5;
+ }
+
offset++;
vic_len = db[8 + offset] 5;
@@ -2702,6 +2753,38 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len)
modes += add_hdmi_mode(connector, vic);
}
+ if (!(multi_present == 1 || multi_present == 2))
+ goto out;
+
+ if ((multi_present == 1 len (9 + offset)) ||
+ (multi_present == 2 len (11 + offset)))
+ goto out;
These checks must happen after 'offset += 1 + vic_len'.
Otherwise it looks good to me.
+
+ hdmi_3d_len = db[8 + offset] 0x1f;
+
+ if ((multi_present == 1 hdmi_3d_len 2) ||
+ (multi_present == 2 hdmi_3d_len 4))
+ goto out;
+
+ offset += 1 + vic_len;
+
+ /* 3D_Structure_ALL */
+ structure_all = (db[8 + offset] 8) | db[9 + offset];
+
+ /* check if 3D_MASK is present */
+ if (multi_present == 2)
+ mask = (db[10 + offset] 8) | db[11 + offset];
+ else
+ mask = 0x;
+
+ for (i = 0; i 16; i++) {
+ if (mask (1 i))
+ modes += add_3d_struct_modes(connector,
+ structure_all,
+ video_db,
+ video_len, i);
+ }
+
out:
return modes;
}
@@ -2759,8 +2842,8 @@ static int
add_cea_modes(struct drm_connector *connector, struct edid *edid)
{
const u8 *cea = drm_find_cea_extension(edid);
-
[Intel-gfx] [PATCH 15/16] drm/i915: Only one open() allowed on pipe CRC result files
It doesn't really make sense to have two processes dequeueing the CRC
values at the same time. Forbid that usage.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 16
drivers/gpu/drm/i915/i915_drv.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 5137f8f..826ebce 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1768,6 +1768,15 @@ struct pipe_crc_info {
static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
{
+ struct pipe_crc_info *info = inode-i_private;
+ struct drm_i915_private *dev_priv = info-dev-dev_private;
+ struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[info-pipe];
+
+ if (!atomic_dec_and_test(pipe_crc-available)) {
+ atomic_inc(pipe_crc-available);
+ return -EBUSY; /* already open */
+ }
+
filep-private_data = inode-i_private;
return 0;
@@ -1775,6 +1784,12 @@ static int i915_pipe_crc_open(struct inode *inode,
struct file *filep)
static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
{
+ struct pipe_crc_info *info = inode-i_private;
+ struct drm_i915_private *dev_priv = info-dev-dev_private;
+ struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[info-pipe];
+
+ atomic_inc(pipe_crc-available); /* release the device */
+
return 0;
}
@@ -2684,6 +2699,7 @@ void intel_display_crc_init(struct drm_device *dev)
for (i = 0; i INTEL_INFO(dev)-num_pipes; i++) {
struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[i];
+ atomic_set(pipe_crc-available, 1);
init_waitqueue_head(pipe_crc-wq);
}
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e16cf1e..3206358 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1234,6 +1234,7 @@ struct intel_pipe_crc_entry {
#define INTEL_PIPE_CRC_ENTRIES_NR 128
struct intel_pipe_crc {
+ atomic_t available; /* exclusive access to the device */
struct intel_pipe_crc_entry *entries;
enum intel_pipe_crc_source source;
atomic_t head, tail;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use pipe_name() instead of the pipe number
On Tue, Oct 15, 2013 at 02:45:43PM +0100, Damien Lespiau wrote: Yet another direct usage of the pipe number instead of pipe_name(). We've been tracking them lately but managed to miss this one. Signed-off-by: Damien Lespiau damien.lesp...@intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com I actually see two more in intel_dsi.c and intel_panel.c. --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b9cede7..81f7fa5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10734,11 +10734,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) } encoder-connectors_active = false; - DRM_DEBUG_KMS([ENCODER:%d:%s] hw state readout: %s, pipe=%i\n, + DRM_DEBUG_KMS([ENCODER:%d:%s] hw state readout: %s, pipe %c\n, encoder-base.base.id, drm_get_encoder_name(encoder-base), encoder-base.crtc ? enabled : disabled, - pipe); + pipe_name(pipe)); } list_for_each_entry(connector, dev-mode_config.connector_list, -- 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] [RFC] Runtime display PM for VLV/BYT
On Tue, 2013-10-15 at 09:23 -0700, Jesse Barnes wrote:
On Tue, 15 Oct 2013 15:16:11 +0300
Imre Deak imre.d...@intel.com wrote:
On Tue, 2013-10-15 at 11:06 +0300, Ville Syrjälä wrote:
On Mon, Oct 14, 2013 at 04:07:44PM -0700, Jesse Barnes wrote:
This set adds bits needed for runtime power support, currently only
lightly tested on VLV/BYT:
1) suspend/resume callbacks for different platforms
2) save/restore of display state across a power well toggle
3) get/put of display power well in critical places
The TODO list still has a few items on it, and I'm looking for feedback:
1) sprinkle around some power well WARNs so we can catch things easily
2) add some tests using DPMS and NULL mode sets and comparing power
well state
3) better debugfs support for multiple wells
4) refcount of power well in debugfs (with ref holders?)
5) more testing - I think the load time ref is still busted here and
on HSW
6) convert HSW as well so DPMS will shut things down, not just mode
sets
Thoughts or comments?
I'd also like to see what Imre cooked up, and then come up with some
grand unified design. Based on our discussions I think his power well
abstraction sounded somewhat nicer and more general.
I've pushed what I have so far to:
https://github.com/ideak/linux/commits/powerwells
I've tested this on VLV with VGA output so far and somewhat on HSW. I'd
still have to check the need to do any HW state save/restore and the GFX
clock forcing, afaics Jesse has already code for these in his patchset.
I think a few can be pushed upstream now:
drm/i915: change power_well-lock to be mutex
drm/i915: factor out is_always_on_domain
drm/i915: factor out reset_vblank_counter
drm/i915: fix HAS_POWER_WELL-IS_HASWELL - pushed this but it doesn't
look like Daniel has picked it up yet
drm/i915: check powerwell in get_pipe_config
Ok, I can send the missing ones to the list.
drm/i915: enable only the needed power domains during modeset
Looks good too, and I guess there's no harm in pushing the earlier
patch to move the get of the power wells out of the HSW global res
function, even if we do move to get/put later.
Agreed, with the above we'd preserve the current behavior and later
things can be fine grained.
For drm/i915: support for multiple power wells, I think I prefer a
single uncore function taking a power well, rather than an array of
power_well structs. If we went that direction, we could probably
rename the power_well variable to audio_power_wells or something and
just track the ones we need to expose to the audio driver there, rather
than everything. That might be a little clearer than what we have
today, but I guess I'd have to see it coded to be sure.
I added the array mainly so that we can ref count each power well
separately. Currently this isn't a problem as we only need to refcount a
single power well, but on future HW with more of those we need to keep
the ref counters somewhere. I also store the mask of domains for which
we need the given power well there and having a vtable for Gen dependent
HW accessors looked like a nice way.
I agree the interface for requesting power wells for audio could be
better. We could add a new POWER_DOMAIN_AUDIO for that and map it to the
required power wells. But yea, this could be done later.
For drm/i915: add intel_encoder_get_hw_state, do you think it would be
clearer to take refs in the caller of the encoder-get_hw_state instead?
What would be the point, if all the callers would take a ref anyway? Or
do you want to take the reference once and do additional things besides
get_hw_state()?
Related to this: I made intel_encoder_get_hw_state() only check if the
power well is on and return false if it's not to indicate that the
encoder is off. I also thought of doing the same as you and take a ref
instead, not sure what's the right way. Maybe doing the readout only if
the power is on, but also making sure we have a reference in this case?
So with a new helper we'd have in intel_encoder_get_hw_state():
{
if (!intel_display_power_get_if_enabled(...))
return false;
... do the readout
intel_display_power_put(...)
}
I like drm/i915: get port power domains for connector detect and
drm/i915: add output power domains too, we'll need those if we want to
do fine grained output control on BYT and future chips. But they'll
probably need to be rebased on top of the above once it goes upstream.
Yea, we can reconsider them once we agree about the above.
Overall I don't think there's a ton of conflict between our patchsets,
they seem mostly complimentary and let me remove hacks in mine.
Ok, thanks a lot for the review,
Imre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/8] lib: Add a small helper to open debugfs files
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/Makefile.am | 2 ++
lib/igt_debugfs.c | 75 +++
lib/igt_debugfs.h | 36 ++
3 files changed, 113 insertions(+)
create mode 100644 lib/igt_debugfs.c
create mode 100644 lib/igt_debugfs.h
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 387141b..5710802 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -11,6 +11,8 @@ libintel_tools_la_SOURCES = \
i830_reg.h \
i915_3d.h \
i915_reg.h \
+ igt_debugfs.c \
+ igt_debugfs.h \
instdone.c \
instdone.h \
intel_batchbuffer.c \
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
new file mode 100644
index 000..33c4fc1
--- /dev/null
+++ b/lib/igt_debugfs.c
@@ -0,0 +1,75 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include sys/stat.h
+#include sys/mount.h
+#include errno.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+
+#include igt_debugfs.h
+
+int igt_debugfs_init(igt_debugfs_t *debugfs)
+{
+ const char *path = /sys/kernel/debug;
+ struct stat st;
+ int n;
+
+ if (stat(/debug/dri, st) == 0) {
+ path = /debug/dri;
+ goto find_minor;
+ }
+
+ if (stat(/sys/kernel/debug/dri, st) == 0)
+ goto find_minor;
+
+ if (stat(/sys/kernel/debug, st))
+ return errno;
+
+ if (mount(debug, /sys/kernel/debug, debugfs, 0, 0))
+ return errno;
+
+find_minor:
+ strcpy(debugfs-root, path);
+ for (n = 0; n 16; n++) {
+ int len = sprintf(debugfs-dri_path, %s/dri/%d, path, n);
+ sprintf(debugfs-dri_path + len, /i915_error_state);
+ if (stat(debugfs-dri_path, st) == 0) {
+ debugfs-dri_path[len] = '\0';
+ return 0;
+ }
+ }
+
+ debugfs-dri_path[0] = '\0';
+ return ENOENT;
+}
+
+int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename)
+{
+ char buf[1024];
+
+ sprintf(buf, %s/%s, debugfs-dri_path, filename);
+ return open(buf, O_RDONLY);
+}
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
new file mode 100644
index 000..aa9449a
--- /dev/null
+++ b/lib/igt_debugfs.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_DEBUGFS_H__
+#define __IGT_DEBUGFS_H__
+
+typedef struct {
+ char root[128];
+ char dri_path[128];
+} igt_debugfs_t;
+
+int igt_debugfs_init(igt_debugfs_t *debugfs);
+int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename);
[Intel-gfx] [PATCH 5/8] lib: Add a igt_display.h with a few enums and defines from the kernel
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/Makefile.am | 1 +
lib/igt_display.h | 55 +++
2 files changed, 56 insertions(+)
create mode 100644 lib/igt_display.h
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 5710802..06d406f 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -13,6 +13,7 @@ libintel_tools_la_SOURCES = \
i915_reg.h \
igt_debugfs.c \
igt_debugfs.h \
+ igt_display.h \
instdone.c \
instdone.h \
intel_batchbuffer.c \
diff --git a/lib/igt_display.h b/lib/igt_display.h
new file mode 100644
index 000..22c8a9f
--- /dev/null
+++ b/lib/igt_display.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_DISPLAY_H__
+#define __IGT_DISPLAY_H__
+
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+PIPE_C,
+I915_MAX_PIPES
+};
+#define pipe_name(p) ((p) + 'A')
+
+enum plane {
+PLANE_A = 0,
+PLANE_B,
+PLANE_C,
+};
+#define plane_name(p) ((p) + 'A')
+
+#define sprite_name(p, s) ((p) * dev_priv-num_plane + (s) + 'A')
+
+enum port {
+PORT_A = 0,
+PORT_B,
+PORT_C,
+PORT_D,
+PORT_E,
+I915_MAX_PORTS
+};
+#define port_name(p) ((p) + 'A')
+
+#endif /* __IGT_DISPLAY_H__ */
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/8] lib: Add igt_wait_for_vblank() helper
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/Makefile.am | 1 +
lib/{igt_display.h = igt_display.c} | 38
lib/igt_display.h| 2 ++
3 files changed, 15 insertions(+), 26 deletions(-)
copy lib/{igt_display.h = igt_display.c} (68%)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 06d406f..431fd93 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -13,6 +13,7 @@ libintel_tools_la_SOURCES = \
i915_reg.h \
igt_debugfs.c \
igt_debugfs.h \
+ igt_display.c \
igt_display.h \
instdone.c \
instdone.h \
diff --git a/lib/igt_display.h b/lib/igt_display.c
similarity index 68%
copy from lib/igt_display.h
copy to lib/igt_display.c
index 22c8a9f..28e21e6 100644
--- a/lib/igt_display.h
+++ b/lib/igt_display.c
@@ -22,34 +22,20 @@
*
*/
-#ifndef __IGT_DISPLAY_H__
-#define __IGT_DISPLAY_H__
+#include string.h
-enum pipe {
-PIPE_A = 0,
-PIPE_B,
-PIPE_C,
-I915_MAX_PIPES
-};
-#define pipe_name(p) ((p) + 'A')
+#include drmtest.h
+#include igt_display.h
-enum plane {
-PLANE_A = 0,
-PLANE_B,
-PLANE_C,
-};
-#define plane_name(p) ((p) + 'A')
+void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
+{
+ drmVBlank wait_vbl;
-#define sprite_name(p, s) ((p) * dev_priv-num_plane + (s) + 'A')
+ memset(wait_vbl, 0, sizeof(wait_vbl));
-enum port {
-PORT_A = 0,
-PORT_B,
-PORT_C,
-PORT_D,
-PORT_E,
-I915_MAX_PORTS
-};
-#define port_name(p) ((p) + 'A')
+ wait_vbl.request.type = pipe DRM_VBLANK_HIGH_CRTC_SHIFT |
+ DRM_VBLANK_RELATIVE;
+ wait_vbl.request.sequence = 1;
-#endif /* __IGT_DISPLAY_H__ */
+ igt_assert(drmWaitVBlank(drm_fd, wait_vbl) == 0);
+}
diff --git a/lib/igt_display.h b/lib/igt_display.h
index 22c8a9f..1357ce9 100644
--- a/lib/igt_display.h
+++ b/lib/igt_display.h
@@ -52,4 +52,6 @@ enum port {
};
#define port_name(p) ((p) + 'A')
+void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
+
#endif /* __IGT_DISPLAY_H__ */
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/8] lib: Add kmstest_paint_color()
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/drmtest.c | 8
lib/drmtest.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 2660af7..435a745 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -1347,6 +1347,14 @@ static int create_bo_for_fb(int fd, int width, int
height, int bpp,
return 0;
}
+void kmstest_paint_color(cairo_t *cr, int x, int y, int w, int h,
+double r, double g, double b)
+{
+ cairo_rectangle(cr, x, y, w, h);
+ cairo_set_source_rgb(cr, r, g, b);
+ cairo_fill(cr);
+}
+
void
kmstest_paint_color_gradient(cairo_t *cr, int x, int y, int w, int h,
int r, int g, int b)
diff --git a/lib/drmtest.h b/lib/drmtest.h
index f45780b..b7909df 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -359,6 +359,8 @@ unsigned int kmstest_create_fb2(int fd, int width, int
height, uint32_t format,
bool tiled, struct kmstest_fb *fb);
void kmstest_remove_fb(int fd, struct kmstest_fb *fb_info);
cairo_t *kmstest_get_cairo_ctx(int fd, struct kmstest_fb *fb);
+void kmstest_paint_color(cairo_t *cr, int x, int y, int w, int h,
+double r, double g, double b);
void kmstest_paint_color_gradient(cairo_t *cr, int x, int y, int w, int h,
int r, int g, int b);
void kmstest_paint_test_pattern(cairo_t *cr, int width, int height);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/8] lib: Add igt_debugfs_fopen()
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_debugfs.c | 9 +
lib/igt_debugfs.h | 4
2 files changed, 13 insertions(+)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 33c4fc1..f194439 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -73,3 +73,12 @@ int igt_debugfs_open(igt_debugfs_t *debugfs, const char
*filename)
sprintf(buf, %s/%s, debugfs-dri_path, filename);
return open(buf, O_RDONLY);
}
+
+FILE *igt_debugfs_fopen(igt_debugfs_t *debugfs, const char *filename,
+ const char *mode)
+{
+ char buf[1024];
+
+ sprintf(buf, %s/%s, debugfs-dri_path, filename);
+ return fopen(buf, mode);
+}
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index aa9449a..1035a93 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -25,6 +25,8 @@
#ifndef __IGT_DEBUGFS_H__
#define __IGT_DEBUGFS_H__
+#include stdio.h
+
typedef struct {
char root[128];
char dri_path[128];
@@ -32,5 +34,7 @@ typedef struct {
int igt_debugfs_init(igt_debugfs_t *debugfs);
int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename);
+FILE *igt_debugfs_fopen(igt_debugfs_t *debugfs, const char *filename,
+ const char *mode);
#endif /* __IGT_DEBUGFS_H__ */
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/8] lib: Make igt_debugfs_open() take the mode as argument
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_debugfs.c | 4 ++--
lib/igt_debugfs.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index f194439..7e625e1 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -66,12 +66,12 @@ find_minor:
return ENOENT;
}
-int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename)
+int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename, int mode)
{
char buf[1024];
sprintf(buf, %s/%s, debugfs-dri_path, filename);
- return open(buf, O_RDONLY);
+ return open(buf, mode);
}
FILE *igt_debugfs_fopen(igt_debugfs_t *debugfs, const char *filename,
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 1035a93..1ae6bbd 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -33,7 +33,7 @@ typedef struct {
} igt_debugfs_t;
int igt_debugfs_init(igt_debugfs_t *debugfs);
-int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename);
+int igt_debugfs_open(igt_debugfs_t *debugfs, const char *filename, int mode);
FILE *igt_debugfs_fopen(igt_debugfs_t *debugfs, const char *filename,
const char *mode);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/8] debugfs_pipe_crc: Let's check CRCs!
Let's add a new test that sets a mode, wait for a few vblanks (3) and
then make sure we read 3 identical CRCs.
Some subtests check for various parsing errors.
In the process, improve the debugfs helpers to deal with CRCs.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_debugfs.c| 199 +++
lib/igt_debugfs.h| 37
tests/.gitignore | 1 +
tests/Makefile.am| 1 +
tests/debugfs_pipe_crc.c | 237 +++
5 files changed, 475 insertions(+)
create mode 100644 tests/debugfs_pipe_crc.c
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 7e625e1..371f583 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -28,7 +28,10 @@
#include stdio.h
#include string.h
#include fcntl.h
+#include unistd.h
+#include drmtest.h
+#include igt_display.h
#include igt_debugfs.h
int igt_debugfs_init(igt_debugfs_t *debugfs)
@@ -82,3 +85,199 @@ FILE *igt_debugfs_fopen(igt_debugfs_t *debugfs, const char
*filename,
sprintf(buf, %s/%s, debugfs-dri_path, filename);
return fopen(buf, mode);
}
+
+/*
+ * Pipe CRC
+ */
+
+bool igt_crc_is_null(igt_crc_t *crc)
+{
+ int i;
+
+ for (i = 0; i crc-n_words; i++)
+ if (crc-crc[i])
+ return false;
+
+ return true;
+}
+
+bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b)
+{
+ int i;
+
+ if (a-n_words != b-n_words)
+ return false;
+
+ for (i = 0; i a-n_words; i++)
+ if (a-crc[i] != b-crc[i])
+ return false;
+
+ return true;
+}
+
+char *igt_crc_to_string(igt_crc_t *crc)
+{
+ char buf[128];
+
+ if (crc-n_words == 5)
+ sprintf(buf, %08x %08x %08x %08x %08x, crc-crc[0],
+ crc-crc[1], crc-crc[2], crc-crc[3], crc-crc[4]);
+ else
+ igt_assert(0);
+
+ return strdup(buf);
+}
+
+/* (6 fields, 8 chars each, space separated (5) + '\n') */
+#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
+/* account for \'0' */
+#define PIPE_CRC_BUFFER_LEN (PIPE_CRC_LINE_LEN + 1)
+
+struct _igt_pipe_crc {
+ int drm_fd;
+
+ int ctl_fd;
+ int crc_fd;
+ int line_len;
+ int buffer_len;
+
+ enum pipe pipe;
+ enum intel_pipe_crc_source source;
+};
+
+igt_pipe_crc_t *
+igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
+enum intel_pipe_crc_source source)
+{
+ igt_pipe_crc_t *pipe_crc;
+ char buf[128];
+
+ pipe_crc = calloc(1, sizeof(struct _igt_pipe_crc));
+
+ pipe_crc-ctl_fd = igt_debugfs_open(debugfs,
+ i915_display_crc_ctl, O_WRONLY);
+ igt_assert(pipe_crc-crc_fd != -1);
+
+ sprintf(buf, i915_pipe_%c_crc, pipe_name(pipe));
+ pipe_crc-crc_fd = igt_debugfs_open(debugfs, buf, O_RDONLY);
+ igt_assert(pipe_crc-crc_fd != -1);
+
+ pipe_crc-line_len = PIPE_CRC_LINE_LEN;
+ pipe_crc-buffer_len = PIPE_CRC_BUFFER_LEN;
+ pipe_crc-drm_fd = drm_fd;
+ pipe_crc-pipe = pipe;
+ pipe_crc-source = source;
+
+ return pipe_crc;
+}
+
+static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
+{
+ char buf[32];
+
+ sprintf(buf, pipe %c none, pipe_name(pipe));
+ write(fd, buf, strlen(buf));
+}
+
+/*
+ * Turn off everything
+ */
+void igt_pipe_crc_reset(void)
+{
+ igt_debugfs_t debugfs;
+ int fd;
+
+ igt_debugfs_init(debugfs);
+ fd = igt_debugfs_open(debugfs, i915_display_crc_ctl, O_WRONLY);
+
+ igt_pipe_crc_pipe_off(fd, PIPE_A);
+ igt_pipe_crc_pipe_off(fd, PIPE_B);
+ igt_pipe_crc_pipe_off(fd, PIPE_C);
+}
+
+void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
+{
+ close(pipe_crc-ctl_fd);
+ close(pipe_crc-crc_fd);
+ free(pipe_crc);
+}
+
+static const char *pipe_crc_sources[] = {
+none,
+plane1,
+plane2,
+pf,
+};
+
+static const char *pipe_crc_source_name(enum intel_pipe_crc_source source)
+{
+return pipe_crc_sources[source];
+}
+
+void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
+{
+ char buf[64];
+ igt_crc_t *crcs = NULL;
+
+ igt_wait_for_vblank(pipe_crc-drm_fd, pipe_crc-pipe);
+
+ sprintf(buf, pipe %c %s, pipe_name(pipe_crc-pipe),
+ pipe_crc_source_name(pipe_crc-source));
+ write(pipe_crc-ctl_fd, buf, strlen(buf));
+
+ /*
+* For some no yet identified reason, the first CRC is bonkers. So
+* let's just wait for the next vblank and read out the buggy result.
+*/
+ igt_pipe_crc_get_crcs(pipe_crc, 1, crcs);
+ free(crcs);
+}
+
+void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
+{
+ char buf[32];
+
+ sprintf(buf, pipe %c none, pipe_name(pipe_crc-pipe));
+ write(pipe_crc-ctl_fd, buf, strlen(buf));
+}
+
+static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
+{
+
Re: [Intel-gfx] [PATCH 4/5] drm/i915: take power well refs when needed
2013/10/14 Jesse Barnes jbar...@virtuousgeek.org:
When accessing the display regs for hw state readout or cross check, we
need to make sure the power well is enabled so we can read valid
register state.
On the current code (HSW) we already check for the power wells in the
HW state readout code: if the power well is disabled, then transcoders
A/B/C are disabled. The current code takes care to not touch registers
on a disabled power well.
Likewise, in an actual mode set, we need to take a ref on the
appropriate power well so that the mode set succeeds. From then on, the
power well ref will be tracked by the CRTC enable/disable code.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_dma.c | 2 ++
drivers/gpu/drm/i915/intel_display.c | 30 ++
2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 313a8c9..91c3e6c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1367,6 +1367,8 @@ static int i915_load_modeset_init(struct drm_device
*dev)
i915_disable_vga_mem(dev);
intel_display_power_put(dev, POWER_DOMAIN_VGA);
+ intel_display_power_put(dev, POWER_DOMAIN_PIPE(0));
+
Why is this here? It certainly deserves a comment in the code.
/* Only enable hotplug handling once the fbdev is fully set up. */
dev_priv-enable_hotplug_processing = true;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6e4729b..62ee110 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3841,6 +3841,8 @@ static void valleyview_crtc_enable(struct drm_crtc
*crtc)
if (intel_crtc-active)
return;
+ intel_display_power_get(dev, POWER_DOMAIN_PIPE(pipe));
+
intel_crtc-active = true;
for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -3975,6 +3977,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
intel_update_fbc(dev);
+
+ if (IS_VALLEYVIEW(dev))
+ intel_display_power_put(dev, POWER_DOMAIN_PIPE(pipe));
So now HSW uses global_resources while VLV uses crtc enable/disable. I
really think both platforms should try to do the same thing.
By the way, at least on Haswell, if we do an equivalent change we'll
have problems, because when you disable the power well at
crtc_disable, then everything you did at crtc_mode_set will be undone,
and it won't be redone at crtc_enable. When you reenable the power
well, the registers go back to their default values, not the values
that were previously there. Did you check if VLV behaves the same?
}
static void i9xx_crtc_off(struct drm_crtc *crtc)
@@ -4134,6 +4139,11 @@ static void intel_connector_check_state(struct
intel_connector *connector)
* consider. */
void intel_connector_dpms(struct drm_connector *connector, int mode)
{
+ struct intel_crtc *intel_crtc =
to_intel_crtc(connector-encoder-crtc);
+ enum intel_display_power_domain domain;
+
+ domain = POWER_DOMAIN_PIPE(intel_crtc-pipe);
+
/* All the simple cases only support two dpms states. */
if (mode != DRM_MODE_DPMS_ON)
mode = DRM_MODE_DPMS_OFF;
@@ -4141,6 +4151,7 @@ void intel_connector_dpms(struct drm_connector
*connector, int mode)
if (mode == connector-dpms)
return;
+ intel_display_power_get(connector-dev, domain);
connector-dpms = mode;
/* Only need to change hw state when actually enabled */
@@ -4148,6 +4159,7 @@ void intel_connector_dpms(struct drm_connector
*connector, int mode)
intel_encoder_dpms(to_intel_encoder(connector-encoder),
mode);
intel_modeset_check_state(connector-dev);
+ intel_display_power_put(connector-dev, domain);
}
/* Simple connector-get_hw_state implementation for encoders that support
only
@@ -9192,6 +9204,15 @@ static int __intel_set_mode(struct drm_crtc *crtc,
for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
intel_crtc_disable(intel_crtc-base);
+ /*
+* We take a ref here so the mode set will hit live hw. Once
+* we call the CRTC enable, we can drop our ref since it'll get
+* tracked there from then on.
+*/
+ for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
+ intel_display_power_get(dev,
+ POWER_DOMAIN_PIPE(intel_crtc-pipe));
+
for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
if (intel_crtc-base.enabled)
dev_priv-display.crtc_disable(intel_crtc-base);
@@ -9247,6 +9268,10 @@ done:
}
out:
+ for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
+
Re: [Intel-gfx] [PATCH 0/2] drm/i915: fix hdmi audio pixel clock config
Good news! Audio works now in 1080p@50/60 but only in 48000khz which fixes most of my problems :-) What (still) does not work in 50/60hz is 41000khz audio (still works in lower modes) so the patch is not 100% :-) I noticed this because menu sounds in XBMC are 41000 khz and they wouldn't play where all me test audio content worked well. I fixed this by adding a custom resample option where sound in menus started working too. Anything i can do tho help ? I would like to thank you for all your superbly great work so far :-) You rock dude! :-) On Mon, Oct 14, 2013 at 6:02 PM, Jani Nikula jani.nik...@intel.com wrote: This is a completely untested and possibly naïve attempt at fixing [1] and [2]. Based on top of drm-intel-nightly branch of [3]. David, Jasper, please give it a try. Thanks, Jani. [1] http://mid.gmane.org/cagpeb3ep1lrzetpxhgrfbdqr5ts2tac8gcukwwuguf1u5ny...@mail.gmail.com [2] http://mid.gmane.org/20130206213533.ga16...@hardeman.nu [3] git://people.freedesktop.org/~danvet/drm-intel Jani Nikula (2): drm/i915: pass mode to write eld drm/i915: set hdmi audio clock drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/i915_reg.h | 12 ++- drivers/gpu/drm/i915/intel_display.c | 58 +- 3 files changed, 63 insertions(+), 10 deletions(-) -- 1.7.9.5 -- Met Vriendelijke Groeten Jasper Smet Developer Twitter: josbeir E-mail: josb...@gmail.com Mobile: 0486/41.75.45 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915/vlv: support save/restore of display state around power well toggle
2013/10/14 Jesse Barnes jbar...@virtuousgeek.org:
If we disable the power well at runtime, we need to save enough display
state so we can restore it when the power well comes back again. Add
support for that on VLV by reusing some of the _freeze and _thaw code.
Note we need to drop the power well lock in this path around the restore,
since we'll end up in mode set functions that take more refs on the
power well.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_uncore.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index b126f5a..070ff00 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -406,7 +406,26 @@ static void __vlv_set_power_well(struct drm_i915_private
*dev_priv,
static void vlv_set_display_power(struct drm_i915_private *dev_priv,
bool enable)
{
- __vlv_set_power_well(dev_priv, DISP2D_PWRGT, enable);
+ struct drm_device *dev = dev_priv-dev;
+ struct i915_power_well *power_well = dev_priv-power_well;
+
+ if (enable) {
+ /* Lost all the display state, restore it */
+ if (vlv_display_power_enabled(dev_priv))
+ return; /* already on, skip the fireworks */
+ __vlv_set_power_well(dev_priv, DISP2D_PWRGT, true);
+ spin_unlock_irq(power_well-lock);
+ i915_restore_state(dev);
At least on Haswell, i915_restore_state restores a lot more than just
what was lost on the power well. Do you really need all this? Besides,
I kinda fear you can break things by restoring stuff that is already
running. While we're doing this restoring, interrputs are happening,
everything is running, yet we call stuff like intel_i2c_reset,
i915_gem_restore_fences and others. Another example: we have code to
restore the backlight registers, but backlight is usually on the
output that works even with the power well disabled. So if we disable
the power well, then change backlight on the only output that is
working, then reenable the power well we'll restore a bogus
backlight state on a case where we shouldn't be touching backlight at
all.
+ intel_modeset_init_hw(dev);
+ intel_modeset_setup_hw_state(dev, true);
+ spin_lock_irq(power_well-lock);
+ } else {
+ if (!vlv_display_power_enabled(dev_priv))
+ return; /* already off, skip the fireworks */
+ /* Make sure we save the state we need */
+ i915_save_state(dev);
+ __vlv_set_power_well(dev_priv, DISP2D_PWRGT, enable);
+ }
}
static void vlv_set_render_power(struct drm_i915_private *dev_priv, bool
enable)
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: take power well refs when needed
On Tue, 15 Oct 2013 16:54:00 -0300 Paulo Zanoni przan...@gmail.com wrote: 2013/10/14 Jesse Barnes jbar...@virtuousgeek.org: When accessing the display regs for hw state readout or cross check, we need to make sure the power well is enabled so we can read valid register state. On the current code (HSW) we already check for the power wells in the HW state readout code: if the power well is disabled, then transcoders A/B/C are disabled. The current code takes care to not touch registers on a disabled power well. Yeah and we should probably preserve that behavior, for the readout code at least. + intel_display_power_put(dev, POWER_DOMAIN_PIPE(0)); + Why is this here? It certainly deserves a comment in the code. It probably needs to be removed. The refcounting for the initial load is still screwy due to the unconditional enable later on. So now HSW uses global_resources while VLV uses crtc enable/disable. I really think both platforms should try to do the same thing. Definitely agreed. By the way, at least on Haswell, if we do an equivalent change we'll have problems, because when you disable the power well at crtc_disable, then everything you did at crtc_mode_set will be undone, and it won't be redone at crtc_enable. When you reenable the power well, the registers go back to their default values, not the values that were previously there. Did you check if VLV behaves the same? No that's taken into account here. In __intel_set_mode we take a private ref on the appropriate power well so that we'll preserve state until we do the first crtc_enable. From then on, the ref is tracked there and we drop the private one in __intel_set_mode + if (crtc-active) + intel_display_power_get(dev, + POWER_DOMAIN_PIPE(crtc-pipe)); + What about the panel fitter power domains? Sometimes the panel fitter is the thing that makes you require a power well, even though you're on a pipe that doesn't need it. And on Haswell you also have to take into account TRANSCODER_EDP+PIPE_A versus TRANSCODER_A+PIPE_A, where the first doesn't need the power well but the second needs it. Yeah I'm still not sure how to handle this in generic code. Maybe the power well mapping function Imre added will be enough, but it definitely gets tricky when we look at all the different platforms we have to (and will have to) handle. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915/vlv: support save/restore of display state around power well toggle
On Tue, 15 Oct 2013 17:09:19 -0300
Paulo Zanoni przan...@gmail.com wrote:
2013/10/14 Jesse Barnes jbar...@virtuousgeek.org:
If we disable the power well at runtime, we need to save enough display
state so we can restore it when the power well comes back again. Add
support for that on VLV by reusing some of the _freeze and _thaw code.
Note we need to drop the power well lock in this path around the restore,
since we'll end up in mode set functions that take more refs on the
power well.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_uncore.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index b126f5a..070ff00 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -406,7 +406,26 @@ static void __vlv_set_power_well(struct
drm_i915_private *dev_priv,
static void vlv_set_display_power(struct drm_i915_private *dev_priv,
bool enable)
{
- __vlv_set_power_well(dev_priv, DISP2D_PWRGT, enable);
+ struct drm_device *dev = dev_priv-dev;
+ struct i915_power_well *power_well = dev_priv-power_well;
+
+ if (enable) {
+ /* Lost all the display state, restore it */
+ if (vlv_display_power_enabled(dev_priv))
+ return; /* already on, skip the fireworks */
+ __vlv_set_power_well(dev_priv, DISP2D_PWRGT, true);
+ spin_unlock_irq(power_well-lock);
+ i915_restore_state(dev);
At least on Haswell, i915_restore_state restores a lot more than just
what was lost on the power well. Do you really need all this? Besides,
I kinda fear you can break things by restoring stuff that is already
running. While we're doing this restoring, interrputs are happening,
everything is running, yet we call stuff like intel_i2c_reset,
i915_gem_restore_fences and others. Another example: we have code to
restore the backlight registers, but backlight is usually on the
output that works even with the power well disabled. So if we disable
the power well, then change backlight on the only output that is
working, then reenable the power well we'll restore a bogus
backlight state on a case where we shouldn't be touching backlight at
all.
Yeah this is definitely a WIP. We'll probably need power well specific
save/restore functions. Depending on which well was toggled, the state
that needs to be saved restored will be different. It may be best to
try to push all this into modeset_setup_hw_state, since it should know
which power wells are needed for different ops and thus restore the
right state. But that means ripping apart save/restore_state and
putting bits in the right places, potentially with specific hooks like
the uncore save/restore I added for other stuff.
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: take power well refs when needed
2013/10/15 Jesse Barnes jbar...@virtuousgeek.org: On Tue, 15 Oct 2013 16:54:00 -0300 Paulo Zanoni przan...@gmail.com wrote: 2013/10/14 Jesse Barnes jbar...@virtuousgeek.org: When accessing the display regs for hw state readout or cross check, we need to make sure the power well is enabled so we can read valid register state. On the current code (HSW) we already check for the power wells in the HW state readout code: if the power well is disabled, then transcoders A/B/C are disabled. The current code takes care to not touch registers on a disabled power well. Yeah and we should probably preserve that behavior, for the readout code at least. + intel_display_power_put(dev, POWER_DOMAIN_PIPE(0)); + Why is this here? It certainly deserves a comment in the code. It probably needs to be removed. The refcounting for the initial load is still screwy due to the unconditional enable later on. So now HSW uses global_resources while VLV uses crtc enable/disable. I really think both platforms should try to do the same thing. Definitely agreed. By the way, at least on Haswell, if we do an equivalent change we'll have problems, because when you disable the power well at crtc_disable, then everything you did at crtc_mode_set will be undone, and it won't be redone at crtc_enable. When you reenable the power well, the registers go back to their default values, not the values that were previously there. Did you check if VLV behaves the same? No that's taken into account here. In __intel_set_mode we take a private ref on the appropriate power well so that we'll preserve state until we do the first crtc_enable. From then on, the ref is tracked there and we drop the private one in __intel_set_mode Yeah, but then after all this is done, at some point we'll call crtc_disable, then we'll put the last ref back and then the power well will be disabled. Then the user moves the mouse and we call crtc_enable, so we enable the power well, all its registers to back to their reset state, then crtc_enable sets some of the registers, but that won't really be enough since no one called crtc_mode_set again, and all the state set by crtc_mode_set (not touched by crtc_enable) is back to the default values. No? What am I missing? + if (crtc-active) + intel_display_power_get(dev, + POWER_DOMAIN_PIPE(crtc-pipe)); + What about the panel fitter power domains? Sometimes the panel fitter is the thing that makes you require a power well, even though you're on a pipe that doesn't need it. And on Haswell you also have to take into account TRANSCODER_EDP+PIPE_A versus TRANSCODER_A+PIPE_A, where the first doesn't need the power well but the second needs it. Yeah I'm still not sure how to handle this in generic code. Maybe the power well mapping function Imre added will be enough, but it definitely gets tricky when we look at all the different platforms we have to (and will have to) handle. Thanks, -- Jesse Barnes, Intel Open Source Technology Center -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915_drm: add exec flag to warn kernel that userspace is using predication
If Userspace isn't using MI_PREDICATE all slices must be enabled for
backward compatibility.
If I915_EXEC_USE_PREDICATE isn't set and defaul is set to half, kernel will
force
all slices on.
v2: include more tests and s/GT_FULL/USE_PREDICATE
on code and on commit message.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
include/drm/i915_drm.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index aa983f3..933656c 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -670,6 +670,12 @@ struct drm_i915_gem_execbuffer2 {
/** Resets the SO write offset registers for transform feedback on gen7. */
#define I915_EXEC_GEN7_SOL_RESET (18)
+/* If this flag is set userspace is using predicate and half slices can be
+ * let disabled for power saving. Otherwise use all slices even when disabled
+ * by boot parameter or via sysfs interface
+ */
+#define I915_EXEC_USE_PREDICATE(113)
+
#define I915_EXEC_CONTEXT_ID_MASK (0x)
#define i915_execbuffer2_set_context_id(eb2, context) \
(eb2).rsvd1 = context I915_EXEC_CONTEXT_ID_MASK
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: HSW GT3 Slices: exec flag to warn kernel that userspace is using predication
If Userspace isn't using MI_PREDICATE all slices must be enabled for
backward compatibility.
If I915_EXEC_USE_PREDICATE isn't set and defaul is set to half, kernel will
force
all slices on.
v2: fix the inverted logic for backwards compatibility
USE_PREDICATE unset force gt_full when defaul is half
instead of GT_FULL flag.
CC: Eric Anholt e...@anholt.net
CC: Kenneth Graunke kenn...@whitecape.org
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.h| 8
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 67 ++
drivers/gpu/drm/i915/i915_reg.h| 11 +
drivers/gpu/drm/i915/i915_sysfs.c | 11 -
drivers/gpu/drm/i915/intel_display.c | 6 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c| 39 -
include/uapi/drm/i915_drm.h| 8 +++-
8 files changed, 146 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 685fb1d..bd4774e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1219,6 +1219,12 @@ struct i915_package_c8 {
} regsave;
};
+struct i915_gt_slices {
+ int state_default;
+ int forcing_full;
+ struct mutex m_lock;
+};
+
typedef struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1418,6 +1424,8 @@ typedef struct drm_i915_private {
struct i915_package_c8 pc8;
+ struct i915_gt_slices gt_slices;
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
* here! */
struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 0ce0d47..eb09a51 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -923,6 +923,66 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
}
static int
+i915_gt_full_immediate(struct drm_device *dev, struct intel_ring_buffer *ring)
+{
+ drm_i915_private_t *dev_priv = dev-dev_private;
+ int ret;
+
+ if (!HAS_SLICE_SHUTDOWN(dev) || ring == dev_priv-ring[BCS])
+ return 0;
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, HSW_GT_SLICE_INFO);
+ intel_ring_emit(ring, SLICE_SEL_BOTH);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, MI_PREDICATE_RESULT_2);
+ intel_ring_emit(ring, LOWER_SLICE_ENABLED);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, HSW_SLICESHUTDOWN);
+ intel_ring_emit(ring, ~SLICE_SHUTDOWN);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, RC_IDLE_MAX_COUNT);
+ intel_ring_emit(ring, CS_IDLE_COUNT_1US);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, WAIT_FOR_RC6_EXIT);
+ intel_ring_emit(ring, WAIT_RC6_EXIT | MASK_WAIT_RC6_EXIT);
+ intel_ring_advance(ring);
+
+ ret = intel_ring_begin(ring, 3);
+ if (ret)
+ return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, RC_IDLE_MAX_COUNT);
+ intel_ring_emit(ring, CS_IDLE_COUNT_5US);
+ intel_ring_advance(ring);
+
+ return 0;
+}
+
+static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
struct drm_i915_gem_execbuffer2 *args,
@@ -999,6 +1059,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
return -EINVAL;
}
+ if ((args-flags I915_EXEC_USE_PREDICATE) == 0
+ dev_priv-gt_slices.state_default == 0
+ !dev_priv-gt_slices.forcing_full) {
+ dev_priv-gt_slices.forcing_full = true;
+ i915_gt_full_immediate(dev, ring);
+ }
+
mode = args-flags I915_EXEC_CONSTANTS_MASK;
mask = I915_EXEC_CONSTANTS_MASK;
switch (mode) {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 497c441..0146bef 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -277,6 +277,17 @@
#define SLICE_STATUS_MAIN_ON (20)
#define SLICE_STATUS_BOTH_ON (30)
[Intel-gfx] [PATCH] testcases: Slice Shutdown testscases.
1. sysfs half/full switch.
2. on full execbuf without I915_EXEC_USE_PREDICATE
3. on full execbuf with I915_EXEC_USE_PREDICATE
4. on half execbuf without I915_EXEC_USE_PREDICATE
5. on half execbuf with I915_EXEC_USE_PREDICATE
v2: include more tests and s/GT_FULL/USE_PREDICATE
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
tests/Makefile.am | 1 +
tests/gt_slice_config.c | 325
2 files changed, 326 insertions(+)
create mode 100644 tests/gt_slice_config.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0290ae0..cf105e4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -50,6 +50,7 @@ TESTS_progs_M = \
gem_tiled_blits \
gem_tiled_partial_pwrite_pread \
gem_write_read_ring_switch \
+ gt_slice_config \
kms_flip \
kms_render \
kms_setmode \
diff --git a/tests/gt_slice_config.c b/tests/gt_slice_config.c
new file mode 100644
index 000..7b9f554
--- /dev/null
+++ b/tests/gt_slice_config.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Rodrigo Vivi rodrigo.v...@intel.com
+ *
+ */
+
+/*
+ * Testcase: Test GT slice shutdown feature
+ *
+ * Sub tests:
+ * 1. sysfs half/full switch.
+ * 2. on full execbuf without I915_EXEC_USE_PREDICATE
+ * 3. on full execbuf with I915_EXEC_USE_PREDICATE
+ * 4. on half execbuf without I915_EXEC_USE_PREDICATE
+ * 5. on half execbuf with I915_EXEC_USE_PREDICATE
+ */
+
+#define _GNU_SOURCE
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include unistd.h
+#include drmtest.h
+
+static void exec(int fd, uint32_t handle, bool use_predicate)
+{
+struct drm_i915_gem_execbuffer2 execbuf;
+struct drm_i915_gem_exec_object2 gem_exec[1];
+uint32_t b[2] = {MI_BATCH_BUFFER_END};
+int loops = 200;
+int ret = 0;
+
+gem_write(fd, handle, 0, b, sizeof(b));
+
+gem_exec[0].handle = handle;
+gem_exec[0].relocation_count = 0;
+gem_exec[0].relocs_ptr = 0;
+gem_exec[0].alignment = 0;
+gem_exec[0].offset = 0;
+gem_exec[0].flags = 0;
+gem_exec[0].rsvd1 = 0;
+gem_exec[0].rsvd2 = 0;
+
+execbuf.buffers_ptr = (uintptr_t)gem_exec;
+execbuf.buffer_count = 1;
+execbuf.batch_start_offset = 0;
+execbuf.batch_len = 8;
+execbuf.cliprects_ptr = 0;
+execbuf.num_cliprects = 0;
+execbuf.DR1 = 0;
+execbuf.DR4 = 0;
+execbuf.flags = I915_EXEC_RENDER;
+ if (use_predicate)
+ execbuf.flags |= I915_EXEC_USE_PREDICATE;
+i915_execbuffer2_set_context_id(execbuf, 0);
+execbuf.rsvd2 = 0;
+
+while (loops-- ret == 0) {
+ret = drmIoctl(fd,
+ DRM_IOCTL_I915_GEM_EXECBUFFER2,
+ execbuf);
+}
+ gem_sync(fd, handle);
+}
+
+static bool is_full(const int device)
+{
+ char *path;
+ FILE *file;
+ int ret;
+ char str[5];
+
+ ret = asprintf(path, /sys/class/drm/card%d/power/gt_slice_config,
+ device);
+ igt_assert(ret != -1);
+
+ file = fopen(path, r);
+ igt_require(file);
+
+ ret = fscanf(file, %s, str);
+ igt_assert(ret != 0);
+
+ fclose(file);
+ return strcmp(str, half);
+}
+
+static int set_status(const int device, bool full)
+{
+ char *path;
+ FILE *file;
+ int ret;
+ char str[5];
+ int attempts = 10;
+
+ ret = asprintf(path, /sys/class/drm/card%d/power/gt_slice_config,
+ device);
+ igt_assert(ret != -1);
+
+ file = fopen(path, r);
+ igt_require(file);
+
+ ret = fscanf(file, %s, str);
+ igt_assert(ret != 0);
+
+ fclose(file);
+
+ if (full == strcmp(str,
Re: [Intel-gfx] [PATCH 4/5] drm/i915: take power well refs when needed
On Tue, 15 Oct 2013 17:47:20 -0300 Paulo Zanoni przan...@gmail.com wrote: 2013/10/15 Jesse Barnes jbar...@virtuousgeek.org: On Tue, 15 Oct 2013 16:54:00 -0300 Paulo Zanoni przan...@gmail.com wrote: 2013/10/14 Jesse Barnes jbar...@virtuousgeek.org: When accessing the display regs for hw state readout or cross check, we need to make sure the power well is enabled so we can read valid register state. On the current code (HSW) we already check for the power wells in the HW state readout code: if the power well is disabled, then transcoders A/B/C are disabled. The current code takes care to not touch registers on a disabled power well. Yeah and we should probably preserve that behavior, for the readout code at least. + intel_display_power_put(dev, POWER_DOMAIN_PIPE(0)); + Why is this here? It certainly deserves a comment in the code. It probably needs to be removed. The refcounting for the initial load is still screwy due to the unconditional enable later on. So now HSW uses global_resources while VLV uses crtc enable/disable. I really think both platforms should try to do the same thing. Definitely agreed. By the way, at least on Haswell, if we do an equivalent change we'll have problems, because when you disable the power well at crtc_disable, then everything you did at crtc_mode_set will be undone, and it won't be redone at crtc_enable. When you reenable the power well, the registers go back to their default values, not the values that were previously there. Did you check if VLV behaves the same? No that's taken into account here. In __intel_set_mode we take a private ref on the appropriate power well so that we'll preserve state until we do the first crtc_enable. From then on, the ref is tracked there and we drop the private one in __intel_set_mode Yeah, but then after all this is done, at some point we'll call crtc_disable, then we'll put the last ref back and then the power well will be disabled. Then the user moves the mouse and we call crtc_enable, so we enable the power well, all its registers to back to their reset state, then crtc_enable sets some of the registers, but that won't really be enough since no one called crtc_mode_set again, and all the state set by crtc_mode_set (not touched by crtc_enable) is back to the default values. No? What am I missing? That's where the save/restore state of the later patch comes in. We need that if we're going to support DPMS and runtime suspend w/o a mode set. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: take power well refs when needed
2013/10/15 Jesse Barnes jbar...@virtuousgeek.org: On Tue, 15 Oct 2013 17:47:20 -0300 Paulo Zanoni przan...@gmail.com wrote: 2013/10/15 Jesse Barnes jbar...@virtuousgeek.org: On Tue, 15 Oct 2013 16:54:00 -0300 Paulo Zanoni przan...@gmail.com wrote: 2013/10/14 Jesse Barnes jbar...@virtuousgeek.org: When accessing the display regs for hw state readout or cross check, we need to make sure the power well is enabled so we can read valid register state. On the current code (HSW) we already check for the power wells in the HW state readout code: if the power well is disabled, then transcoders A/B/C are disabled. The current code takes care to not touch registers on a disabled power well. Yeah and we should probably preserve that behavior, for the readout code at least. + intel_display_power_put(dev, POWER_DOMAIN_PIPE(0)); + Why is this here? It certainly deserves a comment in the code. It probably needs to be removed. The refcounting for the initial load is still screwy due to the unconditional enable later on. So now HSW uses global_resources while VLV uses crtc enable/disable. I really think both platforms should try to do the same thing. Definitely agreed. By the way, at least on Haswell, if we do an equivalent change we'll have problems, because when you disable the power well at crtc_disable, then everything you did at crtc_mode_set will be undone, and it won't be redone at crtc_enable. When you reenable the power well, the registers go back to their default values, not the values that were previously there. Did you check if VLV behaves the same? No that's taken into account here. In __intel_set_mode we take a private ref on the appropriate power well so that we'll preserve state until we do the first crtc_enable. From then on, the ref is tracked there and we drop the private one in __intel_set_mode Yeah, but then after all this is done, at some point we'll call crtc_disable, then we'll put the last ref back and then the power well will be disabled. Then the user moves the mouse and we call crtc_enable, so we enable the power well, all its registers to back to their reset state, then crtc_enable sets some of the registers, but that won't really be enough since no one called crtc_mode_set again, and all the state set by crtc_mode_set (not touched by crtc_enable) is back to the default values. No? What am I missing? That's where the save/restore state of the later patch comes in. We need that if we're going to support DPMS and runtime suspend w/o a mode set. Oh... I wasn't even thinking about it since it's on a later patch. But makes sense. But instead of the save/restore thing, shouldn't we just move all the code that touches registers from mode_set to crtc_enable? IMHO it's the shiny solution here. And anything else that we may need to save/restore should be moved to crtc enable/disable and be saved in struct intel_crtc every time it is touched. I really think that removing stuff from the save/restore code is the way to go. Also, with my suggestion, crtc_enable will fully implement the mode set sequence from BSpec, so people like the Audio guys will have an easier time understanding our code :) -- Jesse Barnes, Intel Open Source Technology Center -- Paulo Zanoni ___ 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: fix hdmi audio pixel clock config
Not a surprising outcome since it essentially means feature parity with the Windows driver (48kHz working, 44.1kHz not working). I still suspect it's a bug with the Pioneer receiver. On Tue, Oct 15, 2013 at 10:02:54PM +0200, Jasper Smet wrote: Good news! Audio works now in 1080p@50/60 but only in 48000khz which fixes most of my problems :-) What (still) does not work in 50/60hz is 41000khz audio (still works in lower modes) so the patch is not 100% :-) I noticed this because menu sounds in XBMC are 41000 khz and they wouldn't play where all me test audio content worked well. I fixed this by adding a custom resample option where sound in menus started working too. Anything i can do tho help ? I would like to thank you for all your superbly great work so far :-) You rock dude! :-) On Mon, Oct 14, 2013 at 6:02 PM, Jani Nikula jani.nik...@intel.com wrote: This is a completely untested and possibly naïve attempt at fixing [1] and [2]. Based on top of drm-intel-nightly branch of [3]. David, Jasper, please give it a try. Thanks, Jani. [1] http://mid.gmane.org/cagpeb3ep1lrzetpxhgrfbdqr5ts2tac8gcukwwuguf1u5ny...@mail.gmail.com [2] http://mid.gmane.org/20130206213533.ga16...@hardeman.nu [3] git://people.freedesktop.org/~danvet/drm-intel Jani Nikula (2): drm/i915: pass mode to write eld drm/i915: set hdmi audio clock drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/i915_reg.h | 12 ++- drivers/gpu/drm/i915/intel_display.c | 58 +- 3 files changed, 63 insertions(+), 10 deletions(-) -- 1.7.9.5 -- Met Vriendelijke Groeten Jasper Smet Developer Twitter: josbeir E-mail: josb...@gmail.com Mobile: 0486/41.75.45 -- David Härdeman ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Runtime display PM for VLV/BYT
On Tue, Oct 15, 2013 at 8:15 PM, Imre Deak imre.d...@intel.com wrote: Related to this: I made intel_encoder_get_hw_state() only check if the power well is on and return false if it's not to indicate that the encoder is off. I also thought of doing the same as you and take a ref instead, not sure what's the right way. Maybe doing the readout only if the power is on, but also making sure we have a reference in this case? So with a new helper we'd have in intel_encoder_get_hw_state(): I think the approach we've quickly discussed in today's call is probably simplest: We grab a temporary reference to all the display power wells around all the dpms/modeset functions and ignore any power well checks on top of that. The hw will (well, should) be in the power on default state, so nothing should magically turn on if we don't want that. -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 v5 0/4] Fix Win8 backlight issue
On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote: v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; 2 Rename bd_list_head and bd_list_mutex in backlight.c to backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael - patch 1/4. v4: Remove decleration and stub for acpi_video_unregister_backlight in video.h of patch 2/4 since that function doesn't exist anymore in v3. v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister_backlight introduced in patch 2/4 as pointed out by Jani Nikula. v2: 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 four 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/4 for details. Then patch 2/4 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/4 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. Patch 4/4 fixes some problems in thinkpad-acpi module. Technically, patch 2/4 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Aaron Lu (4): 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 thinkpad-acpi: fix handle locate for video and query of _BCL drivers/acpi/internal.h | 4 +- drivers/acpi/video.c | 457 --- drivers/acpi/video_detect.c | 4 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/linux/backlight.h| 4 + 6 files changed, 326 insertions(+), 205 deletions(-) I've added this series to my queue for 3.13. Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default. Hopefully, in the future we'll be able to fix the problems causing video.use_native_backlight=1 to fail of the systems where it fails and then we'll be able to make that the default behavior and drop the option altogether. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
