[Intel-gfx] [PATCH] drm/i915: only hook up hpd pulse for DP outputs
On HSW+, the digital encoders are shared between HDMI and DP outputs, with one encoder masquerading as both. The VBT should tell us if we need to have DP or HDMI support on a particular port, but if we don't have DP support and we enable the DP hpd pulse handler then we cause an oops. Don't hook up the DP hpd handling if we don't have a DP port. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81856 Reported-by: Intel QA Team. Signed-off-by: Dave Airlie airl...@redhat.com # v1 [ickle: Fix the error handling after a malloc failure] Cc: Dave Airlie airl...@redhat.com Cc: Paulo Zanoni przan...@gmail.com Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_ddi.c | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index a6024de..3634575 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1557,15 +1557,13 @@ void intel_ddi_init(struct drm_device *dev, enum port port) struct intel_digital_port *intel_dig_port; struct intel_encoder *intel_encoder; struct drm_encoder *encoder; - struct intel_connector *hdmi_connector = NULL; - struct intel_connector *dp_connector = NULL; bool init_hdmi, init_dp; init_hdmi = (dev_priv-vbt.ddi_port_info[port].supports_dvi || dev_priv-vbt.ddi_port_info[port].supports_hdmi); init_dp = dev_priv-vbt.ddi_port_info[port].supports_dp; if (!init_dp !init_hdmi) { - DRM_DEBUG_KMS(VBT says port %c is not DVI/HDMI/DP compatible\n, + DRM_DEBUG_KMS(VBT says port %c is not DVI/HDMI/DP compatible, assuming it is\n, port_name(port)); init_hdmi = true; init_dp = true; @@ -1595,23 +1593,28 @@ void intel_ddi_init(struct drm_device *dev, enum port port) DDI_A_4_LANES); intel_encoder-type = INTEL_OUTPUT_UNKNOWN; - intel_encoder-crtc_mask = (1 0) | (1 1) | (1 2); + intel_encoder-crtc_mask = (1 0) | (1 1) | (1 2); intel_encoder-cloneable = 0; intel_encoder-hot_plug = intel_ddi_hot_plug; - intel_dig_port-hpd_pulse = intel_dp_hpd_pulse; - dev_priv-hpd_irq_port[port] = intel_dig_port; + if (init_dp) { + if (!intel_ddi_init_dp_connector(intel_dig_port)) + goto err; - if (init_dp) - dp_connector = intel_ddi_init_dp_connector(intel_dig_port); + intel_dig_port-hpd_pulse = intel_dp_hpd_pulse; + dev_priv-hpd_irq_port[port] = intel_dig_port; + } /* In theory we don't need the encoder-type check, but leave it just in * case we have some really bad VBTs... */ - if (intel_encoder-type != INTEL_OUTPUT_EDP init_hdmi) - hdmi_connector = intel_ddi_init_hdmi_connector(intel_dig_port); - - if (!dp_connector !hdmi_connector) { - drm_encoder_cleanup(encoder); - kfree(intel_dig_port); + if (intel_encoder-type != INTEL_OUTPUT_EDP init_hdmi) { + if (!intel_ddi_init_hdmi_connector(intel_dig_port)) + goto err; } + + return; + +err: + drm_encoder_cleanup(encoder); + kfree(intel_dig_port); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: only hook up hpd pulse for DP outputs
On 4 August 2014 16:15, Chris Wilson ch...@chris-wilson.co.uk wrote: On HSW+, the digital encoders are shared between HDMI and DP outputs, with one encoder masquerading as both. The VBT should tell us if we need to have DP or HDMI support on a particular port, but if we don't have DP support and we enable the DP hpd pulse handler then we cause an oops. Don't hook up the DP hpd handling if we don't have a DP port. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81856 Reviewed-by: Dave Airlie airl...@redhat.com Thanks, I'll put this into -next straight, since when I merged MST I agreed I'd be fast about fixing my crap. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-next
Hi Dave, Final feature pull for 3.17. drm-intel-next-2014-07-25: - Ditch UMS support (well just the config option for now) - Prep work for future platforms (Sonika Jindal, Damien) - runtime pm/soix fixes (Paulo, Jesse) - psr tracking improvements, locking fixes, now enabled by default! - rps fixes for chv (Deepak, Ville) - drm core patches for rotation support (Ville, Sagar Kamble) - the i915 parts unfortunately didn't make it yet - userptr fixes (Chris) - minimum backlight brightness (Jani), acked long ago by Matthew Garret on irc - I've forgotten about this patch :( QA is a bit unhappy about the DP MST stuff since it broke hpd testing a bit, but otherwise looks sane. I've backmerged drm-next to resolve conflicts with the mst stuff, which means the new tag itself doesn't contain the overview as usual. Cheers, Daniel The following changes since commit e05444be705b5c7c7f85d7722b6f97f3a6732d54: drm/i915: fix initial fbdev setup warnings (2014-07-24 10:27:42 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2014-07-25-merged for you to fetch changes up to 4dac3edfe68e5e1b3c2216b84ba160572420fa40: Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next (2014-07-29 20:49:36 +0200) Armin Reese (1): drm/i915: Do not unmap object unless no other VMAs reference it Ben Widawsky (2): drm/i915/error: Check the potential ctx obj's vm drm/i915: Reorder ctx unref on ppgtt cleanup Borun Fu (1): drm/i915: Power gating display wells during i915_pm_suspend Chris Wilson (5): drm/i915: Handle failure to kick out a conflicting fb driver drm/i915: Abandon oom quickly if killed by a signal drm/i915: Initialise userptr mmu_notifier serial to 1 drm/i915: Allow overlapping userptr objects drm/i915/userptr: Keep spin_lock/unlock in the same block Damien Lespiau (3): drm/i915: PM irq enabling is generic on gen8, too drm/i915: Also give the sprite width for WM computation drm/i915: Make the WRPLL names const Daniel Vetter (14): drm/i915: ddi: enable runtime pm during dpms drm/i915: Run psr_setup unconditionally drm/i915: Add a FIXME about drrs/psr interactions drm/i915: Track the psr dp connector in dev_priv-psr.enabled drm/i915: Don't try to disable psr harder from the work item drm/i915: Lock down psr sw/hw state tracking drm/i915: More checks for psr.enabled drm/i915: Add locking to psr code drm/i915: Fix up PSR frontbuffer tracking drm/i915: Improve PSR debugfs output drm/i915: Remove redundant HAS_PSR checks drm/i915: Use genX_ prefix for gt irq enable/disable functions drm/i915: Ditch UMS config option Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next Deepak S (9): drm/i915: Read guaranteed freq for valleyview drm/i915: Add RP0/RP1/RPn render P state thresholds in VLV sysfs drm/i915: populate mem_freq/cz_clock for chv drm/i915: CHV GPU frequency to opcode functions drm/i915/chv: Add basic PM interrupt support for CHV drm/i915: Add RP1 render P state thresholds in CHV drm/i915: Force GPU Freq to lowest while suspending. drm/i915/chv: Drop WaGsvBringDownFreqInRc6 drm/i915: Fix printing proper min/min/rpe values in debugfs Fengguang Wu (1): drm/i915: byt_gpu_freq() can be static Jani Nikula (2): drm/i915: extract backlight minimum brightness from VBT drm/i915: respect the VBT minimum backlight brightness Jesse Barnes (5): drm/i915: don't warn if IRQs are disabled when shutting down display IRQs drm/i915: add helper for checking whether IRQs are enabled drm/i915: set pm._irqs_disabled at IRQ init time drm/i915: clear pm._irqs_disabled field after installing IRQs drm/i915: mark IRQs as disabled on unload Mika Kuoppala (1): drm/i915/chv: calculate rc6 residency correctly Paulo Zanoni (7): drm/i915: remove useless runtime PM get calls drm/i915: don't write powered down IRQ registers on Gen 8 drm/i915: HSW_BLC_PWM2_CTL doesn't exist on BDW drm/i915: extract and improve gen8_irq_power_well_post_enable drm/i915: reorganize the unclaimed register detection code drm/i915: BDW can also detect unclaimed registers drm/i915: remove plane/cursor/pipe assertions from intel_crtc_disable Rodrigo Vivi (2): drm/i915: Enable PSR by default. drm/i915: Fix possible overflow when recording semaphore states. Sagar Kamble (2): Documentation: drm: Removing placeholders for generic drm properties description Documentation: drm: describing rotation property Sonika Jindal (8): drm/i915: Adding HAS_GMCH_DISPLAY macro drm/i915: Allowing changing of wm latencies for valid platforms drm/i915: Returning the right VGA control reg for
Re: [Intel-gfx] [PATCH 1/1] Documentation: drm: describing drm properties exposed by various drivers
On Fri, Aug 01, 2014 at 02:58:21PM +0200, Laurent Pinchart wrote: Hi Randy, On Thursday 31 July 2014 15:16:21 Randy Dunlap wrote: On 05/12/14 11:04, Randy Dunlap wrote: On 05/12/2014 08:54 AM, Daniel Vetter wrote: On Mon, May 12, 2014 at 08:23:45AM -0700, Randy Dunlap wrote: On 05/12/2014 01:58 AM, Daniel Vetter wrote: On Mon, May 12, 2014 at 06:24:57PM +1000, Dave Airlie wrote: If we decide to go for property documentation inside the source code then I believe we'll have to create our own format, as creating a properties table from kerneldoc information extracted from comments is probably not possible. Can comeone pick up the ball here and figure out what needs to be done? The reason why I want a central place for the documentation is to force people to collaborate outside their own sandbox when adding properties. Whether that's docbook or some text file I don't care so much at this point. The fact that it's a central place should mandate that the patches changing it will go through dri-devel and so everyone should se them, and when adding new properties it would make the patch author more likely to look around a bit before adding another slighty incompatible version of the same property. If someone has a better suggestion how to encforce this I'm all ears. Of course this idea can still fail if our esteemed maintainer merges stuff without checking for violations of this policy. Dave, any thoughts on the subject? Yeah I'm happy to block merging stuff, if we can spot new properties when stuff is posted on dri-devel, so much the better, most drivers still send everything via dri-devel anyways, its only really Intel I have to worry about so far, I'll enforce that all prop stuff gets cc: dri-devel and that it has updates for the prop docs. But we should definitely add it to the new driver review checklist as well. I'm also on the side of this patch is ugly and makes my eyes burn, please please get a plan to use something else ASAP, I'm willing to merge this but I'm tempted to give it a lifetime of a kernel or two before I burn it. Ok, I'll try to move make kerneldoc suck less up the task list and maybe find someone to do it for me internally ;-) I certainly have no objections to making kerneldoc suck less. There was already an attempt to use asciidoc (like git uses) for kernel-doc (a few years ago, by Sam Ravnborg). I support(ed) that effort. Hm, do you have pointers to those? My google-fu seems lacking ... I googled for /kernel doc asciidoc ravnborg/ and found several hits for them. Ok, let's move this to the top and start discussions. The past few months I've written piles of kerneldoc comments for the DRM DocBook (all pulled in as kerneldoc, docbook .tmpl has just the chapter structure). DOC: sections are really useful to pull all the actual documentation out of the docbook xml into kerneldoc. But I've also done piles of docs for intel-gpu-tools, which is using gtkdoc. And there are some clear deficiencies: - No markdown for inline coments. Lack of lists and tables is hurting especially badly. If we add this (and I don't care one iota whether it's Yes, I've tried to add lists to kernel-doc notation but have failed so far. markdown or asciidoc or something else as long as it's readable plain text in comments) we should be able to move all the existing docbook xml paragraphs/lists/tables into kerneldoc comments. - Automatic cross-referencing of functions. If you place e.g. function() or #struct anywhere in a documentation comment gtk-doc automatically inserts a hyperlink to the relevant documentation page across the entire project. Really powerful and makes overview sections much more useful entry points for beginners since they can easily jump backforth betweeen the high-level overview and low-level per-function documentation. That's a nice-to-have IMO, but a really nice one. - In a really perfect world it would help if kerneldoc could collect structure member kerneldoc from per-member comments. Especially for large structures with lots of comments this would bring the kerneldoc and struct member much closer together. So that's my wishlist. OTOH, I would only want to add another way to do kernel-doc if it can be a full replacement for all of our docbook usage, i.e., it should provide a way that we can eliminate docbook and stop using it completely. Hm, I don't mind docbook at all, as long as all the real content is embedded into source files as kerneldoc and the docbook template just pulls in all the right bits and pieces. Even gtkdoc allos you to do that and pull in the different libararies (== header files with declarations for C) in the order you
Re: [Intel-gfx] [PULL] drm-intel-next
On 4 August 2014 17:10, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi Dave, Final feature pull for 3.17. drm-intel-next-2014-07-25: - Ditch UMS support (well just the config option for now) - Prep work for future platforms (Sonika Jindal, Damien) - runtime pm/soix fixes (Paulo, Jesse) - psr tracking improvements, locking fixes, now enabled by default! - rps fixes for chv (Deepak, Ville) - drm core patches for rotation support (Ville, Sagar Kamble) - the i915 parts unfortunately didn't make it yet - userptr fixes (Chris) - minimum backlight brightness (Jani), acked long ago by Matthew Garret on irc - I've forgotten about this patch :( QA is a bit unhappy about the DP MST stuff since it broke hpd testing a bit, but otherwise looks sane. I've backmerged drm-next to resolve conflicts with the mst stuff, which means the new tag itself doesn't contain the overview as usual. They are unhappy to do their job? I'm a bit confused. :-P Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw
On Thu, Jul 31, 2014 at 04:37:14PM +, Mcaulay, Alistair wrote: Hi Daniel, Something more like this then? (and revert the change to intel_ring_begin(), putting it back to how it was ) Yeah, roughly. Except that I would place the reload_in_reset wrapping in the i915_reset function. It is paramount that we never leak this outside of the dev-struct_mutex protection so that other threads can't ever observe this to be set. So putting it right next to the mutex locking is better. Also I think you've wrapped the wrong function - the re-init is done in i915_gem_init_hw, this here just resets the software state (mostly) and is done before the actual gpu hw reset is done. gem_init_hw is only run if the reset succeeds. -Daniel diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 991b663..b811ff2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1217,6 +1217,9 @@ struct i915_gpu_error { /* For missed irq/seqno simulation. */ unsigned int test_irq_rings; + + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ + bool reload_in_progress; }; enum modeset_restore { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b38e086..a25d3b5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error, if (i915_terminally_wedged(error)) return -EIO; - return -EAGAIN; + /* Check if GPU Reset is in progress */ + if (!error-reload_in_reset) + return -EAGAIN; } return 0; @@ -2579,6 +2581,8 @@ void i915_gem_reset(struct drm_device *dev) struct intel_engine_cs *ring; int i; + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ + dev_priv-gpu_error.reload_in_reset = true; /* * Before we free the objects from the requests, we need to inspect * them for finding the guilty party. As the requests only borrow @@ -2591,6 +2595,8 @@ void i915_gem_reset(struct drm_device *dev) i915_gem_reset_ring_cleanup(dev_priv, ring); i915_gem_restore_fences(dev); + + dev_priv-gpu_error.reload_in_reset = false; } -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, July 30, 2014 10:01 PM To: Mcaulay, Alistair Cc: Daniel Vetter; Chris Wilson; Ben Widawsky; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw On Wed, Jul 30, 2014 at 04:59:33PM +, Mcaulay, Alistair wrote: Hi Daniel, could you please be clearer on the change you mean. I think you mean something functionally equivalent to the code below, but done in a less hacky way. (This slight change has made no change to test results) Or is the idea to return at a different point to this? I couldn't find dev_priv-mm.reload_in_reset or similar in the code. The only thing I can find is error-reset_counter, which is used in check_wedge(). Bottom bit set means RESET_IN_PROGRESS, top bit means WEDGED Well I've meant that you have to add a new dev_prive-mm.realod_in_reset. And the below won't work since in all other places but when doing a gpu reset we want the -EAGAIN to reach callers. Actually it's really important that if we have an -EGAIN we don't eat it. And I guess the check for mm.reload_in_reset should actually be in gem_check_wedged. -Daniel --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1832,7 +1832,9 @@ int intel_ring_begin(struct intel_engine_cs *ring, ret = i915_gem_check_wedge(dev_priv-gpu_error, dev_priv-mm.interruptible); - if (ret) + + /* -EAGAIN means a reset is in progress, it is Ok to return */ + if (ret == -EAGAIN) + return 0; + if (ret) + return ret; ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t)); Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Tuesday, July 29, 2014 11:33 AM To: Chris Wilson; Daniel Vetter; Ben Widawsky; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw On Tue, Jul 29, 2014 at 08:36:33AM +0100, Chris Wilson wrote: On Mon, Jul 28, 2014 at 11:26:38AM +0200, Daniel Vetter wrote: Oh, I guess that's the tricky bit why the old approach never worked - because reset_in_progress is set we failed the context/ppgtt loading through the rings and screwed up. Problem with your
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of gunit registers need to be followed in PM suspend and resume path similar to runtime suspend and resume. v2: 1. Keeping GT access, wake, gunit save/restore related helpers static. 2. Moved GT access check, Wake Control, Gunit state save to end of i915_drm_freeze. 3. Reusing the sequence in runtime_suspend/resume path at macro level. Cc: Imre Deak imre.deak at intel.com Cc: Paulo Zanoni paulo.r.zanoni at intel.com Cc: Daniel Vetter daniel.vetter at ffwll.ch Cc: Jani Nikula jani.nikula at linux.intel.com Cc: Goel, Akash akash.g...@intel.com Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 39 +-- drivers/gpu/drm/i915/i915_drv.h | 1 + 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c4b25c..385dc74 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -490,11 +490,16 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) return true; } +static int vlv_runtime_suspend(struct drm_i915_private *dev_priv); +static int vlv_runtime_resume(struct drm_i915_private *dev_priv, + bool resume_from_s0ix); + static int i915_drm_freeze(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc; pci_power_t opregion_target_state; + int ret = 0; /* ignore lid events during suspend */ mutex_lock(dev_priv-modeset_restore_lock); @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure + * changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. + + return ret; } int i915_suspend(struct drm_device *dev, pm_message_t state) @@ -610,6 +620,12 @@ void intel_console_resume(struct work_struct *work) static int i915_drm_thaw_early(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; + int ret = 0; + + /* Restore Gunit State and allow wake - Need to make sure + * changes in vlv_runtime_resume path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_resume(dev_priv, true); if (IS_HASWELL(dev) || IS_BROADWELL(dev)) hsw_disable_pc8(dev_priv); @@ -618,7 +634,7 @@ static int i915_drm_thaw_early(struct drm_device *dev) intel_uncore_sanitize(dev); intel_power_domains_init_hw(dev_priv); - return 0; + return ret; } static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) @@ -1098,6 +1114,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv) s-gu_ctl0 = I915_READ(VLV_GU_CTL0); s-gu_ctl1 = I915_READ(VLV_GU_CTL1); s-clock_gate_dis2 = I915_READ(VLV_GUNIT_CLOCK_GATE2); + s-dpio_cfg_data= I915_READ(DPIO_CTL); /* * Not saving any of: @@ -1192,6 +1209,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GU_CTL0, s-gu_ctl0); I915_WRITE(VLV_GU_CTL1, s-gu_ctl1); I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s-clock_gate_dis2); + I915_WRITE(DPIO_CTL,s-dpio_cfg_data); } int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) @@ -1291,6 +1309,8 @@ static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR); } +/* This function is used in system suspend path as well to utilize + * Gfx clock, Wake control, Gunit state save related functionaility */ static int vlv_runtime_suspend(struct drm_i915_private *dev_priv) { u32 mask; @@ -1331,7 +1351,12 @@ err1: return err; } -static int vlv_runtime_resume(struct drm_i915_private *dev_priv) +/* This function is used in system resume path as well to utilize + * Gfx clock, Wake control, Gunit state restore related functionaility. + * GEM and other initialization will differ which will be controlled by + * resume_from_s0ix variable */ +static int vlv_runtime_resume(struct drm_i915_private *dev_priv, +
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: fix VDD state tracking after system resume
On Fri, Aug 01, 2014 at 02:56:54PM +0300, Ville Syrjälä wrote: On Thu, Jul 31, 2014 at 02:03:36PM +0300, Imre Deak wrote: Just like during booting the BIOS can leave the VDD bit enabled after system resume. So apply the same state sanitization there too. This fixes a problem where after resume the port power domain refcount gets unbalanced. v2: - unchanged v3: - call edp sanitizing from the encoder reset handler (Daniel) It happens a bit earlier than with the earlier attempt, but if it works it works. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()
On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote: On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote: On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote: On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote: Daniel, the only way intel_dp-is_mst can get reset is inside this path. Ok, so that one should be safe. Then I guess we can just push the locking down into the respective non-mst leafs (since atm we do link-retraining without any locking, which isn't good). And it needs to be dev-mode_config.mutex, not connection mutex. Why that? We can't be doing a modeset w/o connection_mutex so that seems like it should be enough. Well, there's also dpms which leaves the crtc-encoder-connector links intact but that too takes connection_mutex currently. I'd like to know why we do link training at this point though as well, adding locking is required of course, I was just going to wrap the short irq call to the link status check with the lock, but I think it should be possible to push it down further, I don't really know why the sink generates the hpd when we turn off the port, but that doesn't really matter I think. We need to be prepared for hpds at any time. intel_dp_check_link_status() just checks if there's a crtc, which there is (either the old one or the new one, depending on how far along the modeset path we are I guess), and then it just checks drm_dp_channel_eq_ok() which says false since the link was turned off, and then it proceeds to retrain the link. Maybe it should also check crtc-active? Though that itself won't eliminate the problem unless the locking gets fixed somehow. We check encoder-connectors_active, which is equivalent. -Daniel I'm not sure how vague the spec is on what should happen on HPDs, but if we drop the port clock we obviously will lose the link, but we should also know not to be retraining it at that point anyways. Dave. -- Ville Syrjälä Intel OTC -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
On Fri, Aug 01, 2014 at 05:07:30PM +0300, Ville Syrjälä wrote: On Thu, Jul 31, 2014 at 12:07:44PM -0700, Rodrigo Vivi wrote: According to spec FBC on BDW and HSW are identical without any gaps. So let's copy the nuke and let FBC really start compressing stuff. Without this patch we can verify with false color that nothing is being compressed. With the nuke in place and false color it is possible to see false color debugs. v2: Fix rebase conflict. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2908896..4ba3db1 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, { u32 flags = 0; u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES; + int ret; flags |= PIPE_CONTROL_CS_STALL; @@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; } - return gen8_emit_pipe_control(ring, flags, scratch_addr); + ret = gen8_emit_pipe_control(ring, flags, scratch_addr); + if (ret) + return ret; + + if (!invalidate_domains flush_domains) + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); + + return 0; } static void ring_write_tail(struct intel_engine_cs *ring, @@ -2065,7 +2073,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring, } intel_ring_advance(ring); - if (IS_GEN7(dev) !invalidate flush) + if (INTEL_INFO(dev)-gen = 7 !invalidate flush) return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); Apparently BDW has problems with LRIs to certain register offsets on !RCS and this here would hit that. The suggested workaround is to emit such LRIs only from RCS. Doing that would also involve tossing in a semaphore, so it feels like something that ought to be handled somewhere a bit higher up. *cough* sw frontbuffer tracking *cough* At least with fbc we have this nice igt test already. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Introduce FBC False Color for debug purposes.
On Fri, Aug 01, 2014 at 02:04:45AM -0700, Rodrigo Vivi wrote: With this bit enabled, HW changes the color when compressing frames for debug purposes. ALthough the simple way to enable a single bit is over intel_reg_write, this value is overwriten on next update_fbc so depending on the workload it is not possible to set this bit with intel-gpu-tools. So this patch introduces a persistent way to enable false color over debugfs. v2: Use DEFINE_SIMPLE_ATTRIBUTE as Daniel suggested v3: (Ville) only do false color for IVB+ since according to spec bit is MBZ before IVB. v4: We don't have FBC on valleyview nor on cherryview (Ben) v5: s/!HAS_PCH_SPLIT/!HAS_FBC (Ville) Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt
On Thu, Jul 31, 2014 at 07:15:52PM +0300, Ville Syrjälä wrote: On Wed, Jul 30, 2014 at 09:42:02PM +0200, Daniel Vetter wrote: Stuffing this into the context setup code doesn't make a lot of sense. Also reusing the real ppgtt setup code makes even less sense since the aliasing ppgtt isn't a real address space. Leaving all that stuff unitialized will make sure that we catch any abusers promptly. This is also a prep work to clean up the context-ppgtt link. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_context.c | 13 + drivers/gpu/drm/i915/i915_gem_gtt.c | 31 +-- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3b8367aa8404..7a455fcee3a7 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -276,17 +276,6 @@ i915_gem_create_context(struct drm_device *dev, goto err_unpin; } else ctx-vm = ppgtt-base; - - /* This case is reserved for the global default context and -* should only happen once. */ - if (is_global_default_ctx) { - if (WARN_ON(dev_priv-mm.aliasing_ppgtt)) { - ret = -EEXIST; - goto err_unpin; - } - - dev_priv-mm.aliasing_ppgtt = ppgtt; - } } else if (USES_PPGTT(dev)) { /* For platforms which only have aliasing PPGTT, we fake the * address space and refcounting. */ @@ -361,7 +350,7 @@ int i915_gem_context_init(struct drm_device *dev) } } - ctx = i915_gem_create_context(dev, NULL, USES_PPGTT(dev)); + ctx = i915_gem_create_context(dev, NULL, USES_FULL_PPGTT(dev)); if (IS_ERR(ctx)) { DRM_ERROR(Failed to create default global context (error %ld)\n, PTR_ERR(ctx)); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 4fa7807ed4d5..cb9bb13ff20a 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1191,23 +1191,27 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) return 0; } -int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) +int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) { struct drm_i915_private *dev_priv = dev-dev_private; - int ret = 0; ppgtt-base.dev = dev; ppgtt-base.scratch = dev_priv-gtt.base.scratch; if (INTEL_INFO(dev)-gen 8) - ret = gen6_ppgtt_init(ppgtt); + return gen6_ppgtt_init(ppgtt); else if (IS_GEN8(dev)) - ret = gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total); + return gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total); else BUG(); +} +int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + int ret = 0; + ret = __hw_ppgtt_init(dev, ppgtt); if (!ret) { - struct drm_i915_private *dev_priv = dev-dev_private; kref_init(ppgtt-ref); drm_mm_init(ppgtt-base.mm, ppgtt-base.start, ppgtt-base.total); @@ -1728,6 +1732,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, struct drm_mm_node *entry; struct drm_i915_gem_object *obj; unsigned long hole_start, hole_end; + int ret; BUG_ON(mappable_end end); @@ -1739,7 +1744,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* Mark any preallocated objects as occupied */ list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) { struct i915_vma *vma = i915_gem_obj_to_vma(obj, ggtt_vm); - int ret; + DRM_DEBUG_KMS(reserving preallocated space: %lx + %zx\n, i915_gem_obj_ggtt_offset(obj), obj-base.size); @@ -1766,6 +1771,20 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* And finally clear the reserved guard page */ ggtt_vm-clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true); + if (HAS_ALIASING_PPGTT(dev) USES_FULL_PPGTT(dev)) { Should that be !USES_FULL_PPGTT ? Yup. And since I've also fumbled the check for __hw_ppgtt_init I really wonder what exactly I've tested on my gen6 here ... Time to hide and lick wounds I think ;-) -Daniel + struct i915_hw_ppgtt *ppgtt; + + ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); + if (!ppgtt) + return -ENOMEM; + + ret = __hw_ppgtt_init(dev, ppgtt); + if (!ret) + return ret; + +
Re: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
On Thu, Jul 31, 2014 at 11:55:58AM +, Mcaulay, Alistair wrote: Hi Jeff, Some more comments in the code. Can you please fix your mailer to properly quote? Just doing in-line comments makes it really hard to find them. -Daniel Thanks, Alistair. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of jeff.mc...@intel.com Sent: Thursday, July 31, 2014 3:00 AM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV From: Jeff McGee jeff.mc...@intel.com Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register. I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only! Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_dma.c | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 13 drivers/gpu/drm/i915/intel_sseu.c | 64 +++ drivers/gpu/drm/i915/intel_sseu.h | 2 ++ 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_sseu.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \ i915_irq.o \ i915_trace_points.o \ intel_ringbuffer.o \ - intel_uncore.o + intel_uncore.o \ + intel_sseu.o # autogenerated null render state i915-y += intel_renderstate_gen6.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_gem_unload; } + intel_sseu_init(dev); + intel_power_domains_init(dev_priv); if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5624,6 +5624,19 @@ enum punit_power_well { #define GEN7_MISCCPCTL (0x9424) #define GEN7_DOP_CLOCK_GATE_ENABLE (10) +/* Fuse readout registers for GT */ +#define CHV_FUSE_GT 0x182168 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 10 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 11 These should be: #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0(1 10) #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1(1 11) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xf16) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT 16 Why not use the shift define here? #define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xfCHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT) and for the others. +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK (0xf20) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT 20 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK (0xf24) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT 24 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK (0xf28) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT 28 + /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST1 0xB008 /* L3CD Error Status 1 */ #define HSW_L3CDERRST11 0xB208 /* L3CD Error Status register 1 slice 1 */ diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c new file mode 100644 index 000..6ba4830 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.c @@ -0,0 +1,64 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person +obtaining a + * copy of this software and associated documentation files (the +Software), + * to deal in the Software without restriction, including without +limitation + * the rights to use, copy, modify, merge, publish, distribute, +sublicense, + * and/or sell copies of the Software, and to permit persons to whom +the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the +next + * paragraph) shall be included in all copies or substantial portions +of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +SHALL +
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Slice/Subslice/EU info via GETPARAM
On Wed, Jul 30, 2014 at 08:59:46PM -0500, jeff.mc...@intel.com wrote: From: Jeff McGee jeff.mc...@intel.com Define a struct to capture information on the device's Slice/Subslice/EU (SSEU) configuration. Add this struct to the main device info struct. Define a packed bitfield form for the SSEU info and share it with userspace via a new GETPARAM option. Starting with Cherryview, devices may have a varying number of EU for a given ID due to creative fusing. The surest way to determine the configuration is by reading fuses which is best done in the kernel and communicated to userspace. The immediate need from userspace is to determine the number of threads of compute work that can be safely submitted. The definition of SSEU as a new drm/i915 component, with its own header file and soon-to-be source file, is in anticipation of lots of upcoming code for its management, particularly the power gating functionality. Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_sseu.h | 40 +++ include/uapi/drm/i915_drm.h | 18 ++ 4 files changed, 64 insertions(+) create mode 100644 drivers/gpu/drm/i915/intel_sseu.h diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..f581848 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_CMD_PARSER_VERSION: value = i915_cmd_parser_get_version(); break; + case I915_PARAM_SSEU_INFO: + value = INTEL_INFO(dev)-sseu.gp_sseu_info; + break; default: DRM_DEBUG(Unknown parameter %d\n, param-param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 18c9ad8..01adafd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -45,6 +45,7 @@ #include linux/intel-iommu.h #include linux/kref.h #include linux/pm_qos.h +#include intel_sseu.h /* General customization: */ @@ -562,6 +563,8 @@ struct intel_device_info { int trans_offsets[I915_MAX_TRANSCODERS]; int palette_offsets[I915_MAX_PIPES]; int cursor_offsets[I915_MAX_PIPES]; + /* Slice/Subslice/EU info */ + struct intel_sseu_info sseu; }; #undef DEFINE_FLAG diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h new file mode 100644 index 000..7db7175 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.h @@ -0,0 +1,40 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ +#ifndef _INTEL_SSEU_H_ +#define _INTEL_SSEU_H_ + +struct intel_sseu_info { + /* Total slice count */ + unsigned int slice_cnt; + /* Total subslice count */ + unsigned int subslice_cnt; + /* Total execution unit count */ + unsigned int eu_cnt; + /* Thread count per EU */ + unsigned int threads_per_eu; + /* Bit field representation for I915_PARAM_SSEU_INFO */ + u32 gp_sseu_info; +}; + +#endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index ff57f07..b99c1a2 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -171,6 +171,23 @@ typedef struct _drm_i915_sarea { #define I915_BOX_TEXTURE_LOAD 0x8 #define I915_BOX_LOST_CONTEXT 0x10 +/* + * Slice/Subslice/EU Info + * - Accessed via GETPARAM ioctl option I915_PARAM_SSEU_INFO + * - SLICE_CNT: total slice count + * - SUBSLICE_CNT: total subslice count + * - EU_CNT: total execution unit count + * - THREADS_PER_EU: thread count per EU
Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()
On 4 August 2014 18:10, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote: On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote: On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote: On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote: Daniel, the only way intel_dp-is_mst can get reset is inside this path. Ok, so that one should be safe. Then I guess we can just push the locking down into the respective non-mst leafs (since atm we do link-retraining without any locking, which isn't good). And it needs to be dev-mode_config.mutex, not connection mutex. Why that? We can't be doing a modeset w/o connection_mutex so that seems like it should be enough. Well, there's also dpms which leaves the crtc-encoder-connector links intact but that too takes connection_mutex currently. I'd like to know why we do link training at this point though as well, adding locking is required of course, I was just going to wrap the short irq call to the link status check with the lock, but I think it should be possible to push it down further, I don't really know why the sink generates the hpd when we turn off the port, but that doesn't really matter I think. We need to be prepared for hpds at any time. intel_dp_check_link_status() just checks if there's a crtc, which there is (either the old one or the new one, depending on how far along the modeset path we are I guess), and then it just checks drm_dp_channel_eq_ok() which says false since the link was turned off, and then it proceeds to retrain the link. Maybe it should also check crtc-active? Though that itself won't eliminate the problem unless the locking gets fixed somehow. We check encoder-connectors_active, which is equivalent. -Daniel We still check that. if you mean intel_encoders-connectors_active. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
On Wed, Jul 30, 2014 at 08:59:47PM -0500, jeff.mc...@intel.com wrote: From: Jeff McGee jeff.mc...@intel.com Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register. I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only! Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_dma.c | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 13 drivers/gpu/drm/i915/intel_sseu.c | 64 +++ drivers/gpu/drm/i915/intel_sseu.h | 2 ++ 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_sseu.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \ i915_irq.o \ i915_trace_points.o \ intel_ringbuffer.o \ - intel_uncore.o + intel_uncore.o \ + intel_sseu.o # autogenerated null render state i915-y += intel_renderstate_gen6.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto out_gem_unload; } + intel_sseu_init(dev); + intel_power_domains_init(dev_priv); if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5624,6 +5624,19 @@ enum punit_power_well { #define GEN7_MISCCPCTL (0x9424) #define GEN7_DOP_CLOCK_GATE_ENABLE (10) +/* Fuse readout registers for GT */ +#define CHV_FUSE_GT 0x182168 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 10 +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 11 +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xf16) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT 16 +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK (0xf20) +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT 20 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK (0xf24) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT 24 +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK (0xf28) +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT 28 + /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST1 0xB008 /* L3CD Error Status 1 */ #define HSW_L3CDERRST11 0xB208 /* L3CD Error Status register 1 slice 1 */ diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c new file mode 100644 index 000..6ba4830 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_sseu.c @@ -0,0 +1,64 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ +#include linux/bitops.h +#include i915_drv.h +#include intel_sseu.h + +void intel_sseu_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_sseu_info sseu_info; + u32 gp = 0; + + /* Collect SSEU info per device */ + if (IS_CHERRYVIEW(dev)) { + u32 fuse, mask_ss, mask_eu; + + fuse = I915_READ(CHV_FUSE_GT); + mask_ss = fuse (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 | + CHV_FUSE_GT_SUBSLICE_DISABLE_SS1); +
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Remove now useless comments about the translation values
On Fri, Aug 01, 2014 at 12:29:22PM -0300, Paulo Zanoni wrote: 2014-08-01 7:07 GMT-03:00 Damien Lespiau damien.lesp...@intel.com: We used to carry a default HDMI value in entry 9, but this entry got removed for both HSW and BDW. Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Pulled in entire series, thanks. I'll shuffle the first two for 3.17 when I split my queue. We can cc: stable if they are known to make an actual user happier ;-) -Daniel Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_ddi.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 80526ab..ab5f65f 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -33,7 +33,7 @@ * automatically adapt to HDMI connections as well */ static const u32 hsw_ddi_translations_dp[] = { - 0x00FF, 0x0006000E, /* DP parameters */ + 0x00FF, 0x0006000E, 0x00D75FFF, 0x0005000A, 0x00C30FFF, 0x00040006, 0x80AAAFFF, 0x000B, @@ -45,7 +45,7 @@ static const u32 hsw_ddi_translations_dp[] = { }; static const u32 hsw_ddi_translations_fdi[] = { - 0x00FF, 0x0007000E, /* FDI parameters */ + 0x00FF, 0x0007000E, 0x00D75FFF, 0x000F000A, 0x00C30FFF, 0x00060006, 0x00AAAFFF, 0x001E, @@ -73,7 +73,7 @@ static const u32 hsw_ddi_translations_hdmi[] = { }; static const u32 bdw_ddi_translations_edp[] = { - 0x00FF, 0x0012, /* eDP parameters */ + 0x00FF, 0x0012, 0x00EBAFFF, 0x00020011, 0x00C71FFF, 0x0006000F, 0x00AAAFFF, 0x000E000A, @@ -85,7 +85,7 @@ static const u32 bdw_ddi_translations_edp[] = { }; static const u32 bdw_ddi_translations_dp[] = { - 0x00FF, 0x0007000E, /* DP parameters */ + 0x00FF, 0x0007000E, 0x00D75FFF, 0x000E000A, 0x00BE, 0x00140006, 0x80B2CFFF, 0x001B0002, @@ -97,7 +97,7 @@ static const u32 bdw_ddi_translations_dp[] = { }; static const u32 bdw_ddi_translations_fdi[] = { - 0x00FF, 0x0001000E, /* FDI parameters */ + 0x00FF, 0x0001000E, 0x00D75FFF, 0x0004000A, 0x00C30FFF, 0x00070006, 0x00AAAFFF, 0x000C, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests: Add drv_workarounds
On Fri, Aug 01, 2014 at 08:46:25PM +0300, Mika Kuoppala wrote: To check that static workarounds are set and stay after init, hang and suspend/restore. Checks are currently provided for ivb and bdw only. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- lib/intel_chipset.h |4 + lib/intel_workaround.h | 142 +++ tests/Makefile.sources |1 + tests/drv_workarounds.c | 244 +++ 4 files changed, 391 insertions(+) create mode 100644 lib/intel_workaround.h create mode 100644 tests/drv_workarounds.c diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h index 404c632..5a03f2b 100644 --- a/lib/intel_chipset.h +++ b/lib/intel_chipset.h @@ -263,6 +263,10 @@ void intel_check_pch(void); (devid) == PCI_CHIP_IVYBRIDGE_S || \ (devid) == PCI_CHIP_IVYBRIDGE_S_GT2) +#define IS_IVB_GT1(devid)((devid) == PCI_CHIP_IVYBRIDGE_GT1 || \ + (devid) == PCI_CHIP_IVYBRIDGE_M_GT1 || \ + (devid) == PCI_CHIP_IVYBRIDGE_S) + #define IS_VALLEYVIEW(devid) ((devid) == PCI_CHIP_VALLEYVIEW_PO || \ (devid) == PCI_CHIP_VALLEYVIEW_1 || \ (devid) == PCI_CHIP_VALLEYVIEW_2 || \ diff --git a/lib/intel_workaround.h b/lib/intel_workaround.h new file mode 100644 index 000..87e3a44 --- /dev/null +++ b/lib/intel_workaround.h @@ -0,0 +1,142 @@ +/* + * Copyright © 2013,2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Mika Kuoppala mika.kuopp...@intel.com + * + */ + +#ifndef INTEL_WORKAROUNDS +#define INTEL_WORKAROUNDS + +#include intel_chipset.h +#include intel_io.h +#include igt_core.h + +static int __wa_devid = 0; + +struct wa { + const char * const name; + int (*f)(const int devid); +}; + +static void wa_init(const int devid) +{ + __wa_devid = devid; + intel_register_access_init(intel_get_pci_device(), 0); +} + +static void wa_fini(void) +{ + __wa_devid = 0; + intel_register_access_fini(); +} + +static int wa_check(const struct wa *wa) +{ + igt_assert(__wa_devid); + igt_assert(wa-name); + igt_assert(wa-f); + return wa-f(__wa_devid); +} + +#define WA(_name) /* _name */ static int _name##_fun(const int devid); \ + static const struct wa _name = \ + { #_name, _name##_fun };\ + static int _name##_fun(const int devid) + +#define wa_assert_m(reg, val, mask) { \ + unsigned int x; \ + igt_assert(mask); \ + x = intel_register_read(reg); \ + if (((x) (mask)) != (val)) { \ + igt_warn(a:0x%08x r:0x%08x m:0%08x v:0%08x\n, reg, x, mask, val); \ + return 1; \ + } \ + } + +#define wa_assert(reg, val) wa_assert_m(reg, val, 0x) +#define wa_assert_bit_set(reg, bit) wa_assert_m(reg, (1 bit), (1 bit)) +#define wa_assert_bit_clr(reg, bit) wa_assert_m(reg, 0, (1 bit)) + +#define WA_MASK(_name, reg, val, mask) \ + WA(_name) { \ + wa_assert_m(reg, val, mask);\ + return 0; \ + } + +#define WA_BIT_SET(_name, reg, bit) WA_MASK(_name, reg, (1 bit), (1 bit)) +#define WA_BIT_CLR(_name, reg, bit) WA_MASK(_name, reg, 0, (1 bit)) + + +/*
[Intel-gfx] [PATCH] drm/i915: Don't require dev-struct_mutex in psr_match_conditions
Since I've reworked psr support to no longer require x-tiling we don't check any state protected by the Giant GEM Lock. So drop that check. Also boo for lockdep_assert_held for not yelling when lockdep is disabled. Cc: Paulo Zanoni przan...@gmail.com Reported-by: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_dp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3e6100ea7295..6dbe0f84d455 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1773,7 +1773,6 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); lockdep_assert_held(dev_priv-psr.lock); - lockdep_assert_held(dev-struct_mutex); WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex)); WARN_ON(!drm_modeset_is_locked(crtc-mutex)); -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: remove duplicate register defines
On Fri, Aug 01, 2014 at 04:19:54PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com cat i915_reg.h | sort | uniq -d | grep define Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/i915_reg.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..c45b679 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3755,7 +3755,6 @@ enum punit_power_well { #define PIPE_VSYNC_INTERRUPT_STATUS(1UL9) #define PIPE_DISPLAY_LINE_COMPARE_STATUS (1UL8) #define PIPE_DPST_EVENT_STATUS (1UL7) -#define PIPE_LEGACY_BLC_EVENT_STATUS (1UL6) #define PIPE_A_PSR_STATUS_VLV (1UL6) #define PIPE_LEGACY_BLC_EVENT_STATUS (1UL6) #define PIPE_ODD_FIELD_INTERRUPT_STATUS(1UL5) @@ -5430,7 +5429,6 @@ enum punit_power_well { #define VLV_GTLC_ALLOWWAKEERR (1 1) #define VLV_GTLC_PW_MEDIA_STATUS_MASK (1 5) #define VLV_GTLC_PW_RENDER_STATUS_MASK (1 7) -#define VLV_GTLC_SURVIVABILITY_REG 0x130098 #define FORCEWAKE_MT0xa188 /* multi-threaded */ #define FORCEWAKE_KERNEL 0x1 #define FORCEWAKE_USER 0x2 -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel-gpu-tools: Change type of variable from unsigned to uint64_t in gem_stress
On Fri, Aug 01, 2014 at 12:10:44PM +0700, Pavel Popov wrote: To run gem_stress with the correct number of buffers even if aperture size = 4GB. Signed-off-by: Pavel Popov pavel.e.po...@intel.com --- tests/gem_stress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gem_stress.c b/tests/gem_stress.c index 3bbe487..313a82a 100644 --- a/tests/gem_stress.c +++ b/tests/gem_stress.c @@ -737,7 +737,7 @@ static int parse_options(int opt, int opt_index) static void init(void) { int i; - unsigned tmp; + uint64_t tmp; We have tons of places which use gem_aperture_size. Have you reviewed them all? -Daniel if (options.num_buffers == 0) { tmp = gem_aperture_size(drm_fd); -- 1.9.1 Closed Joint Stock Company Intel A/O Registered legal address: Krylatsky Hills Business Park, 17 Krylatskaya Str., Bldg 4, Moscow 121614, Russian Federation This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Add rotation_property to mode_config and creating it
On 7/31/2014 9:39 AM, Jindal, Sonika wrote: On 7/29/2014 4:00 PM, Daniel Vetter wrote: On Tue, Jul 29, 2014 at 12:40:29PM +0300, Ville Syrjälä wrote: On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote: On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote: On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com v2: Adding creation of rotation_property here. Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_crtc.c |3 ++- include/drm/drm_crtc.h |1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 787631e..49c0747 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1299,7 +1299,8 @@ static int drm_mode_create_standard_plane_properties(struct drm_device *dev) type, drm_plane_type_enum_list, ARRAY_SIZE(drm_plane_type_enum_list)); dev-mode_config.plane_type_property = type; - + dev-mode_config.rotation_property = drm_mode_create_rotation_property(dev, + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); This might not make sense for other (!i915) hardware. And that's the reason why I had the driver create the property in the first place. I think Daniel was thinking that we might want to expose all the bits regardless of what the hardware supports, but I don't like that idea. There are other properties (eg. alpha blending, csc stuff, etc.) that have the same problem of hardware supporting only a (potentially small) subset of the possible values. I'd rather we didn't make life harder for userspace when the kernel can already report that certain values will never work. Well I'd like the property to be in some generic place so that fbcon can unroate and that with the atomic stuff we can have rotation support in the core structures. Which should help with argument checking. But for rotation I don't think we should set it up in generic code, but in i915. So the place where we keep it would be generic, the values would be the sames, but the allowed set would differ depending upon platform or driver. That would still fail if all the planes on the same device don't support the same rotation flags. Eg. on i915 we would have this problem if we exposed the old video overlay as a drm plane. And it wouldn't be the first piece of hardware where I've seen this kind of thing. Problem is still that I'd like to have a somewhat generic internal representation available. We could punt this to atomic though by adding a rotation field to the drm_plane_state structure. Which is more-or-less my plan, but wouldn't work with Rob's approach. Or we keep the property link only in drm_plane (and give drivers the freedom to set up the underlying enum however they want to), but I'm not sure whether our interfaces can cope with that. -Daniel Daniel, Ville So what is the suggestion for this property? Should I be moving it to somewhere else? -Sonika Hi Daniel/Ville, Please let me know if I need to move this property somewhere else. Thanks, Sonika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Add rotation_property to mode_config and creating it
On Mon, Aug 04, 2014 at 03:59:23PM +0530, Jindal, Sonika wrote: On 7/31/2014 9:39 AM, Jindal, Sonika wrote: On 7/29/2014 4:00 PM, Daniel Vetter wrote: On Tue, Jul 29, 2014 at 12:40:29PM +0300, Ville Syrjälä wrote: On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote: On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote: On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com v2: Adding creation of rotation_property here. Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_crtc.c |3 ++- include/drm/drm_crtc.h |1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 787631e..49c0747 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1299,7 +1299,8 @@ static int drm_mode_create_standard_plane_properties(struct drm_device *dev) type, drm_plane_type_enum_list, ARRAY_SIZE(drm_plane_type_enum_list)); dev-mode_config.plane_type_property = type; - + dev-mode_config.rotation_property = drm_mode_create_rotation_property(dev, + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); This might not make sense for other (!i915) hardware. And that's the reason why I had the driver create the property in the first place. I think Daniel was thinking that we might want to expose all the bits regardless of what the hardware supports, but I don't like that idea. There are other properties (eg. alpha blending, csc stuff, etc.) that have the same problem of hardware supporting only a (potentially small) subset of the possible values. I'd rather we didn't make life harder for userspace when the kernel can already report that certain values will never work. Well I'd like the property to be in some generic place so that fbcon can unroate and that with the atomic stuff we can have rotation support in the core structures. Which should help with argument checking. But for rotation I don't think we should set it up in generic code, but in i915. So the place where we keep it would be generic, the values would be the sames, but the allowed set would differ depending upon platform or driver. That would still fail if all the planes on the same device don't support the same rotation flags. Eg. on i915 we would have this problem if we exposed the old video overlay as a drm plane. And it wouldn't be the first piece of hardware where I've seen this kind of thing. Problem is still that I'd like to have a somewhat generic internal representation available. We could punt this to atomic though by adding a rotation field to the drm_plane_state structure. Which is more-or-less my plan, but wouldn't work with Rob's approach. Or we keep the property link only in drm_plane (and give drivers the freedom to set up the underlying enum however they want to), but I'm not sure whether our interfaces can cope with that. -Daniel Daniel, Ville So what is the suggestion for this property? Should I be moving it to somewhere else? -Sonika Hi Daniel/Ville, Please let me know if I need to move this property somewhere else. Well we at least need to move the crationg of the property to the driver. I guess we could leave the property itself in place in mode_config so that fbdev can get at it easily. If we come across a real need for separate per-plane rotation properties we can always move it to drm_plane later. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm: Resetting rotation property
On Tue, Jul 15, 2014 at 02:10:29PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Reset rotation property to 0 wherever applicable Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 3144db9..961afcb 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -345,9 +345,17 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) drm_warn_on_modeset_not_all_locked(dev); - list_for_each_entry(plane, dev-mode_config.plane_list, head) + list_for_each_entry(plane, dev-mode_config.plane_list, head) { + + if (dev-mode_config.rotation_property) { + drm_object_property_set_value(plane-base, + dev-mode_config.rotation_property, + BIT(DRM_ROTATE_0)); + } + if (plane-type != DRM_PLANE_TYPE_PRIMARY) drm_plane_force_disable(plane); I would reset the property after disabling the plane. + } for (i = 0; i fb_helper-crtc_count; i++) { struct drm_mode_set *mode_set = fb_helper-crtc_info[i].mode_set; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm: Resetting rotation property
On 8/4/2014 5:31 PM, Ville Syrjälä wrote: On Tue, Jul 15, 2014 at 02:10:29PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Reset rotation property to 0 wherever applicable Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 3144db9..961afcb 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -345,9 +345,17 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) drm_warn_on_modeset_not_all_locked(dev); - list_for_each_entry(plane, dev-mode_config.plane_list, head) + list_for_each_entry(plane, dev-mode_config.plane_list, head) { + + if (dev-mode_config.rotation_property) { + drm_object_property_set_value(plane-base, + dev-mode_config.rotation_property, + BIT(DRM_ROTATE_0)); + } + if (plane-type != DRM_PLANE_TYPE_PRIMARY) drm_plane_force_disable(plane); I would reset the property after disabling the plane. Ok, I will change that. + } for (i = 0; i fb_helper-crtc_count; i++) { struct drm_mode_set *mode_set = fb_helper-crtc_info[i].mode_set; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Add rotation_property to mode_config and creating it
On 8/4/2014 5:24 PM, Ville Syrjälä wrote: On Mon, Aug 04, 2014 at 03:59:23PM +0530, Jindal, Sonika wrote: On 7/31/2014 9:39 AM, Jindal, Sonika wrote: On 7/29/2014 4:00 PM, Daniel Vetter wrote: On Tue, Jul 29, 2014 at 12:40:29PM +0300, Ville Syrjälä wrote: On Mon, Jul 28, 2014 at 08:47:22PM +0200, Daniel Vetter wrote: On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote: On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com v2: Adding creation of rotation_property here. Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_crtc.c |3 ++- include/drm/drm_crtc.h |1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 787631e..49c0747 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1299,7 +1299,8 @@ static int drm_mode_create_standard_plane_properties(struct drm_device *dev) type, drm_plane_type_enum_list, ARRAY_SIZE(drm_plane_type_enum_list)); dev-mode_config.plane_type_property = type; - + dev-mode_config.rotation_property = drm_mode_create_rotation_property(dev, + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); This might not make sense for other (!i915) hardware. And that's the reason why I had the driver create the property in the first place. I think Daniel was thinking that we might want to expose all the bits regardless of what the hardware supports, but I don't like that idea. There are other properties (eg. alpha blending, csc stuff, etc.) that have the same problem of hardware supporting only a (potentially small) subset of the possible values. I'd rather we didn't make life harder for userspace when the kernel can already report that certain values will never work. Well I'd like the property to be in some generic place so that fbcon can unroate and that with the atomic stuff we can have rotation support in the core structures. Which should help with argument checking. But for rotation I don't think we should set it up in generic code, but in i915. So the place where we keep it would be generic, the values would be the sames, but the allowed set would differ depending upon platform or driver. That would still fail if all the planes on the same device don't support the same rotation flags. Eg. on i915 we would have this problem if we exposed the old video overlay as a drm plane. And it wouldn't be the first piece of hardware where I've seen this kind of thing. Problem is still that I'd like to have a somewhat generic internal representation available. We could punt this to atomic though by adding a rotation field to the drm_plane_state structure. Which is more-or-less my plan, but wouldn't work with Rob's approach. Or we keep the property link only in drm_plane (and give drivers the freedom to set up the underlying enum however they want to), but I'm not sure whether our interfaces can cope with that. -Daniel Daniel, Ville So what is the suggestion for this property? Should I be moving it to somewhere else? -Sonika Hi Daniel/Ville, Please let me know if I need to move this property somewhere else. Well we at least need to move the crationg of the property to the driver. I guess we could leave the property itself in place in mode_config so that fbdev can get at it easily. If we come across a real need for separate per-plane rotation properties we can always move it to drm_plane later. Ok, I will move it back to plane_create function. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()
On Mon, Aug 04, 2014 at 10:10:50AM +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote: On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote: On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote: On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote: Daniel, the only way intel_dp-is_mst can get reset is inside this path. Ok, so that one should be safe. Then I guess we can just push the locking down into the respective non-mst leafs (since atm we do link-retraining without any locking, which isn't good). And it needs to be dev-mode_config.mutex, not connection mutex. Why that? We can't be doing a modeset w/o connection_mutex so that seems like it should be enough. Well, there's also dpms which leaves the crtc-encoder-connector links intact but that too takes connection_mutex currently. I'd like to know why we do link training at this point though as well, adding locking is required of course, I was just going to wrap the short irq call to the link status check with the lock, but I think it should be possible to push it down further, I don't really know why the sink generates the hpd when we turn off the port, but that doesn't really matter I think. We need to be prepared for hpds at any time. intel_dp_check_link_status() just checks if there's a crtc, which there is (either the old one or the new one, depending on how far along the modeset path we are I guess), and then it just checks drm_dp_channel_eq_ok() which says false since the link was turned off, and then it proceeds to retrain the link. Maybe it should also check crtc-active? Though that itself won't eliminate the problem unless the locking gets fixed somehow. We check encoder-connectors_active, which is equivalent. Only after intel_sanitize_encoder(). Before that we can have !crtc-active encoder-connectors_active. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] Documentation: drm: describing drm properties exposed by various drivers
On Monday 04 August 2014 09:30:04 Daniel Vetter wrote: On Fri, Aug 01, 2014 at 02:58:21PM +0200, Laurent Pinchart wrote: On Thursday 31 July 2014 15:16:21 Randy Dunlap wrote: On 05/12/14 11:04, Randy Dunlap wrote: On 05/12/2014 08:54 AM, Daniel Vetter wrote: On Mon, May 12, 2014 at 08:23:45AM -0700, Randy Dunlap wrote: On 05/12/2014 01:58 AM, Daniel Vetter wrote: On Mon, May 12, 2014 at 06:24:57PM +1000, Dave Airlie wrote: If we decide to go for property documentation inside the source code then I believe we'll have to create our own format, as creating a properties table from kerneldoc information extracted from comments is probably not possible. Can comeone pick up the ball here and figure out what needs to be done? The reason why I want a central place for the documentation is to force people to collaborate outside their own sandbox when adding properties. Whether that's docbook or some text file I don't care so much at this point. The fact that it's a central place should mandate that the patches changing it will go through dri-devel and so everyone should se them, and when adding new properties it would make the patch author more likely to look around a bit before adding another slighty incompatible version of the same property. If someone has a better suggestion how to encforce this I'm all ears. Of course this idea can still fail if our esteemed maintainer merges stuff without checking for violations of this policy. Dave, any thoughts on the subject? Yeah I'm happy to block merging stuff, if we can spot new properties when stuff is posted on dri-devel, so much the better, most drivers still send everything via dri-devel anyways, its only really Intel I have to worry about so far, I'll enforce that all prop stuff gets cc: dri-devel and that it has updates for the prop docs. But we should definitely add it to the new driver review checklist as well. I'm also on the side of this patch is ugly and makes my eyes burn, please please get a plan to use something else ASAP, I'm willing to merge this but I'm tempted to give it a lifetime of a kernel or two before I burn it. Ok, I'll try to move make kerneldoc suck less up the task list and maybe find someone to do it for me internally ;-) I certainly have no objections to making kerneldoc suck less. There was already an attempt to use asciidoc (like git uses) for kernel-doc (a few years ago, by Sam Ravnborg). I support(ed) that effort. Hm, do you have pointers to those? My google-fu seems lacking ... I googled for /kernel doc asciidoc ravnborg/ and found several hits for them. Ok, let's move this to the top and start discussions. The past few months I've written piles of kerneldoc comments for the DRM DocBook (all pulled in as kerneldoc, docbook .tmpl has just the chapter structure). DOC: sections are really useful to pull all the actual documentation out of the docbook xml into kerneldoc. But I've also done piles of docs for intel-gpu-tools, which is using gtkdoc. And there are some clear deficiencies: - No markdown for inline coments. Lack of lists and tables is hurting especially badly. If we add this (and I don't care one iota whether it's Yes, I've tried to add lists to kernel-doc notation but have failed so far. markdown or asciidoc or something else as long as it's readable plain text in comments) we should be able to move all the existing docbook xml paragraphs/lists/tables into kerneldoc comments. - Automatic cross-referencing of functions. If you place e.g. function() or #struct anywhere in a documentation comment gtk-doc automatically inserts a hyperlink to the relevant documentation page across the entire project. Really powerful and makes overview sections much more useful entry points for beginners since they can easily jump backforth betweeen the high-level overview and low- level per-function documentation. That's a nice-to-have IMO, but a really nice one. - In a really perfect world it would help if kerneldoc could collect structure member kerneldoc from per-member comments. Especially for large structures with lots of comments this would bring the kerneldoc and struct member much closer together. So that's my wishlist. OTOH, I would only want to add another way to do kernel-doc if it can be a full replacement for all of our docbook usage, i.e., it should provide a way that we can eliminate docbook and stop using it completely. Hm, I don't mind docbook at all, as long as all the real content is embedded into source files as kerneldoc and the docbook template just pulls in all the right bits and pieces. Even gtkdoc allos you to do that and pull in the different libararies (== header files with declarations for C) in the order you want. So imo the docbook toolchain is good enough
[Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs
We don't actually need to write the HDMI entry on DDIs that have no chance to be used as HDMI ports. While this patch shouldn't change the current behaviour, it makes further enabling work easier as we'll have an eDP table filling the full 10 entries. Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_ddi.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ca1f9a8..95933b2 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -163,6 +163,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) const u32 *ddi_translations_edp; const u32 *ddi_translations_hdmi; const u32 *ddi_translations; + bool need_hdmi = false; if (IS_BROADWELL(dev)) { ddi_translations_fdi = bdw_ddi_translations_fdi; @@ -195,12 +196,15 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) case PORT_B: case PORT_C: ddi_translations = ddi_translations_dp; + need_hdmi = true; break; case PORT_D: if (intel_dp_is_edp(dev, PORT_D)) ddi_translations = ddi_translations_edp; - else + else { ddi_translations = ddi_translations_dp; + need_hdmi = true; + } break; case PORT_E: ddi_translations = ddi_translations_fdi; @@ -215,6 +219,9 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) reg += 4; } + if (!need_hdmi) + return; + /* Choose a good default if VBT is badly populated */ if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || hdmi_level = n_hdmi_entries) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't require dev-struct_mutex in psr_match_conditions
2014-08-04 5:46 GMT-03:00 Daniel Vetter daniel.vet...@ffwll.ch: Since I've reworked psr support to no longer require x-tiling we don't check any state protected by the Giant GEM Lock. So drop that check. Also boo for lockdep_assert_held for not yelling when lockdep is disabled. Cc: Paulo Zanoni przan...@gmail.com Reported-by: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch I was going to start reviewing it, but then I realized it's already merged. Do we have any doc explaining all our locks/mutexes and what each one is supposed to protect? Anyway, the patch looks fine. --- drivers/gpu/drm/i915/intel_dp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3e6100ea7295..6dbe0f84d455 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1773,7 +1773,6 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); lockdep_assert_held(dev_priv-psr.lock); - lockdep_assert_held(dev-struct_mutex); WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex)); WARN_ON(!drm_modeset_is_locked(crtc-mutex)); -- 2.0.1 -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs
2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com: We don't actually need to write the HDMI entry on DDIs that have no chance to be used as HDMI ports. While this patch shouldn't change the current behaviour, it makes further enabling work easier as we'll have an eDP table filling the full 10 entries. Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com While your patch looks correct, maybe you could have used dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we already do in intel_ddi_init. Or you could set another variable at intel_ddi_init and use it, or do some other equivalent check that doesn't require knowledge of what can really go in each port (since we already have it at intel_ddi_init and we probably shouldn't duplicate it to avoid future desync). Anyway, the patch looks correct for now. So if you still think the current approach is the best, you can add Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_ddi.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ca1f9a8..95933b2 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -163,6 +163,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) const u32 *ddi_translations_edp; const u32 *ddi_translations_hdmi; const u32 *ddi_translations; + bool need_hdmi = false; if (IS_BROADWELL(dev)) { ddi_translations_fdi = bdw_ddi_translations_fdi; @@ -195,12 +196,15 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) case PORT_B: case PORT_C: ddi_translations = ddi_translations_dp; + need_hdmi = true; break; case PORT_D: if (intel_dp_is_edp(dev, PORT_D)) ddi_translations = ddi_translations_edp; - else + else { ddi_translations = ddi_translations_dp; + need_hdmi = true; + } break; case PORT_E: ddi_translations = ddi_translations_fdi; @@ -215,6 +219,9 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) reg += 4; } + if (!need_hdmi) + return; + /* Choose a good default if VBT is badly populated */ if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || hdmi_level = n_hdmi_entries) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Collect gtier properly on HSW.
2014-08-01 13:12 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com: GTIER and DEIER doesn't have same interface on HSW so this or operation makes the information provided useless. v2: since we have gtier variable already let's split for everybody and avoid the strange | op. Also avoid overriding the value that was set for vlv. In this case I believe that we should reorganize the whole function, but I'll respect the comment that ask to not touch the order and let this organization work to be done later. v3: moving VLV check to the right place. Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gpu_error.c | 21 +++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d604f4f..60227b2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -317,6 +317,7 @@ struct drm_i915_error_state { u32 eir; u32 pgtbl_er; u32 ier; + u32 gtier; u32 ccid; u32 derrmr; u32 forcewake; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 0b3f694..c8f901f 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device); err_printf(m, EIR: 0x%08x\n, error-eir); err_printf(m, IER: 0x%08x\n, error-ier); + if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) + err_printf(m, GTIER: 0x%08x\n, error-gtier); err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er); err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake); err_printf(m, DERRMR: 0x%08x\n, error-derrmr); @@ -1102,7 +1104,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, /* 1: Registers specific to a single generation */ if (IS_VALLEYVIEW(dev)) { - error-ier = I915_READ(GTIER) | I915_READ(VLV_IER); + error-gtier = I915_READ(GTIER); + error-ier = I915_READ(VLV_IER); error-forcewake = I915_READ(FORCEWAKE_VLV); } @@ -1135,16 +1138,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, if (HAS_HW_CONTEXTS(dev)) error-ccid = I915_READ(CCID); - if (HAS_PCH_SPLIT(dev)) - error-ier = I915_READ(DEIER) | I915_READ(GTIER); - else { - if (IS_GEN2(dev)) - error-ier = I915_READ16(IER); - else - error-ier = I915_READ(IER); + if (HAS_PCH_SPLIT(dev)) { + error-ier = I915_READ(DEIER); + error-gtier = I915_READ(GTIER); + } else if (IS_GEN2(dev)) { + error-ier = I915_READ16(IER); + } else if (!IS_VALLEYVIEW(dev)) { + error-ier = I915_READ(IER); } - - /* 4: Everything else */ error-eir = I915_READ(EIR); error-pgtbl_er = I915_READ(PGTBL_ER); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Generalize drain latency computation
On Wed, Jul 16, 2014 at 06:24:04PM +0530, Gajanan Bhat wrote: Modify drain latency computation to use it for any plane. Same function can be used for primary, cursor and sprite planes. Signed-off-by: Gajanan Bhat gajanan.b...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_pm.c | 82 ++- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d2a220b..a1260a2 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3877,6 +3877,7 @@ enum punit_power_well { #define DDL_PLANE_PRECISION_64 (17) #define DDL_PLANE_PRECISION_32 (07) #define DDL_PLANE_SHIFT 0 +#define DRAIN_LATENCY_MAX0x7f /* FIFO watermark sizes etc */ #define G4X_FIFO_LINE_SIZE 64 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 90df1e8..f3a3e90 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1268,33 +1268,21 @@ static bool g4x_compute_srwm(struct drm_device *dev, display, cursor); } -static bool vlv_compute_drain_latency(struct drm_device *dev, - int plane, - int *plane_prec_mult, - int *plane_dl, - int *cursor_prec_mult, - int *cursor_dl) +static bool vlv_compute_drain_latency(struct drm_crtc *crtc, + int pixel_size, + int *prec_mult, + int *drain_latency) { - struct drm_crtc *crtc; - int clock, pixel_size; int entries; + int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock; - crtc = intel_get_crtc_for_plane(dev, plane); - if (!intel_crtc_active(crtc)) + if (clock == 0 || pixel_size == 0) return false; - clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock; - pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */ - - entries = (clock / 1000) * pixel_size; - *plane_prec_mult = (entries 128) ? - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; - *plane_dl = (64 * (*plane_prec_mult) * 4) / entries; - - entries = (clock / 1000) * 4; /* BPP is always 4 for cursor */ - *cursor_prec_mult = (entries 128) ? - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; - *cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries; + entries = DIV_ROUND_UP(clock, 1000) * pixel_size; Yeah rounding the clock up should be safe, rounding down not so much. But this change should really be a separate patch. + *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 : +DRAIN_LATENCY_PRECISION_32; + *drain_latency = (64 * (*prec_mult) * 4) / entries; Another thing you could fix is making sure the result is =0x7f. It's quite possible we'd exceed that (all that's needed is a 16bpp fb and 32MHz pixel clock, which isn't all that far fetched). This could be part of the rounding fix patch since it's somewhat related. return true; } @@ -1309,24 +1297,46 @@ static bool vlv_compute_drain_latency(struct drm_device *dev, static void vlv_update_drain_latency(struct drm_crtc *crtc) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_i915_private *dev_priv = crtc-dev-dev_private; + int pixel_size; + int drain_latency; enum pipe pipe = to_intel_crtc(crtc)-pipe; - int plane_prec, plane_dl; - int cursor_prec, cursor_dl; - int plane_prec_mult, cursor_prec_mult; + int plane_prec, prec_mult, plane_dl; - if (vlv_compute_drain_latency(dev, pipe, plane_prec_mult, plane_dl, - cursor_prec_mult, cursor_dl)) { - cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ? - DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32; - plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ? - DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32; + plane_dl = I915_READ(VLV_DDL(pipe)) ~DDL_PLANE_PRECISION_64 +~DRAIN_LATENCY_MAX ~DDL_CURSOR_PRECISION_64 +~(DRAIN_LATENCY_MAX DDL_CURSOR_SHIFT); - I915_WRITE(VLV_DDL(pipe), cursor_prec | -(cursor_dl DDL_CURSOR_SHIFT) | -plane_prec | (plane_dl DDL_PLANE_SHIFT)); + if (!intel_crtc_active(crtc)) { + I915_WRITE(VLV_DDL(pipe), plane_dl); + return; } + + /* Primary plane Drain Latency */ +
Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs
On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote: 2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com: We don't actually need to write the HDMI entry on DDIs that have no chance to be used as HDMI ports. While this patch shouldn't change the current behaviour, it makes further enabling work easier as we'll have an eDP table filling the full 10 entries. Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com While your patch looks correct, maybe you could have used dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we already do in intel_ddi_init. Or you could set another variable at intel_ddi_init and use it, or do some other equivalent check that doesn't require knowledge of what can really go in each port (since we already have it at intel_ddi_init and we probably shouldn't duplicate it to avoid future desync). Anyway, the patch looks correct for now. So if you still think the current approach is the best, you can add Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com I actually like that better, Daniel, hold on your horses :) -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.
2014-08-01 13:13 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com: BDW has many other Display Engine interrupts and GT interrupts registers. Collecting it properly on gpu_error_state. On debugfs all was properly listed already but besides we were also listing old DEIER and GTIER that doesn't exist on BDW anymore. This was causing unclaimed register messages: https://bugs.freedesktop.org/show_bug.cgi?id=81701 v2: Fix small issues of first version and don't read DEIER regs when pipe's power well is disabled v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 4 drivers/gpu/drm/i915/i915_drv.h | 4 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 32 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9e737b7..b3493d3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data) } for_each_pipe(pipe) { + if (!intel_display_power_enabled(dev_priv, + POWER_DOMAIN_PIPE(pipe))) + continue; + seq_printf(m, Pipe %c IMR:\t%08x\n, pipe_name(pipe), I915_READ(GEN8_DE_PIPE_IMR(pipe))); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 60227b2..d1ae952 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -317,7 +317,9 @@ struct drm_i915_error_state { u32 eir; u32 pgtbl_er; u32 ier; - u32 gtier; + u32 gtier[4]; + u32 deier[3]; + u32 de_misc_ier; u32 ccid; u32 derrmr; u32 forcewake; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c8f901f..402b621 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -328,6 +328,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, struct drm_i915_private *dev_priv = dev-dev_private; struct drm_i915_error_state *error = error_priv-error; struct drm_i915_error_object *obj; + enum pipe pipe; int i, j, offset, elt; int max_hangcheck_score; @@ -359,8 +360,20 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device); err_printf(m, EIR: 0x%08x\n, error-eir); err_printf(m, IER: 0x%08x\n, error-ier); + if (IS_BROADWELL(dev)) { + for_each_pipe(pipe) + err_printf(m, DEIER pipe %c: 0x%08x\n, + pipe_name(pipe), + error-deier[pipe]); + for (i = 0; i 4; i++) + err_printf(m, GTIER gt %d: 0x%08x\n, i, + error-gtier[i]); + err_printf(m, DE_MISC_IER: 0x%08x\n, error-de_misc_ier); + } else { + err_printf(m, IER: 0x%08x\n, error-ier); + } if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) - err_printf(m, GTIER: 0x%08x\n, error-gtier); + err_printf(m, GTIER: 0x%08x\n, error-gtier[0]); err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er); err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake); err_printf(m, DERRMR: 0x%08x\n, error-derrmr); @@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, struct drm_i915_error_state *error) { struct drm_device *dev = dev_priv-dev; + enum pipe pipe; + int i; /* General organization * 1. Registers specific to a single generation @@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, /* 1: Registers specific to a single generation */ if (IS_VALLEYVIEW(dev)) { - error-gtier = I915_READ(GTIER); + error-gtier[0] = I915_READ(GTIER); error-ier = I915_READ(VLV_IER); error-forcewake = I915_READ(FORCEWAKE_VLV); } @@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, if (HAS_HW_CONTEXTS(dev)) error-ccid = I915_READ(CCID); - if (HAS_PCH_SPLIT(dev)) { + if (IS_BROADWELL(dev)) { + for_each_pipe(pipe) + if
[Intel-gfx] [PATCH 1/2] drm/i915: Tune done rc6 enabling output
Power users spot this and then get adventurous and try to adjust module driver options. Nothing good ever came out of that, so hide it better. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_pm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 338a80b6773e..4f879494e0c5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) else mode = 0; } - DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, -(mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, -(mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, -(mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); + DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, + (mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); } static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) @@ -3452,8 +3452,8 @@ static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) mask = INTEL_RC6_ENABLE; if ((enable_rc6 mask) != enable_rc6) - DRM_INFO(Adjusting RC6 mask to %d (requested %d, valid %d)\n, -enable_rc6 mask, enable_rc6, mask); + DRM_DEBUG_KMS(Adjusting RC6 mask to %d (requested %d, valid %d)\n, + enable_rc6 mask, enable_rc6, mask); return enable_rc6 mask; } -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Tune down MCH_SSKPD values warning
Users often can't do anything about this since their vendors stopped providing BIOS updates. Also we seem to be able to hack around it with increased latency values, and thus far the only reports have been for screens with really high resolutions. So tune it down to a level where only developers can see it. Also drop some of the end-user fluff. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_pm.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4f879494e0c5..684dc5f60544 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5235,11 +5235,9 @@ static void gen6_check_mch_setup(struct drm_device *dev) uint32_t tmp; tmp = I915_READ(MCH_SSKPD); - if ((tmp MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) { - DRM_INFO(Wrong MCH_SSKPD value: 0x%08x\n, tmp); - DRM_INFO(This can cause pipe underruns and display issues.\n); - DRM_INFO(Please upgrade your BIOS to fix this.\n); - } + if ((tmp MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) + DRM_DEBUG_KMS(Wrong MCH_SSKPD value: 0x%08x This can cause underruns.\n, + tmp); } static void gen6_init_clock_gating(struct drm_device *dev) -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.
On Mon, Aug 04, 2014 at 11:44:10AM -0300, Paulo Zanoni wrote: 2014-08-01 13:13 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com: BDW has many other Display Engine interrupts and GT interrupts registers. Collecting it properly on gpu_error_state. On debugfs all was properly listed already but besides we were also listing old DEIER and GTIER that doesn't exist on BDW anymore. This was causing unclaimed register messages: https://bugs.freedesktop.org/show_bug.cgi?id=81701 v2: Fix small issues of first version and don't read DEIER regs when pipe's power well is disabled v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Both patches merged to dinq, thanks. -Daniel Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 4 drivers/gpu/drm/i915/i915_drv.h | 4 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 32 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9e737b7..b3493d3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data) } for_each_pipe(pipe) { + if (!intel_display_power_enabled(dev_priv, + POWER_DOMAIN_PIPE(pipe))) + continue; + seq_printf(m, Pipe %c IMR:\t%08x\n, pipe_name(pipe), I915_READ(GEN8_DE_PIPE_IMR(pipe))); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 60227b2..d1ae952 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -317,7 +317,9 @@ struct drm_i915_error_state { u32 eir; u32 pgtbl_er; u32 ier; - u32 gtier; + u32 gtier[4]; + u32 deier[3]; + u32 de_misc_ier; u32 ccid; u32 derrmr; u32 forcewake; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c8f901f..402b621 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -328,6 +328,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, struct drm_i915_private *dev_priv = dev-dev_private; struct drm_i915_error_state *error = error_priv-error; struct drm_i915_error_object *obj; + enum pipe pipe; int i, j, offset, elt; int max_hangcheck_score; @@ -359,8 +360,20 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device); err_printf(m, EIR: 0x%08x\n, error-eir); err_printf(m, IER: 0x%08x\n, error-ier); + if (IS_BROADWELL(dev)) { + for_each_pipe(pipe) + err_printf(m, DEIER pipe %c: 0x%08x\n, + pipe_name(pipe), + error-deier[pipe]); + for (i = 0; i 4; i++) + err_printf(m, GTIER gt %d: 0x%08x\n, i, + error-gtier[i]); + err_printf(m, DE_MISC_IER: 0x%08x\n, error-de_misc_ier); + } else { + err_printf(m, IER: 0x%08x\n, error-ier); + } if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) - err_printf(m, GTIER: 0x%08x\n, error-gtier); + err_printf(m, GTIER: 0x%08x\n, error-gtier[0]); err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er); err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake); err_printf(m, DERRMR: 0x%08x\n, error-derrmr); @@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, struct drm_i915_error_state *error) { struct drm_device *dev = dev_priv-dev; + enum pipe pipe; + int i; /* General organization * 1. Registers specific to a single generation @@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, /* 1: Registers specific to a single generation */ if (IS_VALLEYVIEW(dev)) { - error-gtier = I915_READ(GTIER); + error-gtier[0] = I915_READ(GTIER); error-ier = I915_READ(VLV_IER); error-forcewake = I915_READ(FORCEWAKE_VLV); } @@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct
Re: [Intel-gfx] [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse()
On Mon, Aug 04, 2014 at 03:50:34PM +0300, Ville Syrjälä wrote: On Mon, Aug 04, 2014 at 10:10:50AM +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 02:55:22PM +0300, Ville Syrjälä wrote: On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote: On 31 July 2014 17:37, Daniel Vetter dan...@ffwll.ch wrote: On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie airl...@gmail.com wrote: Daniel, the only way intel_dp-is_mst can get reset is inside this path. Ok, so that one should be safe. Then I guess we can just push the locking down into the respective non-mst leafs (since atm we do link-retraining without any locking, which isn't good). And it needs to be dev-mode_config.mutex, not connection mutex. Why that? We can't be doing a modeset w/o connection_mutex so that seems like it should be enough. Well, there's also dpms which leaves the crtc-encoder-connector links intact but that too takes connection_mutex currently. I'd like to know why we do link training at this point though as well, adding locking is required of course, I was just going to wrap the short irq call to the link status check with the lock, but I think it should be possible to push it down further, I don't really know why the sink generates the hpd when we turn off the port, but that doesn't really matter I think. We need to be prepared for hpds at any time. intel_dp_check_link_status() just checks if there's a crtc, which there is (either the old one or the new one, depending on how far along the modeset path we are I guess), and then it just checks drm_dp_channel_eq_ok() which says false since the link was turned off, and then it proceeds to retrain the link. Maybe it should also check crtc-active? Though that itself won't eliminate the problem unless the locking gets fixed somehow. We check encoder-connectors_active, which is equivalent. Only after intel_sanitize_encoder(). Before that we can have !crtc-active encoder-connectors_active. We grab all modeset locks when calling modeset_setup_hw_state, which means no one will ever notice the inconsistent state between state readout and sanitizing it. I think we're safe. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't require dev-struct_mutex in psr_match_conditions
On Mon, Aug 04, 2014 at 11:16:10AM -0300, Paulo Zanoni wrote: 2014-08-04 5:46 GMT-03:00 Daniel Vetter daniel.vet...@ffwll.ch: Since I've reworked psr support to no longer require x-tiling we don't check any state protected by the Giant GEM Lock. So drop that check. Also boo for lockdep_assert_held for not yelling when lockdep is disabled. Cc: Paulo Zanoni przan...@gmail.com Reported-by: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch I was going to start reviewing it, but then I realized it's already merged. Oh, I've figured I'll sneak this one by the danvet must have reviewed-by too rule ;-) But I'll drop such patches asap if anyone spots something with them ofc. Do we have any doc explaining all our locks/mutexes and what each one is supposed to protect? Unfortunately not. It's also constantly changing (e.g. the recent introduction of the connection_mutex) and rather shockingly often not quite correct. Atm you need to dig through git history and for drm core locks through all drm drivers to figure this out :( Anyway, the patch looks fine. I'll count this as an ack and added it, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs
On Mon, Aug 04, 2014 at 03:44:12PM +0100, Damien Lespiau wrote: On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote: 2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com: We don't actually need to write the HDMI entry on DDIs that have no chance to be used as HDMI ports. While this patch shouldn't change the current behaviour, it makes further enabling work easier as we'll have an eDP table filling the full 10 entries. Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com While your patch looks correct, maybe you could have used dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we already do in intel_ddi_init. Or you could set another variable at intel_ddi_init and use it, or do some other equivalent check that doesn't require knowledge of what can really go in each port (since we already have it at intel_ddi_init and we probably shouldn't duplicate it to avoid future desync). Anyway, the patch looks correct for now. So if you still think the current approach is the best, you can add Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com I actually like that better, Daniel, hold on your horses :) So I've spotted some other nice cleanups fly around in the internal m-l, how do those relate to the ones here? Can we please have them for bdw/hsw right away, I liked the added pretty ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Tune down MCH_SSKPD values warning
On Mon, Aug 04, 2014 at 11:18:28AM +0200, Daniel Vetter wrote: Users often can't do anything about this since their vendors stopped providing BIOS updates. Also we seem to be able to hack around it with increased latency values, and thus far the only reports have been for screens with really high resolutions. So tune it down to a level where only developers can see it. Also drop some of the end-user fluff. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Both patches make sense to me so: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4f879494e0c5..684dc5f60544 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5235,11 +5235,9 @@ static void gen6_check_mch_setup(struct drm_device *dev) uint32_t tmp; tmp = I915_READ(MCH_SSKPD); - if ((tmp MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) { - DRM_INFO(Wrong MCH_SSKPD value: 0x%08x\n, tmp); - DRM_INFO(This can cause pipe underruns and display issues.\n); - DRM_INFO(Please upgrade your BIOS to fix this.\n); - } + if ((tmp MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) + DRM_DEBUG_KMS(Wrong MCH_SSKPD value: 0x%08x This can cause underruns.\n, + tmp); } static void gen6_init_clock_gating(struct drm_device *dev) -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Tune done rc6 enabling output
2014-08-04 6:18 GMT-03:00 Daniel Vetter daniel.vet...@ffwll.ch: Power users spot this and then get adventurous and try to adjust module driver options. Nothing good ever came out of that, so hide it better. Agreed. Back when we were toggling the default behavior of RC6 at every new -rc release, this was useful. Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_pm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 338a80b6773e..4f879494e0c5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) else mode = 0; } - DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, -(mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, -(mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, -(mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); + DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, + (mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); } static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) @@ -3452,8 +3452,8 @@ static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) mask = INTEL_RC6_ENABLE; if ((enable_rc6 mask) != enable_rc6) - DRM_INFO(Adjusting RC6 mask to %d (requested %d, valid %d)\n, -enable_rc6 mask, enable_rc6, mask); + DRM_DEBUG_KMS(Adjusting RC6 mask to %d (requested %d, valid %d)\n, + enable_rc6 mask, enable_rc6, mask); return enable_rc6 mask; } -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Tune down MCH_SSKPD values warning
2014-08-04 12:03 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Mon, Aug 04, 2014 at 11:18:28AM +0200, Daniel Vetter wrote: Users often can't do anything about this since their vendors stopped providing BIOS updates. Also we seem to be able to hack around it with increased latency values, and thus far the only reports have been for screens with really high resolutions. So tune it down to a level where only developers can see it. Also drop some of the end-user fluff. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Both patches make sense to me so: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com I never really understood why we expect a specific value for a specific field of the SSKPD on SNB. Isn't this value based on a lot of different machine-dependent variables? On HSW this doesn't make sense at all (and we don't have this check there, so the code is already fine). --- drivers/gpu/drm/i915/intel_pm.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4f879494e0c5..684dc5f60544 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5235,11 +5235,9 @@ static void gen6_check_mch_setup(struct drm_device *dev) uint32_t tmp; tmp = I915_READ(MCH_SSKPD); - if ((tmp MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) { - DRM_INFO(Wrong MCH_SSKPD value: 0x%08x\n, tmp); - DRM_INFO(This can cause pipe underruns and display issues.\n); - DRM_INFO(Please upgrade your BIOS to fix this.\n); - } + if ((tmp MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL) + DRM_DEBUG_KMS(Wrong MCH_SSKPD value: 0x%08x This can cause underruns.\n, + tmp); } static void gen6_init_clock_gating(struct drm_device *dev) -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't write the HDMI buffer translation entries on eDP/FDI DDIs
On Mon, Aug 04, 2014 at 05:04:31PM +0200, Daniel Vetter wrote: On Mon, Aug 04, 2014 at 03:44:12PM +0100, Damien Lespiau wrote: On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote: 2014-08-04 11:15 GMT-03:00 Damien Lespiau damien.lesp...@intel.com: We don't actually need to write the HDMI entry on DDIs that have no chance to be used as HDMI ports. While this patch shouldn't change the current behaviour, it makes further enabling work easier as we'll have an eDP table filling the full 10 entries. Suggested-by: Satheeshakrishna M satheeshakrishn...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com While your patch looks correct, maybe you could have used dev_priv-vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we already do in intel_ddi_init. Or you could set another variable at intel_ddi_init and use it, or do some other equivalent check that doesn't require knowledge of what can really go in each port (since we already have it at intel_ddi_init and we probably shouldn't duplicate it to avoid future desync). Anyway, the patch looks correct for now. So if you still think the current approach is the best, you can add Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com I actually like that better, Daniel, hold on your horses :) So I've spotted some other nice cleanups fly around in the internal m-l, how do those relate to the ones here? Can we please have them for bdw/hsw right away, I liked the added pretty ;-) We already have all the patches that have been written in tree (excluding this one, which is really anecdotal for current platforms, that will just remove a couple of register writes, a patch is coming this way soon-ish anyway). The remaining clean-ups Sonika and/or I are signed up for are: - rename the DP training vswing/pre-emph defines to not include the nominal values but include the level (as for instance eDP 1.4 introduces low voltage swings and those values makes no sense then) - rename the HSW-specific defines to select the DDI buffer translation slot to just include the slot index as the specifics (vswing/pre-emph) depend on the platform and are starting to be more confusing than helpful. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] reverse dp link param selection, prefer fast over wide again
On Mon, Aug 4, 2014 at 5:09 PM, mario_limoncie...@dell.com wrote: (sorry re-sending without Internal use disclaimer – my mail client does that if I don’t explicitly tell it not to) Thanks Daniel. I wasn't sure if this was better discussed in the open or directly with your team. Well for internal discussions I'll force you to go through the proper support channels - the community fastpath only counts if it's public ;-) Do you have a branch somewhere that we can point folks encountering this problem that Dave Airlie has queued up but not yet merged for 3.17? We could try to garner feedback if that would help. drm-intel-nightly from git://anongit.freedesktop.org/drm-intel is the recommended branch. Probably only for internal testing and not annoying customers yet since the dp mst support hasn't fully stabilized yet. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Tune done rc6 enabling output
On Mon, Aug 04, 2014 at 11:18:27AM +0200, Daniel Vetter wrote: Power users spot this and then get adventurous and try to adjust module driver options. Nothing good ever came out of that, so hide it better. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_pm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 338a80b6773e..4f879494e0c5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) else mode = 0; } - DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, - (mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, - (mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, - (mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); + DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, + (mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); } I disagree. This is a good informative message about the capabilities enabled for their GPU. If you want to make the statement that the user shouldn't touch the values, make that adjustment emit a WARNING and implement the kernel tainting you suggested. Also fixing [drm] to be more specific would be useful. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Initialize the aliasing ppgtt as part of global gtt
On Fri, Aug 01, 2014 at 09:54:09AM +, Thierry, Michel wrote: @@ -1766,6 +1771,20 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* And finally clear the reserved guard page */ ggtt_vm-clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true); + if (HAS_ALIASING_PPGTT(dev) USES_FULL_PPGTT(dev)) { Should that be !USES_FULL_PPGTT ? I think we need this for aliasing full ppgtt (the global default ctx)... so it should be USES_PPGTT. We want to setup the aliasing ppgtt _only_ when required, so we don't actually need it for USES_FULL_PPGTT. So Ville's right here. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Tune done rc6 enabling output
On Mon, Aug 04, 2014 at 04:34:28PM +0100, Chris Wilson wrote: On Mon, Aug 04, 2014 at 11:18:27AM +0200, Daniel Vetter wrote: Power users spot this and then get adventurous and try to adjust module driver options. Nothing good ever came out of that, so hide it better. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_pm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 338a80b6773e..4f879494e0c5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) else mode = 0; } - DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, -(mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, -(mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, -(mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); + DRM_DEBUG_KMS(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n, + (mode GEN6_RC_CTL_RC6_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off, + (mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off); } I disagree. This is a good informative message about the capabilities enabled for their GPU. If you want to make the statement that the user shouldn't touch the values, make that adjustment emit a WARNING and implement the kernel tainting you suggested. Also fixing [drm] to be more specific would be useful. Well I'd agree if we'd generally report this, but we don't. This really is the only feature where we tell the user in dmesg what we've done with it, for a lot of other stuff (psr, fbc, rps ...) we just silently set stuff up. rc6 really is the only thing and I don't see why it's special. Imo users should use tools like powertop to assess the effectivenes or our pm code anyway, not look at dmesg. So this is orthogonal to the module param tainting which unfortunately missed 3.17 since Jani (who took over) went on vacatino before completing the revised patches. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Initialize the aliasing ppgtt as part of global gtt
Stuffing this into the context setup code doesn't make a lot of sense. Also reusing the real ppgtt setup code makes even less sense since the aliasing ppgtt isn't a real address space. Leaving all that stuff unitialized will make sure that we catch any abusers promptly. This is also a prep work to clean up the context-ppgtt link. v2: Fix up the logic fail, I've fumbled it so badly to completely disable ppgtt on gen6. Spotted by Ville and Michel. Also move around the pde write into the gen6 init function, since otherwise it won't work at all. Cc: Thierry, Michel michel.thie...@intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_context.c | 13 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 42 +++-- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3b8367aa8404..7a455fcee3a7 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -276,17 +276,6 @@ i915_gem_create_context(struct drm_device *dev, goto err_unpin; } else ctx-vm = ppgtt-base; - - /* This case is reserved for the global default context and -* should only happen once. */ - if (is_global_default_ctx) { - if (WARN_ON(dev_priv-mm.aliasing_ppgtt)) { - ret = -EEXIST; - goto err_unpin; - } - - dev_priv-mm.aliasing_ppgtt = ppgtt; - } } else if (USES_PPGTT(dev)) { /* For platforms which only have aliasing PPGTT, we fake the * address space and refcounting. */ @@ -361,7 +350,7 @@ int i915_gem_context_init(struct drm_device *dev) } } - ctx = i915_gem_create_context(dev, NULL, USES_PPGTT(dev)); + ctx = i915_gem_create_context(dev, NULL, USES_FULL_PPGTT(dev)); if (IS_ERR(ctx)) { DRM_ERROR(Failed to create default global context (error %ld)\n, PTR_ERR(ctx)); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 4fa7807ed4d5..a4c1740d79be 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1188,35 +1188,38 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) ppgtt-node.size 20, ppgtt-node.start / PAGE_SIZE); + gen6_write_pdes(ppgtt); + DRM_DEBUG(Adding PPGTT at offset %x\n, + ppgtt-pd_offset 10); + return 0; } -int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) +int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) { struct drm_i915_private *dev_priv = dev-dev_private; - int ret = 0; ppgtt-base.dev = dev; ppgtt-base.scratch = dev_priv-gtt.base.scratch; if (INTEL_INFO(dev)-gen 8) - ret = gen6_ppgtt_init(ppgtt); + return gen6_ppgtt_init(ppgtt); else if (IS_GEN8(dev)) - ret = gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total); + return gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total); else BUG(); +} +int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + int ret = 0; - if (!ret) { - struct drm_i915_private *dev_priv = dev-dev_private; + ret = __hw_ppgtt_init(dev, ppgtt); + if (ret == 0) { kref_init(ppgtt-ref); drm_mm_init(ppgtt-base.mm, ppgtt-base.start, ppgtt-base.total); i915_init_vm(dev_priv, ppgtt-base); - if (INTEL_INFO(dev)-gen 8) { - gen6_write_pdes(ppgtt); - DRM_DEBUG(Adding PPGTT at offset %x\n, - ppgtt-pd_offset 10); - } } return ret; @@ -1728,6 +1731,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, struct drm_mm_node *entry; struct drm_i915_gem_object *obj; unsigned long hole_start, hole_end; + int ret; BUG_ON(mappable_end end); @@ -1739,7 +1743,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* Mark any preallocated objects as occupied */ list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) { struct i915_vma *vma = i915_gem_obj_to_vma(obj, ggtt_vm); - int ret; + DRM_DEBUG_KMS(reserving preallocated space: %lx + %zx\n, i915_gem_obj_ggtt_offset(obj), obj-base.size); @@ -1766,6
[Intel-gfx] [PATCH] drm/i915: Only track real ppgtt for a context
There's a bit a confusion since we track the global gtt, the aliasing and real ppgtt in the ctx-vm pointer. And not all callers really bother to check for the different cases and just presume that it points to a real ppgtt. Now looking closely we don't actually need -vm to always point at an address space - the only place that cares actually has fixup code already to decide whether to look at the per-proces or the global address space. So switch to just tracking the ppgtt directly and ditch all the extraneous code. v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx-ppgtt. Also drop the early exit - without aliasing ppgtt we want to dump all the ppgtts of the contexts if we have full ppgtt. Cc: Thierry, Michel michel.thie...@intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com OTC-Jira: VIZ-3724 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_debugfs.c| 11 --- drivers/gpu/drm/i915/i915_drv.h| 3 +-- drivers/gpu/drm/i915/i915_gem_context.c| 28 +++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++-- drivers/gpu/drm/i915/i915_gpu_error.c | 10 +++--- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3bf1d20c598b..2bd4beada1bd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data) { struct intel_context *ctx = ptr; struct seq_file *m = data; - struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx); + struct i915_hw_ppgtt *ppgtt = ctx-ppgtt; + + if (!ppgtt) { + seq_puts(m, no ppgtt for context %d\n, +ctx-user_handle); + return 0; + } if (i915_gem_context_is_default(ctx)) seq_puts(m, default context:\n); @@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev) seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd_offset); ppgtt-debug_dump(ppgtt, m); - } else - return; + } list_for_each_entry_reverse(file, dev-filelist, lhead) { struct drm_i915_file_private *file_priv = file-driver_priv; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0762e19f9bc6..3230b08aff13 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -616,7 +616,7 @@ struct intel_context { uint8_t remap_slice; struct drm_i915_file_private *file_priv; struct i915_ctx_hang_stats hang_stats; - struct i915_address_space *vm; + struct i915_hw_ppgtt *ppgtt; struct { struct drm_i915_gem_object *rcs_state; @@ -2504,7 +2504,6 @@ i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj) void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj); /* i915_gem_context.c */ -#define ctx_to_ppgtt(ctx) container_of((ctx)-vm, struct i915_hw_ppgtt, base) int __must_check i915_gem_context_init(struct drm_device *dev); void i915_gem_context_fini(struct drm_device *dev); void i915_gem_context_reset(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 7a455fcee3a7..c00e5d027774 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -136,15 +136,8 @@ void i915_gem_context_free(struct kref *ctx_ref) { struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref); - struct i915_hw_ppgtt *ppgtt = NULL; - if (ctx-legacy_hw_ctx.rcs_state) { - /* We refcount even the aliasing PPGTT to keep the code symmetric */ - if (USES_PPGTT(ctx-legacy_hw_ctx.rcs_state-base.dev)) - ppgtt = ctx_to_ppgtt(ctx); - } - - i915_ppgtt_put(ppgtt); + i915_ppgtt_put(ctx-ppgtt); if (ctx-legacy_hw_ctx.rcs_state) drm_gem_object_unreference(ctx-legacy_hw_ctx.rcs_state-base); list_del(ctx-link); @@ -240,7 +233,6 @@ i915_gem_create_context(struct drm_device *dev, bool create_vm) { const bool is_global_default_ctx = file_priv == NULL; - struct drm_i915_private *dev_priv = dev-dev_private; struct intel_context *ctx; int ret = 0; @@ -274,15 +266,10 @@ i915_gem_create_context(struct drm_device *dev, PTR_ERR(ppgtt)); ret = PTR_ERR(ppgtt); goto err_unpin; - } else - ctx-vm = ppgtt-base; - } else if (USES_PPGTT(dev)) { - /* For platforms which only have aliasing PPGTT, we fake the -* address space and
Re: [Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps
Keith Packard kei...@keithp.com writes: Eric Anholt e...@anholt.net writes: This change appears to be unrelated, and possibly harmful (if X has dropped the last ref to the BO, but it's still the scanout buffer, a new allocation would now reuse the BO and scribble on scanout until the next modeset happens). Yeah, it's unrelated. intel_allocate_framebuffer calls disable_reuse, so there's no need to call it from these two other places. I'll split that change out into a separate patch with separate comment. Unrelated whitespace. There are a bunch of whitespace fixups; should I pull those into a separate patch or just leave them scattered in the first patch to change a file? One patch at the front is fine with me. pgpytHLVSAeTJ.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation
Keith Packard kei...@keithp.com writes: Eric Anholt e...@anholt.net writes: Keith Packard kei...@keithp.com writes: I don't see anything indicating that this code path is only used by glamor. True. It's a fix for DRI3 for either UXA or none. Mesa allocates a single page for a 1x1 texture, but this code thinks that should take two pages causing a texture-to-pixmap operation to fail. OK, but isn't the problem with the code you're #if 0ing (which, really? #if 0 in a patch being submitted for review?) that it's aligning to 2*height instead of height? pgp418rnoUsvi.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Get CZ clock for VLV
CZ clock is related to data flow from memory to display plane. This is required for comparison with CD clock before programming PFI credits. Signed-off-by: Vandana Kannan vandana.kan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 20 3 files changed, 23 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 174a294..baeb56f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val); int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); +int valleyview_get_cz_clock_speed(struct drm_device *dev); + #define FORCEWAKE_RENDER (1 0) #define FORCEWAKE_MEDIA(1 1) #define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index fe5c276..1b8f095 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -601,6 +601,7 @@ enum punit_power_well { #define DISPLAY_FREQUENCY_STATUS (0x1f 8) #define DISPLAY_FREQUENCY_STATUS_SHIFT8 #define DISPLAY_FREQUENCY_VALUES (0x1f 0) +#define CCK_CZ_CONTROL 0x62 /** * DOC: DPIO diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 99eb7ca..56a8090 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev) return DIV_ROUND_CLOSEST(vco 1, divider + 1); } +int valleyview_get_cz_clock_speed(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + int vco = valleyview_get_vco(dev_priv); + u32 val; + int divider; + + mutex_lock(dev_priv-dpio_lock); + val = vlv_cck_read(dev_priv, CCK_CZ_CONTROL); + mutex_unlock(dev_priv-dpio_lock); + + divider = val DISPLAY_FREQUENCY_VALUES; + + WARN((val DISPLAY_FREQUENCY_STATUS) != +(divider DISPLAY_FREQUENCY_STATUS_SHIFT), +czclk change in progress\n); + + return DIV_ROUND_CLOSEST(vco 1, divider + 1); +} + static int i945_get_display_clock_speed(struct drm_device *dev) { return 40; -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Program PFI credits for VLV
From: Vidya Srinivas vidya.srini...@intel.com PFI credit programming is required when CD clock (related to data flow from display pipeline to end display) is greater than CZ clock (related to data flow from memory to display plane). This programming should be done when all planes are OFF to avoid intermittent hangs while accessing memory even from different Gfx units (not just display). If cdclk/czclk =1, PFI credits could be set as any number. To get better performance, larger PFI credit can be assigned to PND. Otherwise if cdclk/czclk1, the default PFI credit of 8 should be set. v2: - Change log to lower log level instead of DRM_ERROR - Change function name to valleyview_program_pfi_credits - Move program PFI credits to modeset_init instead of intel_set_mode - Change magic numbers to logical constants Signed-off-by: Vidya Srinivas vidya.srini...@intel.com Signed-off-by: Gajanan Bhat gajanan.b...@intel.com Signed-off-by: Vandana Kannan vandana.kan...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 6 ++ drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_reg.h | 5 + drivers/gpu/drm/i915/intel_display.c | 4 +++- drivers/gpu/drm/i915/intel_pm.c | 22 ++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b2b649c..73617b5 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev) intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED); console_unlock(); + if (IS_VALLEYVIEW(dev)) + valleyview_program_pfi_credits(dev_priv, false); + dev_priv-suspend_count++; intel_display_set_init_power(dev_priv, false); @@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) dev_priv-modeset_restore = MODESET_DONE; mutex_unlock(dev_priv-modeset_restore_lock); + if (IS_VALLEYVIEW(dev)) + valleyview_program_pfi_credits(dev_priv, true); + intel_opregion_notify_adapter(dev, PCI_D0); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index baeb56f..38907ef 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2152,6 +2152,8 @@ extern struct i915_params i915 __read_mostly; /* i915_dma.c */ void i915_update_dri1_breadcrumb(struct drm_device *dev); +extern void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv, + bool flag); extern void i915_kernel_lost_context(struct drm_device * dev); extern int i915_driver_load(struct drm_device *, unsigned long flags); extern int i915_driver_unload(struct drm_device *); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1b8f095..92b8afc 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1914,6 +1914,11 @@ enum punit_power_well { #define CZCLK_FREQ_MASK 0xf #define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510) +#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C) +#define PFI_CREDIT (7 28) +#define PFI_CREDIT_RESEND(1 27) +#define VGA_FAST_MODE_DISABLE (1 14) + /* * Palette regs */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 56a8090..521943a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12555,8 +12555,10 @@ void intel_modeset_init_hw(struct drm_device *dev) { intel_prepare_ddi(dev); - if (IS_VALLEYVIEW(dev)) + if (IS_VALLEYVIEW(dev)) { vlv_update_cdclk(dev); + valleyview_program_pfi_credits(dev-dev_private, true); + } intel_init_clock_gating(dev); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3f88f29..fe55c54 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6780,6 +6780,28 @@ void intel_fini_runtime_pm(struct drm_i915_private *dev_priv) pm_runtime_disable(device); } +void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv, + bool flag) +{ + int cd_clk, cz_clk; + + if (!flag) { + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE); + return; + } + + cd_clk = dev_priv-display.get_display_clock_speed(dev_priv-dev); + cz_clk = valleyview_get_cz_clock_speed(dev_priv-dev); + + if (cd_clk = cz_clk) { + /* WA - write default credits before re-programming */ + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE); + I915_WRITE(GCI_CONTROL, (PFI_CREDIT | PFI_CREDIT_RESEND | +
Re: [Intel-gfx] [PATCH] drm/i915: Only track real ppgtt for a context
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Monday, August 04, 2014 3:21 PM To: Intel Graphics Development Cc: Daniel Vetter; Thierry, Michel; Ville Syrjälä Subject: [PATCH] drm/i915: Only track real ppgtt for a context There's a bit a confusion since we track the global gtt, the aliasing and real ppgtt in the ctx-vm pointer. And not all callers really bother to check for the different cases and just presume that it points to a real ppgtt. Now looking closely we don't actually need -vm to always point at an address space - the only place that cares actually has fixup code already to decide whether to look at the per-proces or the global address space. So switch to just tracking the ppgtt directly and ditch all the extraneous code. v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx-ppgtt. Also drop the early exit - without aliasing ppgtt we want to dump all the ppgtts of the contexts if we have full ppgtt. Cc: Thierry, Michel michel.thie...@intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com OTC-Jira: VIZ-3724 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_debugfs.c| 11 --- drivers/gpu/drm/i915/i915_drv.h| 3 +-- drivers/gpu/drm/i915/i915_gem_context.c| 28 +++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++-- drivers/gpu/drm/i915/i915_gpu_error.c | 10 +++--- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3bf1d20c598b..2bd4beada1bd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data) { struct intel_context *ctx = ptr; struct seq_file *m = data; - struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx); + struct i915_hw_ppgtt *ppgtt = ctx-ppgtt; + + if (!ppgtt) { + seq_puts(m, no ppgtt for context %d\n, + ctx-user_handle); Should be seq_printf(). + return 0; + } if (i915_gem_context_is_default(ctx)) seq_puts(m, default context:\n); @@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m, -- 1.9.3 Apart from that, Reviewed-by: Michel Thierry michel.thie...@intel.com smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
According to spec FBC on BDW and HSW are identical without any gaps. So let's copy the nuke and let FBC really start compressing stuff. Without this patch we can verify with false color that nothing is being compressed. With the nuke in place and false color it is possible to see false color debugs. Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer rend cache would never been cleaned and FBC would stop compressing buffer. One alternative is to cache clean on software frontbuffer tracking. v2: Fix rebase conflict. v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c| 3 +++ drivers/gpu/drm/i915/intel_pm.c | 10 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +- 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2a372f2..25d7365 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct drm_device *dev, extern void i915_redisable_vga(struct drm_device *dev); extern void i915_redisable_vga_power_on(struct drm_device *dev); extern bool intel_fbc_enabled(struct drm_device *dev); +extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value); extern void intel_disable_fbc(struct drm_device *dev); extern bool ironlake_set_drps(struct drm_device *dev, u8 val); extern void intel_init_pch_refclk(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 883af0b..c8421cd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device *dev, intel_mark_fb_busy(dev, frontbuffer_bits, NULL); intel_edp_psr_flush(dev, frontbuffer_bits); + + if (IS_GEN8(dev)) + gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN); } /** diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 684dc5f..de07d3e 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev) return dev_priv-display.fbc_enabled(dev); } +void gen8_fbc_sw_flush(struct drm_device *dev, u32 value) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + if (!IS_GEN8(dev)) + return; + + I915_WRITE(MSG_FBC_REND_STATE, value); +} + static void intel_fbc_work_fn(struct work_struct *__work) { struct intel_fbc_work *work = diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2908896..2fe871c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, { u32 flags = 0; u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES; + int ret; flags |= PIPE_CONTROL_CS_STALL; @@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; } - return gen8_emit_pipe_control(ring, flags, scratch_addr); + ret = gen8_emit_pipe_control(ring, flags, scratch_addr); + if (ret) + return ret; + + if (!invalidate_domains flush_domains) + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); + + return 0; } static void ring_write_tail(struct intel_engine_cs *ring, -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Only track real ppgtt for a context
There's a bit a confusion since we track the global gtt, the aliasing and real ppgtt in the ctx-vm pointer. And not all callers really bother to check for the different cases and just presume that it points to a real ppgtt. Now looking closely we don't actually need -vm to always point at an address space - the only place that cares actually has fixup code already to decide whether to look at the per-proces or the global address space. So switch to just tracking the ppgtt directly and ditch all the extraneous code. v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx-ppgtt. Also drop the early exit - without aliasing ppgtt we want to dump all the ppgtts of the contexts if we have full ppgtt. v3: Actually git add the compile fix. Reviewed-by: Michel Thierry michel.thie...@intel.com Cc: Thierry, Michel michel.thie...@intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com OTC-Jira: VIZ-3724 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_debugfs.c| 11 --- drivers/gpu/drm/i915/i915_drv.h| 3 +-- drivers/gpu/drm/i915/i915_gem_context.c| 28 +++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++-- drivers/gpu/drm/i915/i915_gpu_error.c | 10 +++--- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3bf1d20c598b..7e8d94b5 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data) { struct intel_context *ctx = ptr; struct seq_file *m = data; - struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx); + struct i915_hw_ppgtt *ppgtt = ctx-ppgtt; + + if (!ppgtt) { + seq_printf(m, no ppgtt for context %d\n, + ctx-user_handle); + return 0; + } if (i915_gem_context_is_default(ctx)) seq_puts(m, default context:\n); @@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev) seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd_offset); ppgtt-debug_dump(ppgtt, m); - } else - return; + } list_for_each_entry_reverse(file, dev-filelist, lhead) { struct drm_i915_file_private *file_priv = file-driver_priv; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0762e19f9bc6..3230b08aff13 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -616,7 +616,7 @@ struct intel_context { uint8_t remap_slice; struct drm_i915_file_private *file_priv; struct i915_ctx_hang_stats hang_stats; - struct i915_address_space *vm; + struct i915_hw_ppgtt *ppgtt; struct { struct drm_i915_gem_object *rcs_state; @@ -2504,7 +2504,6 @@ i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj) void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj); /* i915_gem_context.c */ -#define ctx_to_ppgtt(ctx) container_of((ctx)-vm, struct i915_hw_ppgtt, base) int __must_check i915_gem_context_init(struct drm_device *dev); void i915_gem_context_fini(struct drm_device *dev); void i915_gem_context_reset(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 7a455fcee3a7..c00e5d027774 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -136,15 +136,8 @@ void i915_gem_context_free(struct kref *ctx_ref) { struct intel_context *ctx = container_of(ctx_ref, typeof(*ctx), ref); - struct i915_hw_ppgtt *ppgtt = NULL; - if (ctx-legacy_hw_ctx.rcs_state) { - /* We refcount even the aliasing PPGTT to keep the code symmetric */ - if (USES_PPGTT(ctx-legacy_hw_ctx.rcs_state-base.dev)) - ppgtt = ctx_to_ppgtt(ctx); - } - - i915_ppgtt_put(ppgtt); + i915_ppgtt_put(ctx-ppgtt); if (ctx-legacy_hw_ctx.rcs_state) drm_gem_object_unreference(ctx-legacy_hw_ctx.rcs_state-base); list_del(ctx-link); @@ -240,7 +233,6 @@ i915_gem_create_context(struct drm_device *dev, bool create_vm) { const bool is_global_default_ctx = file_priv == NULL; - struct drm_i915_private *dev_priv = dev-dev_private; struct intel_context *ctx; int ret = 0; @@ -274,15 +266,10 @@ i915_gem_create_context(struct drm_device *dev, PTR_ERR(ppgtt)); ret = PTR_ERR(ppgtt); goto err_unpin; - } else - ctx-vm = ppgtt-base; - } else if (USES_PPGTT(dev)) { - /*
[Intel-gfx] [PATCH] drm/i915: Android sync points for i915 v2
Expose an ioctl to create Android fences based on the Android sync point infrastructure (which in turn is based on DMA-buf fences). Just a sketch at this point, no testing has been done. There are a couple of goals here: 1) allow applications and libraries to create fences without an associated buffer 2) re-use a common API so userspace doesn't have to impedance mismatch between different driver implementations too much 3) allow applications and libraries to use explicit synchronization if they choose by exposing fences directly v2: use struct fence directly using Maarten's new interface Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/Kconfig | 2 + drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 10 ++ drivers/gpu/drm/i915/i915_gem.c | 15 +- drivers/gpu/drm/i915/i915_irq.c | 4 +- drivers/gpu/drm/i915/i915_sync.c | 323 +++ include/uapi/drm/i915_drm.h | 23 +++ 8 files changed, 373 insertions(+), 6 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_sync.c diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 4e39ab3..cd0f2ec 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -6,6 +6,8 @@ config DRM_I915 select INTEL_GTT select AGP_INTEL if AGP select INTERVAL_TREE + select ANDROID + select SYNC # we need shmfs for the swappable backing store, and in particular # the shmem_readpage() which depends upon tmpfs select SHMEM diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..61a3eb5c 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -25,6 +25,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_execbuffer.o \ i915_gem_gtt.o \ i915_gem.o \ + i915_sync.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ i915_gem_userptr.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..84086e1 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -2043,6 +2043,7 @@ const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_FENCE, i915_sync_create_fence_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; int i915_max_ioctl = ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d604f4f..6eb119e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1388,6 +1388,8 @@ struct i915_frontbuffer_tracking { unsigned flip_bits; }; +struct i915_sync_timeline; + struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; @@ -1422,6 +1424,8 @@ struct drm_i915_private { struct drm_i915_gem_object *semaphore_obj; uint32_t last_seqno, next_seqno; + struct i915_sync_timeline *sync_tl[I915_NUM_RINGS]; + drm_dma_handle_t *status_page_dmah; struct resource mch_res; @@ -2275,6 +2279,12 @@ void i915_init_vm(struct drm_i915_private *dev_priv, void i915_gem_free_object(struct drm_gem_object *obj); void i915_gem_vma_destroy(struct i915_vma *vma); +/* i915_sync.c */ +int i915_sync_init(struct drm_i915_private *dev_priv); +void i915_sync_fini(struct drm_i915_private *dev_priv); +int i915_sync_create_fence_ioctl(struct drm_device *dev, void *data, +struct drm_file *file); + #define PIN_MAPPABLE 0x1 #define PIN_NONBLOCK 0x2 #define PIN_GLOBAL 0x4 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dcd8d7b..ace716e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1146,11 +1146,11 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv) * Returns 0 if the seqno was found within the alloted time. Else returns the * errno with remaining time filled in timeout argument. */ -static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno, - unsigned reset_counter, - bool interruptible, - struct timespec *timeout, - struct drm_i915_file_private *file_priv) +int __wait_seqno(struct intel_engine_cs *ring, u32 seqno, +unsigned reset_counter, +bool interruptible, +struct timespec *timeout, +struct drm_i915_file_private *file_priv) { struct drm_device *dev = ring-dev; struct
[Intel-gfx] [PATCH] drm/i915: lock around link status and link training.
From: Dave Airlie airl...@redhat.com We need to take the connection mutex around the link status check for non-MST case, but also around the MST link training on short HPDs. I suspect we actually should have a dpcd lock in the future as well, that just lock the local copies of dpcd and flags stored from that. Signed-off-by: Dave Airlie airl...@redhat.com --- drivers/gpu/drm/i915/intel_dp.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0f05b88..145ac7c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3437,6 +3437,7 @@ intel_dp_handle_test_request(struct intel_dp *intel_dp) static int intel_dp_check_mst_status(struct intel_dp *intel_dp) { + struct drm_device *dev = intel_dp_to_dev(intel_dp); bool bret; if (intel_dp-is_mst) { @@ -3449,12 +3450,14 @@ go_again: if (bret == true) { /* check link status - esi[10] = 0x200c */ + drm_modeset_lock(dev-mode_config.connection_mutex, NULL); if (intel_dp-active_mst_links !drm_dp_channel_eq_ok(esi[10], intel_dp-lane_count)) { DRM_DEBUG_KMS(channel EQ not ok, retraining\n); intel_dp_start_link_train(intel_dp); intel_dp_complete_link_train(intel_dp); intel_dp_stop_link_train(intel_dp); } + drm_modeset_unlock(dev-mode_config.connection_mutex); DRM_DEBUG_KMS(got esi %02x %02x %02x\n, esi[0], esi[1], esi[2]); ret = drm_dp_mst_hpd_irq(intel_dp-mst_mgr, esi, handled); @@ -3502,10 +3505,13 @@ go_again: void intel_dp_check_link_status(struct intel_dp *intel_dp) { + struct drm_device *dev = intel_dp_to_dev(intel_dp); struct intel_encoder *intel_encoder = dp_to_dig_port(intel_dp)-base; u8 sink_irq_vector; u8 link_status[DP_LINK_STATUS_SIZE]; + WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex)); + /* FIXME: This access isn't protected by any locks. */ if (!intel_encoder-connectors_active) return; @@ -4021,7 +4027,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) * we'll check the link status via the normal hot plug path later - * but for short hpds we should check it now */ + drm_modeset_lock(dev-mode_config.connection_mutex, NULL); intel_dp_check_link_status(intel_dp); + drm_modeset_unlock(dev-mode_config.connection_mutex); } } return false; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915: Add thread stall DOP clock gating workaround on Broadwell.
From: Kenneth Graunke kenn...@whitecape.org Ben and I believe this will be necessary on production hardware. Signed-off-by: Kenneth Graunke kenn...@whitecape.org Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 684dc5f..f919596 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5412,6 +5412,10 @@ static void gen8_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN8_ROW_CHICKEN, _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); + /* WaDisableThreadStallDopClockGating:bdw */ + I915_WRITE(GEN8_ROW_CHICKEN, + _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); + /* * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for * pre-production hardware -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/7] drm/i915/bdw: Always issue a force restore
From: Ben Widawsky benjamin.widaw...@intel.com The PDPs seem to get screwed up otherwise, specifically PDP0. I am not really clear why this is required, it just works with full PPGTT. v2: Only do it for gen8, to limit regression potential v3: Fix the bugzilla links Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78891 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78935 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78936 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78937 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78938 Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3b99390..56f7b1e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -586,6 +586,9 @@ mi_set_context(struct intel_engine_cs *ring, else intel_ring_emit(ring, MI_NOOP); + if (INTEL_INFO(ring-dev)-gen == 8) + hw_flags |= MI_FORCE_RESTORE; + intel_ring_emit(ring, MI_NOOP); intel_ring_emit(ring, MI_SET_CONTEXT); intel_ring_emit(ring, i915_gem_obj_ggtt_offset(new_context-legacy_hw_ctx.rcs_state) | -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/7] drm/i915: BDW Semaphore signal with Post Sync
With this bit set MI_SEMAPHORE_SIGNAL command is executed as a pipelined PIPE_CONTROL flush command with Semaphore Signal as post sync operation. However this can only be set only when Fixed Function DOP Clock Gate Disable is set. This brought a bit of stability on Semaphores minimizing the hangs. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 3 +++ drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index dc13961..27a54a0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -269,6 +269,7 @@ #define MI_SEMAPHORE_SIGNALMI_INSTR(0x1b, 0) /* GEN8+ */ #define MI_SEMAPHORE_TARGET(engine) ((engine)15) #define MI_SEMAPHORE_WAIT MI_INSTR(0x1c, 2) /* GEN8+ */ +#define MI_SEMAPHORE_POST_SYNC (121) #define MI_SEMAPHORE_POLL(115) #define MI_SEMAPHORE_SAD_GTE_SDD (112) #define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f919596..ab8cbda 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5481,6 +5481,9 @@ static void gen8_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE)); + I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, + _MASKED_BIT_ENABLE(GEN8_FF_DOP_CLOCK_GATE_DISABLE)); + /* WaDisableSDEUnitClockGating:bdw */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index cd30b39..edb8234 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -730,6 +730,7 @@ static int gen8_rcs_signal(struct intel_engine_cs *signaller, intel_ring_emit(signaller, signaller-outstanding_lazy_seqno); intel_ring_emit(signaller, 0); intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL | + MI_SEMAPHORE_POST_SYNC | MI_SEMAPHORE_TARGET(waiter-id)); intel_ring_emit(signaller, 0); } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/7] drm/i915/bdw: MI_FLUSH_DW a qword instead of dword
From: Ben Widawsky benjamin.widaw...@intel.com The actual post sync op is Write Immediate Data QWord. It is therefore arguable that we should have always done a qword write. The actual impetus for this patch is our decoder complains when we write a dword and I was trying to eliminate the spurious errors. With this patch, I've noticed a really strange reproducible error turns into a different strange reproducible error - so it does indeed have some effect of some sort. This was also recommended to me by someone that is familiar with the Windows driver. It's based on top of the semaphore series, so it won't be easily applied, I'd guess. Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 95 + 1 file changed, 74 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2908896..9a562b5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -727,7 +727,7 @@ static int gen8_rcs_signal(struct intel_engine_cs *signaller, static int gen8_xcs_signal(struct intel_engine_cs *signaller, unsigned int num_dwords) { -#define MBOX_UPDATE_DWORDS 6 +#define MBOX_UPDATE_DWORDS 8 struct drm_device *dev = signaller-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_engine_cs *waiter; @@ -746,15 +746,18 @@ static int gen8_xcs_signal(struct intel_engine_cs *signaller, if (gtt_offset == MI_SEMAPHORE_SYNC_INVALID) continue; - intel_ring_emit(signaller, (MI_FLUSH_DW + 1) | + intel_ring_emit(signaller, (MI_FLUSH_DW + 2) | MI_FLUSH_DW_OP_STOREDW); intel_ring_emit(signaller, lower_32_bits(gtt_offset) | MI_FLUSH_DW_USE_GTT); intel_ring_emit(signaller, upper_32_bits(gtt_offset)); intel_ring_emit(signaller, signaller-outstanding_lazy_seqno); + intel_ring_emit(signaller, 0); /* upper dword */ + intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL | MI_SEMAPHORE_TARGET(waiter-id)); intel_ring_emit(signaller, 0); + intel_ring_emit(signaller, MI_NOOP); } return 0; @@ -1939,8 +1942,6 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs *ring, return ret; cmd = MI_FLUSH_DW; - if (INTEL_INFO(ring-dev)-gen = 8) - cmd += 1; /* * Bspec vol 1c.5 - video engine command streamer: * If ENABLED, all TLBs will be invalidated once the flush @@ -1952,13 +1953,38 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs *ring, MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; intel_ring_emit(ring, cmd); intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT); - if (INTEL_INFO(ring-dev)-gen = 8) { - intel_ring_emit(ring, 0); /* upper addr */ - intel_ring_emit(ring, 0); /* value */ - } else { - intel_ring_emit(ring, 0); - intel_ring_emit(ring, MI_NOOP); - } + intel_ring_emit(ring, 0); + intel_ring_emit(ring, MI_NOOP); + intel_ring_advance(ring); + return 0; +} + +static int gen8_bsd_ring_flush(struct intel_engine_cs *ring, + u32 invalidate, u32 flush) +{ + uint32_t cmd; + int ret; + + ret = intel_ring_begin(ring, 6); + if (ret) + return ret; + + cmd = MI_FLUSH_DW + 2; + /* +* Bspec vol 1c.5 - video engine command streamer: +* If ENABLED, all TLBs will be invalidated once the flush +* operation is complete. This bit is only valid when the +* Post-Sync Operation field is a value of 1h or 3h. +*/ + if (invalidate I915_GEM_GPU_DOMAINS) + cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD | + MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; + intel_ring_emit(ring, cmd); + intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT); + intel_ring_emit(ring, 0); /* upper addr */ + intel_ring_emit(ring, 0); /* value */ + intel_ring_emit(ring, 0); /* value */ + intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); return 0; } @@ -2029,8 +2055,38 @@ gen6_ring_dispatch_execbuffer(struct intel_engine_cs *ring, return 0; } -/* Blitter support (SandyBridge+) */ +static int gen8_ring_flush(struct intel_engine_cs *ring, + u32 invalidate, u32 flush) +{ + uint32_t cmd; + int ret; + + ret = intel_ring_begin(ring, 6); + if (ret) +
[Intel-gfx] [PATCH 4/7] drm/i915/bdw: cs-stall before state cache invld w/a
From: Ben Widawsky benjamin.widaw...@intel.com We do this already for previous GENs. I guess we must do it for BDW too according to DOCS. Pipe_control with CS-stall bit set must be issued before a pipe-control command that has the State Cache Invalidate bit set. This does not solve the problem I have unfortunately. I didn't check if this was in Ville's CHV series. If it was, I apologize. NOTE: I tried to use smaller lengths for the command, but nothing made it happy except 6. Cc: Kenneth Graunke kenn...@whitecape.org Cc: Jordan Justen jljus...@gmail.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 19 --- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 9a562b5..2e566e0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -279,17 +279,25 @@ gen6_render_ring_flush(struct intel_engine_cs *ring, static int gen7_render_ring_cs_stall_wa(struct intel_engine_cs *ring) { - int ret; + int ret, size = 4; - ret = intel_ring_begin(ring, 4); + if (IS_BROADWELL(ring-dev)) + size += 2; + + ret = intel_ring_begin(ring, size); if (ret) return ret; - intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4)); + intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(size)); intel_ring_emit(ring, PIPE_CONTROL_CS_STALL | PIPE_CONTROL_STALL_AT_SCOREBOARD); intel_ring_emit(ring, 0); intel_ring_emit(ring, 0); + if (IS_BROADWELL(ring-dev)) { + intel_ring_emit(ring, 0); + intel_ring_emit(ring, 0); + } + intel_ring_advance(ring); return 0; @@ -422,6 +430,11 @@ gen8_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE; flags |= PIPE_CONTROL_QW_WRITE; flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; + + /* Workaround: we must issue a pipe_control with CS-stall bit +* set before a pipe_control command that has the state cache +* invalidate bit set. */ + gen7_render_ring_cs_stall_wa(ring); } return gen8_emit_pipe_control(ring, flags, scratch_addr); -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/i915: avoid emiting semaphore wait on GEN8 when seqno wrap happened.
C: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2e566e0..cd30b39 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -867,15 +867,26 @@ gen8_ring_sync(struct intel_engine_cs *waiter, if (ret) return ret; - intel_ring_emit(waiter, MI_SEMAPHORE_WAIT | - MI_SEMAPHORE_GLOBAL_GTT | - MI_SEMAPHORE_POLL | - MI_SEMAPHORE_SAD_GTE_SDD); - intel_ring_emit(waiter, seqno); - intel_ring_emit(waiter, - lower_32_bits(GEN8_WAIT_OFFSET(waiter, signaller-id))); - intel_ring_emit(waiter, - upper_32_bits(GEN8_WAIT_OFFSET(waiter, signaller-id))); + /* If seqno wrap happened, omit the wait with no-ops */ + if (likely(!i915_gem_has_seqno_wrapped(waiter-dev, seqno))) { + intel_ring_emit(waiter, MI_SEMAPHORE_WAIT | + MI_SEMAPHORE_GLOBAL_GTT | + MI_SEMAPHORE_POLL | + MI_SEMAPHORE_SAD_GTE_SDD); + intel_ring_emit(waiter, seqno); + intel_ring_emit(waiter, + lower_32_bits(GEN8_WAIT_OFFSET(waiter, + signaller-id))); + intel_ring_emit(waiter, + upper_32_bits(GEN8_WAIT_OFFSET(waiter, + signaller-id))); + } else { + intel_ring_emit(waiter, MI_NOOP); + intel_ring_emit(waiter, MI_NOOP); + intel_ring_emit(waiter, MI_NOOP); + intel_ring_emit(waiter, MI_NOOP); + } + intel_ring_advance(waiter); return 0; } @@ -2572,6 +2583,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) ring-semaphore.mbox.signal[BCS] = GEN6_BVESYNC; ring-semaphore.mbox.signal[VECS] = GEN6_NOSYNC; ring-semaphore.mbox.signal[VCS2] = GEN6_NOSYNC; + } } ring-init = init_ring_common; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/7] Revert drm/i915: Enable semaphores on BDW
This reverts commit 521e62e49a42661a4ee0102644517dbe2f100a23. Although POST_SYNC brought a bit of stability to Semaphores on BDW it didn't solved all issues and some hungs can still occour when semaphores are enabled on BDW. Also some sloweness can be found on some igt tests, althoguth it apparently doesn't affect real workloads. Besides that, no real performance gain was found on our tests with different and even multiple workloads. Let's disable it again for now. At least until we are sure it is safe to re-enable it. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c4b25c..ec96f9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -481,6 +481,10 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) if (i915.semaphores = 0) return i915.semaphores; + /* Until we get further testing... */ + if (IS_GEN8(dev)) + return false; + #ifdef CONFIG_INTEL_IOMMU /* Enable semaphores on SNB when IO remapping is off */ if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915/bdw: Always issue a force restore
On Mon, Aug 04, 2014 at 11:15:13AM -0700, Rodrigo Vivi wrote: From: Ben Widawsky benjamin.widaw...@intel.com The PDPs seem to get screwed up otherwise, specifically PDP0. I am not really clear why this is required, it just works with full PPGTT. v2: Only do it for gen8, to limit regression potential v3: Fix the bugzilla links Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78891 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78935 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78936 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78937 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78938 Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Hi Rodrigo. This patch was superseded by my [partially merged] series here: http://lists.freedesktop.org/archives/intel-gfx/2014-July/048311.html In that series I went through very thoroughly with design why we need to do this, and verified those patches do address this issue. So yeah, NAK from me - and please get those reworked or merged or whatever. Thanks [snip] -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915/bdw: MI_FLUSH_DW a qword instead of dword
On Mon, Aug 04, 2014 at 11:15:15AM -0700, Rodrigo Vivi wrote: From: Ben Widawsky benjamin.widaw...@intel.com The actual post sync op is Write Immediate Data QWord. It is therefore arguable that we should have always done a qword write. The actual impetus for this patch is our decoder complains when we write a dword and I was trying to eliminate the spurious errors. With this patch, I've noticed a really strange reproducible error turns into a different strange reproducible error - so it does indeed have some effect of some sort. This was also recommended to me by someone that is familiar with the Windows driver. It's based on top of the semaphore series, so it won't be easily applied, I'd guess. Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Did we determine this was needed for anything other than shutting up the instruction decoder, for debug? Seems like if the existing stuff ain't broke, don't fix it. [snip] -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Docbook fixes
On 30 July 2014 23:36, Daniel Vetter daniel.vet...@ffwll.ch wrote: Bunch of small leftovers spotted by looking at the make htmldocs output. I've left out dp mst, there's too much amiss there. (btw this patch doesn't apply cleanly - modeset_lock seems different). Just spotted this comment! I do wonder if I should really be documenting the internals of that struct in docbook, The main reason its there is to embed in the driver, I don't think drivers should be looking inside it too often, and we certainly shouldn't be generating info about most of the members in the interface docs for driver writers. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] Stop trying to out-guess mesa for BO allocation
Eric Anholt e...@anholt.net writes: OK, but isn't the problem with the code you're #if 0ing (which, really? #if 0 in a patch being submitted for review?) that it's aligning to 2*height instead of height? I went and did a bit of archaeology to figure out why it's using 2*height instead of just height. The initial change to pixmap allocation, which was then copied to the BO validation logic, was here: commit 736b89504a32239a0c7dfb5961c1b8292dd744bd Author: Chris Wilson ch...@chris-wilson.co.uk Date: Sun Dec 30 10:32:18 2012 + uxa: Align surface allocations to even tile rows Align surface sizes to an even number of tile rows to cater for sampler prefetch. If we read beyond the last page we may catch the PTE in a state of flux and trigger a GPU hang. Also detected by enabling invalid PTE access checking. References: https://bugs.freedesktop.org/show_bug.cgi?id=56916 References: https://bugs.freedesktop.org/show_bug.cgi?id=55984 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk diff --git a/src/intel_uxa.c b/src/intel_uxa.c index f5ac0a6..2f14173 100644 --- a/src/intel_uxa.c +++ b/src/intel_uxa.c @@ -209,7 +209,7 @@ intel_uxa_pixmap_compute_size(PixmapPtr pixmap, tile_height = 8; else tile_height = 32; - aligned_h = ALIGN(h, tile_height); + aligned_h = ALIGN(h, 2*tile_height); *stride = intel_get_fence_pitch(intel, ALIGN(pitch, 512), Look at the referenced bugs, what I found was a long list of random GPU hangs on ILK and SNB hardware that appear to have been caused by a kernel change. Daniel and Chris created a number of scatter shot fixes across the kernel, and this patch to the 2D driver. However, this patch doesn't appear to have actually solved anything; Norbert Preining was still crashing with this patch applied: https://bugs.freedesktop.org/show_bug.cgi?id=55984#c129 Furthermore, SNA has some similar code, but it applies it conditionally for hardware which doesn't have 'relaxed fencing', which is only hardware older than i965 when running on a older kernel that doesn't recognize the HAS_RELAXED_FENCING parameter (the current kernel always returns 'true'). So, as near as I can tell, this fix should never be necessary as the reported bug wasn't fixed by it, and SNA does not apply this rule to any hardware on which either bug was reproduced. If this fix is actually useful, wouldn't we want it in Mesa as well? -- keith.pack...@intel.com pgpyPVU4ToK8H.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx