[Intel-gfx] [PATCH 4/5] drm/i915: Use crtc-state-active in ilk/skl watermark calculations
Existing watermark code calls intel_crtc_active() to determine whether a CRTC is active for the purpose of watermark calculations (and bails out early if it determines the CRTC is not active). However intel_crtc_active() only returns true if crtc-primary-fb is non-NULL, which isn't appropriate in the modern age of universal planes and atomic modeset since userspace can now disable the primary plane, but leave the CRTC (and other planes) running. Note that commit commit 0fda65680e92545caea5be7805a7f0a617fb6c20 Author: Tvrtko Ursulin tvrtko.ursu...@intel.com Date: Fri Feb 27 15:12:35 2015 + drm/i915/skl: Update watermarks for Y tiling adds a test for primary plane enable/disable to trigger a watermark update (previously we ignored updates to primary planes, which wasn't really correct, but we got lucky since we always pretended the primary plane was on). Tvrtko's patch tries to update watermarks when we re-enable the primary plane, but that watermark computation gets aborted early because intel_crtc_active() returns false due to the disabled primary plane. Switch the ILK and SKL watermark code over to use crtc-state-active rather than calling intel_crtc_active() so that we'll properly compute watermarks when re-enabling the primary plane. Note that this commit doesn't touch callsites in the watermark code for older platforms since there were concerns that doing so would lead to other types of breakage. Also note that all of the watermark calculation at the moment takes place after new crtc/plane states are swapped into the DRM objects. This will change in the future, so we'll be working with in-flight state objects, but for the time being, crtc-state is what we want to operate on. v2: Don't drop primary-fb check from intel_crtc_active(), but rather replace ILK/SKL callsites with direct tests of crtc-state-active. There is concern that messing with intel_crtc_active() will lead to other breakage for old hardware platforms. (Ville) Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Tvrtko Ursulin tvrtko.ursu...@intel.com Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d9b115e..dafd7de 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -822,7 +822,7 @@ static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc, * FIXME the plane might have an fb * but be invisible (eg. due to clipping) */ - if (!intel_crtc-base.state-active || !plane-state-fb) + if (!crtc-state-active || !plane-state-fb) return 0; if (WARN(clock == 0, Pixel clock is zero!\n)) @@ -1701,7 +1701,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) struct drm_display_mode *mode = intel_crtc-config-base.adjusted_mode; u32 linetime, ips_linetime; - if (!intel_crtc_active(crtc)) + if (!crtc-state-active) return 0; /* The WM are computed with base on how long it takes to fill a single @@ -1956,7 +1956,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, enum pipe pipe = intel_crtc-pipe; struct drm_plane *plane; - if (!intel_crtc_active(crtc)) + if (!crtc-state-active) return; p-active = true; @@ -2468,7 +2468,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, nth_active_pipe = 0; for_each_crtc(dev, crtc) { - if (!intel_crtc_active(crtc)) + if (!crtc-state-active) continue; if (crtc == for_crtc) @@ -2708,7 +2708,7 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev, struct drm_plane *plane; list_for_each_entry(crtc, dev-mode_config.crtc_list, head) - config-num_pipes_active += intel_crtc_active(crtc); + config-num_pipes_active += crtc-state-active; /* FIXME: I don't think we need those two global parameters on SKL */ list_for_each_entry(plane, dev-mode_config.plane_list, head) { @@ -2729,7 +2729,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, struct drm_framebuffer *fb; int i = 1; /* Index for sprite planes start */ - p-active = intel_crtc_active(crtc); + p-active = crtc-state-active; if (p-active) { p-pipe_htotal = intel_crtc-config-base.adjusted_mode.crtc_htotal; p-pixel_rate = skl_pipe_pixel_rate(intel_crtc-config); @@ -2860,7 +2860,7 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv, static uint32_t skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p) { - if (!intel_crtc_active(crtc)) +
[Intel-gfx] [PATCH 2/5] drm/i915: Kill intel_crtc-active
Replace all uses of intel_crtc-active with crtc-state-active. At the moment we don't have atomic state handling wired up for CRTC's so this means we just directly set the state active field at various points in our (legacy) modeset pipeline. Once we have CRTC atomic states properly hooked up, crtc_state-active will be set during the check phase. Patch generated with Coccinelle via: // If we already have the base pointer, don't access it through // intel_crtc-base needlessly. @@ expression E; struct intel_crtc *IC; @@ IC = to_intel_crtc(E); +... - IC-active + E-state-active ...+ // Not sure why the rule above doesn't catch these, but this seems // to mop up the remainders. @@ identifier I; expression E; @@ { struct intel_crtc *I = to_intel_crtc(E); +... - I-active + E-state-active ...+ } // We might have left an unused intel_crtc in some functions; clean that up. @@ identifier IC; constant C; @@ { ... - struct intel_crtc *IC; ... when != IC } // Change all of the other uses of intel_crtc-active @@ struct intel_crtc *IC; @@ - IC-active + IC-base.state-active // Ditto @@ expression E; @@ - to_intel_crtc(E)-active + E-state-active // Remove the active field from intel_crtc @@ @@ struct intel_crtc { ... - bool active; ... }; Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 9 ++- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +- drivers/gpu/drm/i915/intel_display.c | 129 +++--- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_drv.h | 6 -- drivers/gpu/drm/i915/intel_fbdev.c| 6 +- drivers/gpu/drm/i915/intel_overlay.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 10 +-- drivers/gpu/drm/i915/intel_sprite.c | 6 +- 10 files changed, 86 insertions(+), 92 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3f64786..28176fd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2656,9 +2656,10 @@ static int i915_display_info(struct seq_file *m, void *unused) seq_printf(m, CRTC %d: pipe: %c, active=%s (size=%dx%d)\n, crtc-base.base.id, pipe_name(crtc-pipe), - yesno(crtc-active), crtc-config-pipe_src_w, + yesno(crtc-base.state-active), + crtc-config-pipe_src_w, crtc-config-pipe_src_h); - if (crtc-active) { + if (crtc-base.state-active) { intel_crtc_info(m, crtc); active = cursor_position(dev, crtc-pipe, x, y); @@ -2951,7 +2952,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused) for_each_intel_crtc(dev, intel_crtc) { drm_modeset_lock(intel_crtc-base.mutex, NULL); - if (intel_crtc-active) { + if (intel_crtc-base.state-active) { active_crtc_cnt++; seq_printf(m, \nCRTC %d: , active_crtc_cnt); @@ -3650,7 +3651,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, pipe_name(pipe)); drm_modeset_lock(crtc-base.mutex, NULL); - if (crtc-active) + if (crtc-base.state-active) intel_wait_for_vblank(dev, pipe); drm_modeset_unlock(crtc-base.mutex); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9baecb7..ac834de 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -649,7 +649,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, int ret = 0; unsigned long irqflags; - if (!intel_crtc-active) { + if (!intel_crtc-base.state-active) { DRM_DEBUG_DRIVER(trying to get scanoutpos for disabled pipe %c\n, pipe_name(pipe)); return 0; diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 976b891..43cb21f 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -144,9 +144,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane, intel_state-clip.x1 = 0; intel_state-clip.y1 = 0; intel_state-clip.x2 = -
[Intel-gfx] [PATCH 3/5] drm/i915: Update intel_crtc_active() to use state values
With the switch to atomic plumbing for planes, some of our commit-time work (e.g., watermarks) is done after the new atomic state is swapped into the relevant DRM object, but before the DRM core has a chance to update its legacy state values. Switch intel_crtc_active() to look at the state objects rather than legacy fields to ensure we operate on the proper values. Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b11528f..4f8c622d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -886,8 +886,6 @@ chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc, bool intel_crtc_active(struct drm_crtc *crtc) { - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - /* Be paranoid as we can arrive here with only partial * state retrieved from the hardware during setup. * @@ -897,8 +895,8 @@ bool intel_crtc_active(struct drm_crtc *crtc) * We can ditch the crtc-primary-fb check as soon as we can * properly reconstruct framebuffers. */ - return crtc-state-active crtc-primary-fb - intel_crtc-config-base.adjusted_mode.crtc_clock; + return crtc-state-active crtc-primary-state-fb + crtc-state-adjusted_mode.crtc_clock; } enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: Don't assume primary cursor are always on for wm calculation (v3)
Current ILK-style watermark code assumes the primary plane and cursor plane are always enabled. This assumption, along with the combination of two independent commits that got merged at the same time, results in a NULL dereference. The offending commits are: commit fd2d61341bf39d1054256c07d6eddd624ebc4241 Author: Matt Roper matthew.d.ro...@intel.com Date: Fri Feb 27 10:12:01 2015 -0800 drm/i915: Use plane-state-fb in watermark code (v2) and commit 0fda65680e92545caea5be7805a7f0a617fb6c20 Author: Tvrtko Ursulin tvrtko.ursu...@intel.com Date: Fri Feb 27 15:12:35 2015 + drm/i915/skl: Update watermarks for Y tiling The first commit causes us to use the FB from plane-state-fb rather than the legacy plane-fb, which is updated a bit later in the process. The second commit includes a change that now triggers watermark reprogramming on primary plane enable/disable where we didn't have one before (which wasn't really correct, but we had been getting lucky because we always calculated as if the primary plane was on). Together, these two commits cause the watermark calculation to (properly) see plane-state-fb = NULL when we're in the process of disabling the primary plane. However the existing watermark code assumes there's always a primary fb and tries to dereference it to find out pixel format / bpp information. The fix is to make ILK-style watermark calculation actually check the true status of primary cursor planes and adjust our watermark logic accordingly. v2: Update unchecked uses of state-fb for other platforms (pnv, skl, etc.). Note that this is just a temporary fix. Ultimately the useful information is going to be computed at check time and stored right in the state structures so that we don't have to figure this all out while we're supposed to be programming the watermarks. (caught by Tvrtko) v3: Fix a couple copy/paste mistakes in SKL code. (Tvrtko) Cc: Tvrtko Ursulin tvrtko.ursu...@intel.com Reported-by: Michael Leuchtenburg mich...@slashhome.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89388 Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 113 +--- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index dafd7de..c5d3238 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -608,12 +608,17 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc) crtc = single_enabled_crtc(dev); if (crtc) { const struct drm_display_mode *adjusted_mode; - int pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + int pixel_size; int clock; adjusted_mode = to_intel_crtc(crtc)-config-base.adjusted_mode; clock = adjusted_mode-crtc_clock; + if (crtc-primary-state-fb) + pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + else + pixel_size = 4; + /* Display SR */ wm = intel_calculate_wm(clock, pineview_display_wm, pineview_display_wm.fifo_size, @@ -684,7 +689,11 @@ static bool g4x_compute_wm0(struct drm_device *dev, clock = adjusted_mode-crtc_clock; htotal = adjusted_mode-crtc_htotal; hdisplay = to_intel_crtc(crtc)-config-pipe_src_w; - pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + + if (crtc-primary-state-fb) + pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + else + pixel_size = 4; /* Use the small buffer method to calculate plane watermark */ entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000; @@ -771,7 +780,11 @@ static bool g4x_compute_srwm(struct drm_device *dev, clock = adjusted_mode-crtc_clock; htotal = adjusted_mode-crtc_htotal; hdisplay = to_intel_crtc(crtc)-config-pipe_src_w; - pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + + if (crtc-primary-state-fb) + pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + else + pixel_size = 4; line_time_us = max(htotal * 1000 / clock, 1); line_count = (latency_ns / line_time_us + 1000) / 1000; @@ -1118,10 +1131,15 @@ static void i965_update_wm(struct drm_crtc *unused_crtc) int clock = adjusted_mode-crtc_clock; int htotal = adjusted_mode-crtc_htotal; int hdisplay = to_intel_crtc(crtc)-config-pipe_src_w; - int pixel_size = crtc-primary-state-fb-bits_per_pixel / 8; + int pixel_size; unsigned long line_time_us; int entries; + if (crtc-primary-state-fb) +
[Intel-gfx] [PATCH 0/5] Fix recent watermark breakage
A couple recent changes to the display watermark code by Tvrtko and me haven't played well together and resulted in a situation where we can miscalculate watermarks or even panic the kernel by disabling/enabling the primary display plane. This series should clean up the problem areas and prevent these issues until the proper atomic watermark rework is ready. Matt Roper (5): drm/i915: Ensure crtc_state backpointer is initialized drm/i915: Kill intel_crtc-active drm/i915: Update intel_crtc_active() to use state values drm/i915: Use crtc-state-active in ilk/skl watermark calculations drm/i915: Don't assume primary cursor are always on for wm calculation (v3) drivers/gpu/drm/i915/i915_debugfs.c | 9 +- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +- drivers/gpu/drm/i915/intel_display.c | 134 ++-- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_drv.h | 6 -- drivers/gpu/drm/i915/intel_fbdev.c| 6 +- drivers/gpu/drm/i915/intel_overlay.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 139 -- drivers/gpu/drm/i915/intel_sprite.c | 6 +- 10 files changed, 178 insertions(+), 134 deletions(-) -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Ensure crtc_state backpointer is initialized
From: Matt Roper m...@mattrope.com Since our legacy modeset paths directly kzalloc CRTC state objects at the moment rather than calling intel_crtc_duplicate_state(), we need to manually ensure the -crtc backpointer is always initialized. Signed-off-by: Matt Roper m...@mattrope.com --- drivers/gpu/drm/i915/intel_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 43d3575..188f87f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11228,6 +11228,7 @@ static int __intel_set_mode(struct drm_crtc *crtc, /* mode_set/enable/disable functions rely on a correct pipe * config. */ intel_crtc_set_state(to_intel_crtc(crtc), pipe_config); +pipe_config-base.crtc = intel_crtc-base; /* * Calculate and store various constants which -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] [PATCH 1/2] Add driver callback for updating device info
This patchset is a must for beignet to support CHV. One comment is that we should put the usage of these new libdrm APIs to conditional block thus we don't break the build on old system. For the other parts of the patchset: Reviewed-by: Zhigang Gong zhigang.g...@linux.intel.com Thanks, Zhigang Gong. -Original Message- From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of jeff.mc...@intel.com Sent: Tuesday, March 3, 2015 7:43 AM To: beig...@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org Subject: [Beignet] [PATCH 1/2] Add driver callback for updating device info From: Jeff McGee jeff.mc...@intel.com We need to update some fields of the device's cl_device_id struct at runtime using driver-specific methods. It is best to group all such updates into a single driver callback to avoid opening/initing and deiniting/closing the device multiple times. Signed-off-by: Jeff McGee jeff.mc...@intel.com --- src/cl_device_id.c | 20 ++-- src/cl_driver.h | 4 src/cl_driver_defs.c | 1 + src/intel/intel_driver.c | 36 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/cl_device_id.c b/src/cl_device_id.c index 4e01c9f..fefcef3 100644 --- a/src/cl_device_id.c +++ b/src/cl_device_id.c @@ -506,24 +506,8 @@ skl_gt4_break: ret-profile_sz = strlen(ret-profile) + 1; } -#ifdef HAS_USERPTR - cl_driver dummy = cl_driver_new(NULL); - cl_buffer_mgr bufmgr = cl_driver_get_bufmgr(dummy); - - const size_t sz = 4096; - void* host_ptr = cl_aligned_malloc(sz, 4096);; - if (host_ptr != NULL) { -cl_buffer bo = cl_buffer_alloc_userptr(bufmgr, CL memory object, host_ptr, sz, 0); -if (bo == NULL) - ret-host_unified_memory = CL_FALSE; -else - cl_buffer_unreference(bo); -cl_free(host_ptr); - } - else -ret-host_unified_memory = CL_FALSE; - cl_driver_delete(dummy); -#endif + /* Apply any driver-dependent updates to the device info */ + cl_driver_update_device_info(ret); struct sysinfo info; if (sysinfo(info) == 0) { diff --git a/src/cl_driver.h b/src/cl_driver.h index 16f8bba..3f54a27 100644 --- a/src/cl_driver.h +++ b/src/cl_driver.h @@ -376,6 +376,10 @@ extern cl_buffer_get_tiling_align_cb *cl_buffer_get_tiling_align; typedef int (cl_driver_get_device_id_cb)(void); extern cl_driver_get_device_id_cb *cl_driver_get_device_id; +/* Update the device info */ +typedef void (cl_driver_update_device_info_cb)(cl_device_id device); +extern cl_driver_update_device_info_cb *cl_driver_update_device_info; + /*** *** * cl_khr_gl_sharing. **/ diff --git a/src/cl_driver_defs.c b/src/cl_driver_defs.c index 2b68539..9a47210 100644 --- a/src/cl_driver_defs.c +++ b/src/cl_driver_defs.c @@ -26,6 +26,7 @@ LOCAL cl_driver_delete_cb *cl_driver_delete = NULL; LOCAL cl_driver_get_bufmgr_cb *cl_driver_get_bufmgr = NULL; LOCAL cl_driver_get_ver_cb *cl_driver_get_ver = NULL; LOCAL cl_driver_get_device_id_cb *cl_driver_get_device_id = NULL; +LOCAL cl_driver_update_device_info_cb *cl_driver_update_device_info = +NULL; /* Buffer */ LOCAL cl_buffer_alloc_cb *cl_buffer_alloc = NULL; diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c index ff0cf27..d61988c 100644 --- a/src/intel/intel_driver.c +++ b/src/intel/intel_driver.c @@ -754,6 +754,41 @@ static int intel_buffer_set_tiling(cl_buffer bo, return ret; } +static void +intel_update_device_info(cl_device_id device) { #ifdef HAS_USERPTR + intel_driver_t *driver; + const size_t sz = 4096; + void *host_ptr; + + driver = intel_driver_new(); + assert(driver != NULL); + if (intel_driver_open(driver, NULL) != CL_SUCCESS) { +intel_driver_delete(driver); +return; + } + + host_ptr = cl_aligned_malloc(sz, 4096); if (host_ptr != NULL) { +cl_buffer bo = intel_buffer_alloc_userptr((cl_buffer_mgr)driver-bufmgr, + CL memory object, host_ptr, sz, 0); +if (bo == NULL) + device-host_unified_memory = CL_FALSE; +else + drm_intel_bo_unreference((drm_intel_bo*)bo); +cl_free(host_ptr); + } + else +device-host_unified_memory = CL_FALSE; + + intel_driver_context_destroy(driver); + intel_driver_close(driver); + intel_driver_terminate(driver); + intel_driver_delete(driver); +#endif +} + LOCAL void intel_setup_callbacks(void) { @@ -762,6 +797,7 @@ intel_setup_callbacks(void) cl_driver_get_ver = (cl_driver_get_ver_cb *) intel_driver_get_ver; cl_driver_get_bufmgr = (cl_driver_get_bufmgr_cb *) intel_driver_get_bufmgr; cl_driver_get_device_id = (cl_driver_get_device_id_cb *) intel_get_device_id; + cl_driver_update_device_info =
Re: [Intel-gfx] [Beignet] [PATCH] drm/i915: Export total subslice and EU counts
-Original Message- From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of Jeff McGee Sent: Saturday, March 7, 2015 2:44 AM To: Zhigang Gong Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org; beig...@lists.freedesktop.org; dri-de...@lists.freedesktop.org Subject: Re: [Beignet] [PATCH] drm/i915: Export total subslice and EU counts On Thu, Mar 05, 2015 at 12:35:55PM +0800, Zhigang Gong wrote: There is one minor conflict when apply the KMD patch to latest drm-intel-nightly branch. It should be easy to fix. Another issue is that IMO, we should bump libdrm's version number when increase these new APIs. Then in Beignet, we can check the libdrm version at build time and determine whether we will use these new interfaces. Thus, we can avoid breaking beignet on those systems which have previous libdrm/kernel installed. Right. I can append a libdrm patch to bump the version. And then I suppose I will follow the process to make a new release. Not sure right now how that works. First time going through it. Also, how should we test for the libdrm version and conditionally use the API? We can check the libdrm version at configuration time and define a macro to indicate whether we can use these new APIs in beignet. Is there a previous example of this in Beignet that I could follow? Yes, one example is userptr. You can check the usage of DRM_INTEL_USERPTR and HAS_USERPTR In beignet. Thanks, Zhigang Gong. Jeff The other parts of the whole patchset, including patches for KMD/libdrm/Intel gpu tools and Beignet, all look good to me. And I just tested it on BDW and SKL platforms, it works fine. Thanks, Zhigang Gong. On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mc...@intel.com wrote: From: Jeff McGee jeff.mc...@intel.com Setup new I915_GETPARAM ioctl entries for subslice total and EU total. Userspace drivers need these values when constructing GPGPU commands. This kernel query method is intended to replace the PCI ID-based tables that userspace drivers currently maintain. The kernel driver can employ fuse register reads as needed to ensure the most accurate determination of GT config attributes. This first became important with Cherryview in which the config could differ between devices with the same PCI ID. The kernel detection of these values is device-specific and not included in this patch. Because zero is not a valid value for any of these parameters, a value of zero is interpreted as unknown for the device. Userspace drivers should continue to maintain ID-based tables for older devices not supported by the new query method. For: VIZ-4636 Signed-off-by: Jeff McGee jeff.mc...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 10 ++ include/uapi/drm/i915_drm.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 053e178..9350ea2 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -150,6 +150,16 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_MMAP_VERSION: value = 1; break; + case I915_PARAM_SUBSLICE_TOTAL: + value = INTEL_INFO(dev)-subslice_total; + if (!value) + return -ENODEV; + break; + case I915_PARAM_EU_TOTAL: + value = INTEL_INFO(dev)-eu_total; + if (!value) + return -ENODEV; + break; default: DRM_DEBUG(Unknown parameter %d\n, param-param); return -EINVAL; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 6eed16b..8672efc 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -347,6 +347,8 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29 #define I915_PARAM_MMAP_VERSION 30 #define I915_PARAM_HAS_BSD2 31 +#define I915_PARAM_SUBSLICE_TOTAL 32 +#define I915_PARAM_EU_TOTAL 33 typedef struct drm_i915_getparam { int param; -- 2.3.0 ___ Beignet mailing list beig...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet ___ Beignet mailing list beig...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet ___ Beignet mailing list beig...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Don't assume primary cursor are always on for wm calculation (v3)
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5912 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 282/282 282/282 ILK 308/308 308/308 SNB 307/307 307/307 IVB -1 375/375 374/375 BYT 294/294 294/294 HSW 385/385 385/385 BDW 315/315 315/315 -Detailed- Platform Testdrm-intel-nightly Series Applied *IVB igt_gem_userptr_blits_minor-normal-sync PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Disable DDR DVFS on CHV
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com DDR DVFS introduces massive memory latencies which can't be handled by the PND deadline stuff. Instead the watermarks will need to be programmed to compensate for the latency and the deadlines will need to be programmed to tight fixed values. That means DDR DVFS can only be enabled if the display FIFOs are large enough, and that pretty much means we have to manually repartition them to suit the needs of the moment. That's a lot of change, so in the meantime let's just disable DDR DVFS to get the display(s) to be stable. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Since this patch disabled DDR DVFS in the driver, can this be done once. Here I assume that DDR DVFS gets disabled on each update watermark call. Moreover enabling DDR DVFS should be followed with updating watermarks and display arbiter. Thanks and Regards, Arun R Murthy --- drivers/gpu/drm/i915/i915_reg.h | 5 + drivers/gpu/drm/i915/intel_pm.c | 34 ++ 2 files changed, 39 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a20f58..744d162 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -630,6 +630,11 @@ enum skl_disp_power_wells { #define FB_GFX_FMIN_AT_VMIN_FUSE 0x137 #define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT8 +#define PUNIT_REG_DDR_SETUP2 0x139 +#define FORCE_DDR_FREQ_REQ_ACK (1 8) +#define FORCE_DDR_LOW_FREQ (1 1) +#define FORCE_DDR_HIGH_FREQ (1 0) + #define PUNIT_GPU_STATUS_REG 0xdb #define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16 #define PUNIT_GPU_STATUS_MAX_FREQ_MASK0xff diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1dd82ec..fc03e24 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -235,6 +235,28 @@ static const struct cxsr_latency *intel_get_cxsr_latency(int is_desktop, return NULL; } +static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable) +{ + u32 val; + + mutex_lock(dev_priv-rps.hw_lock); + + val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2); + if (enable) + val = ~FORCE_DDR_HIGH_FREQ; + else + val |= FORCE_DDR_HIGH_FREQ; + val = ~FORCE_DDR_LOW_FREQ; + val |= FORCE_DDR_FREQ_REQ_ACK; + vlv_punit_write(dev_priv, PUNIT_REG_DDR_SETUP2, val); + + if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2) + FORCE_DDR_FREQ_REQ_ACK) == 0, 3)) + DRM_ERROR(timed out waiting for Punit DDR DVFS request\n); + + mutex_unlock(dev_priv-rps.hw_lock); +} + static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable) { u32 val; @@ -282,6 +304,7 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable) enable ? enabled : disabled); } + /* * Latency for FIFO fetches is dependent on several factors: * - memory configuration (speed, channels) @@ -992,6 +1015,17 @@ static void valleyview_update_wm(struct drm_crtc *crtc) wm.pipe[pipe].primary, wm.pipe[pipe].cursor, wm.sr.plane, wm.sr.cursor); + /* +* FIXME DDR DVFS introduces massive memory latencies which +* are not known to system agent so any deadline specified +* by the display may not be respected. To support DDR DVFS +* the watermark code needs to be rewritten to essentially +* bypass deadline mechanism and rely solely on the +* watermarks. For now disable DDR DVFS. +*/ + if (IS_CHERRYVIEW(dev_priv)) + chv_set_memory_dvfs(dev_priv, false); + if (!cxsr_enabled) intel_set_memory_cxsr(dev_priv, false); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 11/12] drm/i915: Enable the maxfifo PM5 mode when appropriate on CHV
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com CHV has a new knob in Punit to select between some memory power savings modes PM2 and PM5. We can allow the deeper PM5 when maxfifo mode is enabled, so let's do so in the hopes for moar power savings. v2: Put the thing into a separate function to avoid churn later v3: Don't break VLV Reviewed-by: Vijay Purushothaman vijay.a.purushotha...@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Reviewed-by: Arun R Murthy arun.r.mur...@intel.com Thanks and Regards, Arun R Murthy --- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
We don't need MAP_FIXED, we just want to avoid address 0 to be allocated. Though I think using MAP_FIXED is overkill, will bring much unnecessary complexity on both kernel and beignet side. I don't mind if people can provide stable MAP_FIXED patches to resolve this problem a few months or years later. At that time, kernel driver can revert the reserve page 0 patch. Before that reserve page 0 can benefit all the Beignet user without breaking anything. As I know, on CPU side, many arches with flexible address space like IA64 have reserved virtual page 0 to address this problem, I don't see why this is non sense. Thanks Zou Nanhai -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Friday, March 06, 2015 4:39 PM To: Zou, Nanhai Cc: Daniel Vetter; Song, Ruiling; Vetter, Daniel; intel-gfx@lists.freedesktop.org; Yang, Rong R; beig...@lists.freedesktop.org; Weinehall, David Subject: Re: [Beignet] [Intel-gfx] Preventing zero GPU virtual address allocation On Fri, Mar 06, 2015 at 02:11:18AM +, Zou, Nanhai wrote: I don't understand why we need a complex solution when there is already a simple solution with patch. What is the drawback of reserving page 0? Before we going to that complex solution, could we just reserve page zero? It is simple and straight forward. Because it is a nonsense ABI constraint. If you want the equivalent of MAP_FIXED, we should give you MAP_FIXED. -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 1/3] drm/i915/skl: Allow universal planes to position
On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote: On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote: Signed-off-by: Sonika Jindal sonika.jin...@intel.com Imo this needs a little more commit message, and more important it needs igt test coverage. Best approach there is probably to take the plane test we have already and extend it to the primary plane. -Daniel This is just to take care of the case when the size of the fb is smaller than the crtc. I have extended the rotation test (yet to be posted), to create a smaller primary plane fb to be used for 90/270 rotation. Since we still set position to 0 for primary plane, I did not add any test case for positioning of primary plane. That can be added as a separate activity when positioning support is added. Right now this is just to allow smaller fb for primary plane which is possible with universal planes gen =9. Regards, Sonika --- drivers/gpu/drm/i915/intel_display.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 437a679..e1b0c4d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12183,16 +12183,21 @@ intel_check_primary_plane(struct drm_plane *plane, struct drm_rect *dest = state-dst; struct drm_rect *src = state-src; const struct drm_rect *clip = state-clip; + bool can_position = false; int ret; crtc = crtc ? crtc : plane-crtc; intel_crtc = to_intel_crtc(crtc); + if (INTEL_INFO(dev)-gen = 9) + can_position = true; + ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, - false, true, state-visible); + can_position, true, + state-visible); if (ret) return ret; -- 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 v2 01/12] drm/i915: Reduce CHV DDL multiplier to 16/8
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Apparently we must yet halve the DDL drain latency from what we're using currently. This little nugget is not in any spec, but came down through the grapevine. This makes the displays a bit more stable. Not quite fully stable but at least they don't fall over immediately on driver load. v2: Update high_precision in valleyview_update_sprite_wm() too (Jesse) Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 8 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ee1964..d8a0205 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4166,6 +4166,7 @@ enum skl_disp_power_wells { #define DSPFW_PLANEA_WM1_HI_MASK(10) /* drain latency register values*/ +#define DRAIN_LATENCY_PRECISION_8 8 #define DRAIN_LATENCY_PRECISION_1616 #define DRAIN_LATENCY_PRECISION_3232 #define DRAIN_LATENCY_PRECISION_6464 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3c64810..efbcfef 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -728,8 +728,8 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc, entries = DIV_ROUND_UP(clock, 1000) * pixel_size; if (IS_CHERRYVIEW(dev)) - *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_32 : - DRAIN_LATENCY_PRECISION_16; + *prec_mult = (entries 32) ? DRAIN_LATENCY_PRECISION_16 : + DRAIN_LATENCY_PRECISION_8; As per the spec the lower precision is 16 and not 8. With this calculated DDL we see some flickers and hence as a temporary solution we further divide the DDL by 2. else *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32; @@ -759,7 +759,7 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc) enum pipe pipe = intel_crtc-pipe; int plane_prec, prec_mult, plane_dl; const int high_precision = IS_CHERRYVIEW(dev) ? - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_64; + DRAIN_LATENCY_PRECISION_16 : DRAIN_LATENCY_PRECISION_64; The higher precision as per the spec is 32. With this calculated DDL we see some flickers and hence as a temporary solution we further divide the DDL by 2. plane_dl = I915_READ(VLV_DDL(pipe)) ~(DDL_PLANE_PRECISION_HIGH | DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH | @@ -958,7 +958,7 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane, int sprite_dl; int prec_mult; const int high_precision = IS_CHERRYVIEW(dev) ? - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_64; + DRAIN_LATENCY_PRECISION_16 : DRAIN_LATENCY_PRECISION_64; sprite_dl = I915_READ(VLV_DDL(pipe)) ~(DDL_SPRITE_PRECISION_HIGH(sprite) | (DRAIN_LATENCY_MASK DDL_SPRITE_SHIFT(sprite))); Thanks and Regards, Arun R Murthy --- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/12] drm/i915: Kill DRAIN_LATENCY_PRECISION_* defines
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Kill the silly DRAIN_LATENCY_PRECISION_* defines and just use the raw number instead. v2: Move the sprite 32/16 - 16/8 preision multiplier change to another patch (Jesse) Can this be merged with the previous patch? Also the comments on the patch [PATCH v2 01/12] drm/i915: Reduce CHV DDL multiplier to 16/8 holds good here also. Thanks and Regards, Arun R Murthy --- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/12] drm/i915: Hide VLV DDL precision handling
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Move the DDL precision handling into vlv_compute_drain_latency() so the callers don't have to duplicate the same code to deal with it. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Reviewed-by: Arun R Murthy arun.r.mur...@intel.com Thanks and Regards, Arun R Murthy --- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx