- 27 Jul, 2017 40 commits
-
-
Chris Wilson authored
After we detect a i915_vma pin overflow, we call __i915_vma_unpin to cleanup. However, on an overflow the pin_count bitfield will be zero, triggering an assertion, even though we the intention is to merely warn and report the error back to the user (as historically the culprit has be a leak in the display code). Fixes: 20dfbde4 ("drm/i915: Wrap vma->pin_count accessors with small inline helpers") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721145037.25105-2-chris@chris-wilson.co.ukReviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Navare, Manasi D authored
The condition for setting the Loadgen Select bit of PORT_TX_DW4 register during DDI Vswing Sequence should be Bit rate <=6 GHz whereas the existing code checks only Bit Rate < 6GHz. This patch fixes this condition. While at it also remove the redundant paranthesis. Fixes: cf54ca8b ("drm/i915/cnl: Implement voltage swing sequence.") Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500329122-32662-1-git-send-email-manasi.d.navare@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
I wrote this code an year and a half ago and I couldn't exactly remember the main differences of these two structures when reviewing a new FBC patch. Add some comments to help explain what's the purpose of each struct. For the record, the original commits are: b183b3f1 ("drm/i915/fbc: introduce struct intel_fbc_reg_params") aaf78d27 ("drm/i915/fbc: introduce struct intel_fbc_state_cache") Cc: Praveen Paneri <praveen.paneri@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20170714193822.12121-1-paulo.r.zanoni@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
* Don't define it twice. * Define MSBs first, like the rest of i915_reg.h. * Add CNL_ prefix to the bit that arrived in CNL. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170714175228.27019-1-paulo.r.zanoni@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
After applying af2788925ae0 ("drm/i915: Squelch reset messages during selftests") out of sequence, I missed fixing up a call to i915_reset(). Reported-by: kbuild test robot <kbuild-all@01.org> Fixes: af2788925ae0 ("drm/i915: Squelch reset messages during selftests") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20170725125336.11969-1-chris@chris-wilson.co.uk Reviewed-by: David Weinehall <david.weinehall@linux.intel.com Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
During our selftests, we try reseting the GPU tens of thousands of times, flooding the dmesg with our reset spam drowning out any potential warnings. Add an option to i915_reset()/i915_reset_engine() to specify a quiet reset for selftesting. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-19-chris@chris-wilson.co.ukReviewed-by: Michel Thierry <michel.thierry@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
Extract the common barrier against rogue hangchecks from disrupting our direct testing of resets, and in the process expand the lock to include the per-engine reset shortcuts. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-17-chris@chris-wilson.co.ukReviewed-by: Michel Thierry <michel.thierry@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
If the request has been completed before the reset took effect, we don't need to mark it up as being a victim. Touching fence->error after the fence has been signaled is detected by dma_fence_set_error() and triggers a BUG: [ 231.743133] kernel BUG at ./include/linux/dma-fence.h:434! [ 231.743156] invalid opcode: 0000 [#1] SMP KASAN [ 231.743172] Modules linked in: i915 drm_kms_helper drm iptable_nat nf_nat_ipv4 nf_nat x86_pkg_temp_thermal iosf_mbi i2c_algo_bit cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea fb font fbdev [last unloaded: drm] [ 231.743221] CPU: 2 PID: 20 Comm: kworker/2:0 Tainted: G U 4.13.0-rc1+ #52 [ 231.743236] Hardware name: Hewlett-Packard HP EliteBook 8460p/161C, BIOS 68SCF Ver. F.01 03/11/2011 [ 231.743363] Workqueue: events_long i915_hangcheck_elapsed [i915] [ 231.743382] task: ffff8801f42e9780 task.stack: ffff8801f42f8000 [ 231.743489] RIP: 0010:i915_gem_reset_engine+0x45a/0x460 [i915] [ 231.743505] RSP: 0018:ffff8801f42ff770 EFLAGS: 00010202 [ 231.743521] RAX: 0000000000000007 RBX: ffff8801bf6b1880 RCX: ffffffffa02881a6 [ 231.743537] RDX: dffffc0000000000 RSI: dffffc0000000000 RDI: ffff8801bf6b18c8 [ 231.743551] RBP: ffff8801f42ff7c8 R08: 0000000000000001 R09: 0000000000000000 [ 231.743566] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801edb02d00 [ 231.743581] R13: ffff8801e19d4200 R14: 000000000000001d R15: ffff8801ce2a4000 [ 231.743599] FS: 0000000000000000(0000) GS:ffff8801f5a80000(0000) knlGS:0000000000000000 [ 231.743614] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 231.743629] CR2: 00007f0ebd1add10 CR3: 0000000002621000 CR4: 00000000000406e0 [ 231.743643] Call Trace: [ 231.743752] i915_gem_reset+0x6c/0x150 [i915] [ 231.743853] i915_reset+0x175/0x210 [i915] [ 231.743958] i915_reset_device+0x33b/0x350 [i915] [ 231.744061] ? valleyview_pipestat_irq_handler+0xe0/0xe0 [i915] [ 231.744081] ? trace_hardirqs_off_caller+0x70/0x110 [ 231.744102] ? _raw_spin_unlock_irqrestore+0x46/0x50 [ 231.744120] ? find_held_lock+0x119/0x150 [ 231.744138] ? mark_lock+0x6d/0x850 [ 231.744241] ? gen8_gt_irq_ack+0x1f0/0x1f0 [i915] [ 231.744262] ? work_on_cpu_safe+0x60/0x60 [ 231.744284] ? rcu_read_lock_sched_held+0x57/0xa0 [ 231.744400] ? gen6_read32+0x2ba/0x320 [i915] [ 231.744506] i915_handle_error+0x382/0x5f0 [i915] [ 231.744611] ? gen6_rps_reset_ei+0x20/0x20 [i915] [ 231.744630] ? vsnprintf+0x128/0x8e0 [ 231.744649] ? pointer+0x6b0/0x6b0 [ 231.744667] ? debug_check_no_locks_freed+0x1a0/0x1a0 [ 231.744688] ? scnprintf+0x92/0xe0 [ 231.744706] ? snprintf+0xb0/0xb0 [ 231.744820] hangcheck_declare_hang+0x15a/0x1a0 [i915] [ 231.744932] ? engine_stuck+0x440/0x440 [i915] [ 231.744951] ? rcu_read_lock_sched_held+0x57/0xa0 [ 231.745062] ? gen6_read32+0x2ba/0x320 [i915] [ 231.745173] ? gen6_read16+0x320/0x320 [i915] [ 231.745284] ? intel_engine_get_active_head+0x91/0x170 [i915] [ 231.745401] i915_hangcheck_elapsed+0x3d8/0x400 [i915] [ 231.745424] process_one_work+0x3e8/0xac0 [ 231.745444] ? pwq_dec_nr_in_flight+0x110/0x110 [ 231.745464] ? do_raw_spin_lock+0x8e/0x120 [ 231.745484] worker_thread+0x8d/0x720 [ 231.745506] kthread+0x19e/0x1f0 [ 231.745524] ? process_one_work+0xac0/0xac0 [ 231.745541] ? kthread_create_on_node+0xa0/0xa0 [ 231.745560] ret_from_fork+0x27/0x40 [ 231.745581] Code: 8b 7d c8 e8 49 0d 02 e1 49 8b 7f 38 48 8b 75 b8 48 83 c7 10 e8 b8 89 be e1 e9 95 fc ff ff 4c 89 e7 e8 4b b9 ff ff e9 30 ff ff ff <0f> 0b 0f 1f 40 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fe [ 231.745767] RIP: i915_gem_reset_engine+0x45a/0x460 [i915] RSP: ffff8801f42ff770 At first glance this looks to be related to commit c64992e0 ("drm/i915: Look for active requests earlier in the reset path"), but it could easily happen before as well. On the other hand, we no longer logged victims due to the active_request being dropped earlier. v2: Be trickier to unwind the incomplete request as we cannot rely on request retirement for the lockless per-engine reset. v3: Reprobe the active request at the time of the reset. Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch> Fixes: c64992e0 ("drm/i915: Look for active requests earlier in the reset path") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-15-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1 Reviewed-by: Michel Thierry <michel.thierry@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
We require the caller to ensure that the packets they wish to emit into the CS ring are qword aligned (i.e. have an even number of dwords). Double check this. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721161101.1618-1-chris@chris-wilson.co.ukReviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Shashank Sharma authored
This patch sets the is_hdmi2_src identifier in drm connector for GLK platform. GLK contains a native HDMI 2.0 controller. This identifier will help the EDID handling functions to save lot of work which is specific to HDMI 2.0 sources. V3: Added this patch V4: Rebase V4: Rebase V5: Added r-b from Ander V6: Rebase V7: Rebase V8: Rebase V9: Added r-b from Ville V9: Added r-b from Imre Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Imre Deak <imre.deak@intel.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500650709-14447-7-git-send-email-shashank.sharma@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Shashank Sharma authored
When output colorspace is YCBCR420, we have to load the corresponding colorspace in AVI infoframe. This patch fills the colorspace of AVI infoframe as per the output mode. V2: Rebase V3: Rebase V4: Rebase V5: Added r-b from Ander V6: Checking RGB/YCBCR420 output only (Ville) V7: Add colorspace info in driver(not drm layer) (Ville) V8: Rebase V9: Added r-b from Ville V10: Added r-b from Imre Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Imre Deak <imre.deak@intel.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500650709-14447-6-git-send-email-shashank.sharma@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Shashank Sharma authored
To support ycbcr output, we need a pipe CSC block to do RGB->YCBCR conversion. Current Intel platforms have only one pipe CSC unit, so we can either do color correction using it, or we can perform RGB->YCBCR conversion. This function adds a csc handler, which uses recommended bspec values to perform RGB->YCBCR conversion (target color space BT709) V2: Rebase V3: Rebase V4: Rebase V5: Addressed review comments from Ander - Remove extra line added in the patch - Add the spec details in the commit message - Combine two if(cond) while calling intel_crtc_compute_config V6: Handle YCBCR420 outputs only (Ville) V7: Addressed review comments from Ville: - Add description about target colorspace - Remove the comments from CSC function - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy - Remove unnecessary debug message about YCBCR420 possibe V8: Addressed review comments from Ville: - Remove extra comment, not required. - Do not add extra variable for CTM, reuse pipe_config Added r-b from Ville V9: Remove extra whitespace (Imre) V10: Added r-b from Imre Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Imre Deak <imre.deak@intel.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500650709-14447-5-git-send-email-shashank.sharma@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Shashank Sharma authored
To get HDMI YCBCR420 output, the PIPEMISC register should be programmed to: - Generate YCBCR output (bit 11) - In case of YCBCR420 outputs, it should be programmed in full blend mode to use the scaler in 5x3 ratio (bits 26 and 27) This patch: - Adds definition of these bits. - Programs PIPEMISC for YCBCR420 outputs. - Adds readouts to compare HW and SW states. V2: rebase V3: rebase V4: rebase V5: added r-b from Ander V6: Handle only YCBCR420 outputs (ville) V7: rebase V8: Addressed review comments from Ville - Add readouts for state->ycbcr420 and 420 pixel_clock. - Handle warning due to mismatch in clock for ycbcr420 clock. - Rename PIPEMISC macros to match the Bspec. - Add a debug print stating if YCBCR 4:2:0 output enabled. Added r-b from Ville V9: Addressed review comments from Imre: - Add 420 mode clock adjustment in intel_hdmi_mode_valid to prevent 420_only modes getting rejected for high clock. - Add port clock adjustment for ycbcr420 modes in ddi_get_clock - Rename macros as per Ville's suggestion. - Remove unnecessary wl changes. V10: Added r-b from Imre V11: Fixed faulty dotclock handling, and addressed missing comment from previous set of review comments (Imre) V12: Fixed dotclock for 12bpc too, removed 420 check for GEN < 10 Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Imre Deak <imre.deak@intel.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500904172-31717-1-git-send-email-shashank.sharma@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Shashank Sharma authored
To get a YCBCR420 output from intel platforms, we need one scaler to scale down YCBCR444 samples to YCBCR420 samples. This patch: - Does scaler allocation for HDMI ycbcr420 outputs. - Programs PIPE_MISC register for ycbcr420 output. V2: rebase V3: rebase V4: rebase V5: addressed review comments from Ander: - No need to check both scaler_user && hdmi_output. Check for scaler_user is enough. V6: rebase V7: Do not create a new scaler user, use existing pipe scaler user. V8: rebase V9: Addressed review comments from Ville: - Remove leftover comment for HDMI scaler user. - Remove unnecessary blank line. - Make scaler alocation failure a DEBUG log instead of ERROR. Added r-b from Ville V10: Update commit message as per latest code (Imre) Added r-b from Imre Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com> Cc: Imre Deak <imre.deak@intel.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500650709-14447-3-git-send-email-shashank.sharma@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Shashank Sharma authored
This patch checks encoder level support for YCBCR420 outputs. The logic goes as simple as this: If the input mode is YCBCR420-only mode: prepare HDMI for YCBCR420 output, else continue with RGB output mode. It checks if the mode is YCBCR420 and source can support this output then it marks the ycbcr_420 output indicator into crtc state, for further staging in driver. V2: Split the patch into two, kept helper functions in DRM layer. V3: Changed the compute_config function based on new DRM API. V4: Rebase V5: Rebase V6: Check and handle YCBCR420-only modes, discard the property based approach (Ville) V7: Addressed review comments from Ville - add else case in 12BPC check. - extract ycbcr420 state inside hdmi_12bpc_possible function. V8: Addressed review comments from Ville - Remove extra blank lines. - Remove "HDMI" from the description of ycbcr420 state variable. - Remove local variable, use crtc_state->ycbcr420 instead. Added r-b from Ville. V9: Rebase V10: Added r-b from Imre Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1500650709-14447-2-git-send-email-shashank.sharma@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Shuffle the power well->domain mapping macros around so they are at one place in old->new GEN order. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-19-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Move the helper next to the rest of HSW specific code. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-18-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
After the previous refactorings the HSW/BDW and GEN9+ power well helpers are practically identical, so use the HSW power well helpers for GEN9+ too. This means using the HSW power well ops instead of the SKL one and setting the irq_pipe_mask, has_vga and has_fuses attributes as needed. v2: - Rebased on v2 of patch 15. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170711204236.5618-7-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
The pattern of a power well backing a set of fuses whose initialization we need to wait for during power well enabling is common to all GEN9+ platforms. Adding support for this to the HSW power well enable helper allows us to use the HSW/BDW power well code for GEN9+ as well in a follow-up patch. v2: - Use an enum for power gates instead of raw numbers. (Ville) Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170711204236.5618-6-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Similarly to GEN9+ waiting for the power well disabled state is a safer option and also provides diagnostic info if the disabling didn't succeed or the power well was forced on by an external requester. While at it also use the existing GEN9+ helper to wait for the enabled state. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-15-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
The pattern of a power well backing a set of pipe IRQ or VGA functionality applies to all HSW+ platforms. Using power well attributes instead of platform checks to decide whether to init/reset pipe IRQs and VGA correspondingly is cleaner and it allows us to unify the HSW/BDW and GEN9+ power well code in follow-up patches. Also use u8 for pipe_mask in related helpers to match the type in the power well struct. v2: - Use u8 instead of u32 for irq_pipe_mask. (Ville) v3: - Use u8 for pipe_mask in related helpers too for clarity. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170712155413.29839-1-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Although on HSW/BDW there is only a single display global power well, it's programmed the same way as other GEN9+ power wells. This also means we can get at the HSW/BDW request and status flags the same way it's done on GEN9+ by assigning the corresponding HSW/BDW power well ID. This ID was assigned in a recent patch, so we can now switch to using the same macros everywhere on HSW+. Updating the HSW power well control register with RMW is not strictly necessary, but this will allow us to use the same code for GEN9+. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-13-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
We can reduce the code indentation by splitting the set helper to separate enable/disable helpers. This also allows us to unify the HSW/BDW and GEN9+ power well ops in follow-up patches, which introduces some differences between the enable and disable helpers. While at it also remove the redundant enable/disable debug messages, the same info is printed already elsewhere. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-12-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Similarly to the GEN9 power well toggling, saving an occasional extra MMIO write is not worth the code complexity, let's simplify things. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-11-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Atm we enable/disable a power well only if it wasn't already enabled/disabled respectively. The only reason for this I can think of is to save the extra MMIO writes. Since the HW state matches the power well's usage counter most of the time the overhead due to these MMIOs is insignificant. Let's simplify the code by making the writes unconditional. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-10-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
We check already for power wells that are unexpectedly on (or forced on) during power well disabling. Those checks also account for other power well requesters like KVMR or DEBUG. As such this check is redundant, let's remove it to simplify things. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-9-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Follow-up patches will add new fields to the i915_power_well struct that are specific to the hsw_power_well_ops helpers. Prepare for this by changing the generic 'data' field to a union of platform specific structs. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-8-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Check that all the power well IDs are unique on the given platform. v2: - Fix using BIT_ULL() instead of BIT() for 64 bit mask. v3: - Move the check to a separate function. (Ville) Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170711204236.5618-4-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Add an ID for the HSW/BDW global display power well for consistency. The ID is selected so that it can be used to get at the HW request and status flags with the corresponding GEN9+ macros. Unifying the HSW/BDW and GEN9+ versions of these macros and the power well ops using them will be done in follow-up patches. v2: - Rebased on v2 of patch 2. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170711204236.5618-3-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Make the I830 power well ID assignment explicit for consistency. v2: - s/GEN2/I830/ in the comment, since other GEN2s don't have the power well. (Ville) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170711204236.5618-2-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Power well IDs are used for lookup so they must be unique. To ensure this assign the always-on power well ID everywhere where it's missing. This didn't cause a problem so far, since we didn't need to look up power wells that happened to share their IDs. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-4-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Atm, the power well IDs are defined in separate platform specific enums, which isn't ideal for the following reasons: - the IDs are used by helpers like lookup_power_well() in a platform independent way - the always-on power well is used by multiple platforms and so needs now separate IDs, although these IDs refer to the same thing To make things more consistent use a single enum instead of the two separate ones, listing the IDs per platform (or set of very similar platforms like all GEN9/10). Replace the separate always-on power well IDs with a single ID. While at it also add a note clarifying the distinction between regular power wells that follow a common programming pattern and custom ones that are programmed in some other way. The IDs for regular power wells need to stay fixed, since they also define the request and state HW flag positions in their corresponding power well control register(s). v2: - Add comment about id to req,status bit mapping to the enum. (Rodrigo) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170711204236.5618-1-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
The power well IDs are used for lookup, so they must be unique on a given platform; ensure this on CHV. This didn't cause an actual problem since we didn't need to look up power wells which happened to share an ID. Mark this new power well as custom, since its programming pattern doesn't follow that of the rest of VLV/CHV power wells. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1499352040-8819-2-git-send-email-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
The crtc state starts out being bzero'd, so no need to clear scaler_users. Also intel_crtc_init_scalers() knows already which platforms have scalers, so no need for the platform check here. Similarly intel_crtc_init_scalers() will init scaler_id as required, so no need to do it here separately. Cc: Chandra Konduru <chandra.konduru@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170719225057.20131-2-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
The scaler allocation code depends on a non-zero default value for the crtc scaler_id, so make sure we initialize the scaler state accordingly even if the crtc is off. This fixes at least an initial YUV420 modeset (added in a follow-up patchset by Shashank) when booting with the screen off: after the initial HW readout and modeset which enables the scaler a subsequent modeset will disable the scaler which isn't properly allocated. This results in a funky HW state where the pipe scaler HW registers can't be modified and the normally black screen is grey and shifted to the right or jitters. The problem was revealed by Shashank's YUV420 patchset and first reported by Ville. v2: - In the stable tag also include versions which need backporting (Jani) Cc: Jani Nikula <jani.nikula@intel.com> Cc: Shashank Sharma <shashank.sharma@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Chandra Konduru <chandra.konduru@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: <stable@vger.kernel.org> # 4.2.x Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Fixes: a1b2278e ("drm/i915: skylake panel fitting using shared scalers") Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170720112820.26816-1-imre.deak@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
If all goes well, resetting one engine should not affect the operation of any others. So to test this, we setup a continuous stream of requests onto to each of the "innocent" engines whilst constantly resetting our target engine. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Reviewed-by: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-16-chris@chris-wilson.co.ukSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
Triggering a GPU reset for one engine affects another, notably corrupting the context status buffer (CSB) effectively losing track of inflight requests. Adding a few printks: diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ad41836fa5e5..a969456bc0fa 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1953,6 +1953,7 @@ int i915_reset_engine(struct intel_engine_cs *engine) goto out; } + pr_err("Resetting %s\n", engine->name); ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); if (ret) { /* If we fail here, we expect to fallback to a global reset */ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 716e5c9ea222..a72bc35d0870 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -355,6 +355,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); port_set(&port[n], port_pack(rq, count)); desc = execlists_update_context(rq); + pr_err("%s: in (rq=%x) ctx=%d\n", engine->name, rq->global_seqno, upper_32_bits(desc)); GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc)); } else { GEM_BUG_ON(!n); @@ -594,9 +595,23 @@ static void intel_lrc_irq_handler(unsigned long data) if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK)) continue; + pr_err("%s: out CSB (%x head=%d, tail=%d), ctx=%d, rq=%d\n", + engine->name, + readl(csb_mmio), + head, tail, + readl(buf+2*head+1), + port->context_id); + /* Check the context/desc id for this event matches */ - GEM_DEBUG_BUG_ON(readl(buf + 2 * head + 1) != - port->context_id); + if (readl(buf + 2 * head + 1) != port->context_id) { + pr_err("%s: BUG CSB (%x head=%d, tail=%d), ctx=%d, rq=%d\n", + engine->name, + readl(csb_mmio), + head, tail, + readl(buf+2*head+1), + port->context_id); + BUG(); + } rq = port_unpack(port, &count); GEM_BUG_ON(count == 0); Results in: [ 6423.006602] Resetting rcs0 [ 6423.009080] rcs0: in (rq=fffffe70) ctx=1 [ 6423.009216] rcs0: in (rq=fffffe6f) ctx=3 [ 6423.009542] rcs0: out CSB (2 head=1, tail=2), ctx=3, rq=3 [ 6423.009619] Resetting bcs0 [ 6423.009980] rcs0: BUG CSB (0 head=1, tail=2), ctx=0, rq=3 Note that this bug may be affect all machines and not just Broxton, Broxton is just the first machine on which I have confirmed this bug. Fixes: 142bc7d9 ("drm/i915: Modify error handler for per engine hang recovery") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Acked-by: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-13-chris@chris-wilson.co.ukSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
Although a banned context will be told to -EIO off if they try to submit more requests, we have a discrepancy between whole device resets and per-engine resets where we report the GPU reset but not the engine resets. This leaves a bit of mystery as to why the context was banned, and also reduces awareness overall of when a GPU (engine) reset occurs with its possible side-effects. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-13-chris@chris-wilson.co.ukSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
Since we make call i915_gem_context_mark_guilty() concurrently when resetting different engines in parallel, we need to make sure that our updates are safe for the unlocked access. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-12-chris@chris-wilson.co.ukSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
When the GPU is reset, we want to discard all pending notifications as either we have manually completed them, or they are no longer applicable. Make sure we do reset the engine->irq_posted prior to re-enabling the engine (e.g. the interrupt tasklets) in i915_gem_reset_finish_engine(). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-11-chris@chris-wilson.co.ukSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-