Re: [Intel-gfx] [PATCH v4 2/6] drm/i915: tidy up initialisation failure paths (GEM & LRC)
On Mon, Feb 01, 2016 at 09:45:25AM +, Dave Gordon wrote: > On 30/01/16 11:28, Chris Wilson wrote: > >On Sat, Jan 30, 2016 at 10:56:27AM +, Chris Wilson wrote: > >>On Fri, Jan 29, 2016 at 07:19:27PM +, Dave Gordon wrote: > >>>1. add call to i915_gem_context_fini() to deallocate the default > >>>context(s) if the call to init_rings() fails, so that we don't > >>>leak the context in that situation. > >>> > >>>2. remove useless code in intel_logical_ring_cleanup(), presumably > >>>copypasted from legacy ringbuffer version at creation. > >>> If your commit message has a list of independent changes ... > >>>Signed-off-by: Dave Gordon> >>>--- > >>> drivers/gpu/drm/i915/i915_gem.c | 5 - > >>> drivers/gpu/drm/i915/intel_lrc.c | 10 ++ > >>> 2 files changed, 6 insertions(+), 9 deletions(-) > >>> > >>>diff --git a/drivers/gpu/drm/i915/i915_gem.c > >>>b/drivers/gpu/drm/i915/i915_gem.c > >>>index a928823..5a4d468 100644 > >>>--- a/drivers/gpu/drm/i915/i915_gem.c > >>>+++ b/drivers/gpu/drm/i915/i915_gem.c > >>>@@ -4986,8 +4986,11 @@ int i915_gem_init(struct drm_device *dev) > >>> goto out_unlock; > >>> > >>> ret = dev_priv->gt.init_rings(dev); > >>>- if (ret) > >>>+ if (ret) { > >>>+ i915_gem_context_fini(dev); > >>>+ /* XXX: anything else to be undone here? */ > >> > >>Yes. Make this a separate patch and begin the onion unwind. > > > >Hmm. Actually, we have to make sure that we can still modeset if this > >function fails - that is anything but utter catastrophe should just > >result in loss of functionality (no stolen, no GEM execution etc) but we > >can still drive the displays so the user can see how bad the damage is. > >-Chris > > Yes, Mika said that's why (he thought) there wasn't a complete reversal of > everything the driver has done up to this point. > > The addition of the context_fini() seems reasonable, that's going to make it > leak a bit less, while still leaving basic framebuffer working. > > Could be a separate patch if you like, but hardly seems worth splitting from > the other chunk, which after all only replaces unreachable code with a > WARNing. ... it should be split. As a rule of thumb at least. I don't really care all that much here though, so jut for the future. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [IGT PATCH 5/6] kms_force_connector_basic: Add force-load-detect test
On Mon, Feb 01, 2016 at 02:44:01PM +0100, Maarten Lankhorst wrote: > Signed-off-by: Maarten Lankhorst> --- > diff --git a/tests/kms_force_connector_basic.c > b/tests/kms_force_connector_basic.c > index bd80caeffd82..f827d0008f7b 100644 > --- a/tests/kms_force_connector_basic.c > +++ b/tests/kms_force_connector_basic.c > @@ -52,6 +52,8 @@ static void reset_connectors(void) > > drmModeFreeConnector(connector); > } > + > + igt_set_module_param_int("load_detect_test", 0); > } > > static int opt_handler(int opt, int opt_index, void *data) > @@ -108,6 +110,17 @@ int main(int argc, char **argv) > igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED); > } > > + igt_subtest("force-load-detect") { > + igt_set_module_param_int("load_detect_test", 1); Ah here's the testcase. We need to unset this module parameter again at the end of the testcase - atm the magic exithandler stuff only works on binary exit, not when leaving a subtests (for things set within a subtest). With that done Reviewed-by: Daniel Vetter We should probably fix that issue in the exit handler code, but that's another task really. -Daniel > + > + temp = drmModeGetConnectorCurrent(drm_fd, > + vga_connector->connector_id); > + > + igt_assert(temp->connection != DRM_MODE_UNKNOWNCONNECTION); > + > + drmModeFreeConnector(temp); > + } > + > igt_subtest("force-connector-state") { > igt_display_t display; > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 0/6] Check pixel clock when setting mode
On Tue, Feb 02, 2016 at 03:16:37PM +0200, Mika Kahola wrote: > From EDID we can read and request higher pixel clock than > our HW can support. This set of patches add checks if > requested pixel clock is lower than the one supported by the HW. > The requested mode is discarded if we cannot support the requested > pixel clock. For example for Cherryview > > 'cvt 2560 1600 60' gives > > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz > Modeline "2560x1600_60.00" 348.50 2560 2760 3032 3504 1600 1603 1609 1658 > -hsync +vsync > > where pixel clock 348.50 MHz is higher than the supported 304 MHz. > > The missing mode validity checks for DisplayPort, HDMI, DP-MST, SDVO, CRT, > and TV. > > V2: > - The maximum DOT clock frequency is added to debugfs i915_frequency_info. > - max dotclock cached in dev_priv structure > - moved computation of max dotclock to 'intel_display.c' > > V3: > - intel_update_max_dotclk() renamed as intel_compute_max_dotclk() > - for GEN9 and above the max dotclock frequency is equal to CD clock > frequency > - for older generations the dot clock frequency is limited to 90% of the > CD clock frequency > - For Cherryview the dot clock is limited to 95% of CD clock frequency > - for GEN2/3 the maximum dot clock frequency is limited to 90% of the > 2X CD clock frequency as we have on option to use double wide mode > - cleanup > > V4: > - renaming of max_dotclk as max_dotclk_freq in dev_priv (i915_drv.h) > caused changes to all patches in my series even though some of them has > been r-b'd by Ville > - for consistency the max_pixclk variable is renamed as max_dotclk throughout > the whole series > > Mika Kahola (6): > drm/i915: DisplayPort pixel clock check > drm/i915: HDMI pixel clock check > drm/i915: DisplayPort-MST pixel clock check > drm/i915: SDVO pixel clock check > drm/i915: CRT pixel clock check > drm/i915: TV pixel clock check Note that the title of your series is wrong I think - mode_valid is _only_ called at probe time, to filter the list of modes userspace can see. Userspace can still try to set a mode not in that list which would be above that dotclk. From a quick look around I don't see that addressed anywhere. If that's true then I think we also need an igt which exercises this code by trying to set a dotclock that's way too high (but for a resolution that the display actually supports). -Daniel > > drivers/gpu/drm/i915/intel_crt.c| 4 > drivers/gpu/drm/i915/intel_dp.c | 3 ++- > drivers/gpu/drm/i915/intel_dp_mst.c | 5 + > drivers/gpu/drm/i915/intel_hdmi.c | 8 > drivers/gpu/drm/i915/intel_sdvo.c | 4 > drivers/gpu/drm/i915/intel_tv.c | 4 > 6 files changed, 27 insertions(+), 1 deletion(-) > > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4
Daniel, I've already tried Ville's patch you've mentioned with no luck. Kindly find unpatched v4.5-rc3 dmesg with drm debug enabled here: [1] [1] https://gist.github.com/efb44b7c6bc325978b80 11.02.2016 10:21, Daniel Vetter wrote: Please boot with drm.debug=0xe and attach the dmesg. Also please test the patch later on in this thread: https://lkml.org/lkml/2016/2/9/587 Ville is working on a proper fix for this specific case. But could be that you're hitting yet another issue. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v2 4/6] drm/i915/skl+: Use scaling amount for plane data rate calculation
On Wed, Feb 10, 2016 at 11:39:41AM -0800, Matt Roper wrote: > On Wed, Jan 27, 2016 at 09:40:01PM +0530, Shobhit Kumar wrote: > > From: "Kumar, Mahesh"> > > > if downscaling is enabled plane data rate increases according to scaling > > amount. take scaling amount under consideration while calculating plane > > data rate > > > > v2: Address Matt's comments, where data rate was overridden because of > > missing else. > > > > Cc: matthew.d.ro...@intel.com > > Signed-off-by: Kumar, Mahesh > > --- > > drivers/gpu/drm/i915/intel_pm.c | 17 - > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 40fff09..a9f9396 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2912,6 +2912,8 @@ skl_plane_relative_data_rate(const struct > > intel_crtc_state *cstate, > > { > > struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate); > > struct drm_framebuffer *fb = pstate->fb; > > + struct intel_plane *intel_plane = to_intel_plane(pstate->plane); > > + uint32_t down_scale_amount, data_rate; > > uint32_t width = 0, height = 0; > > > > width = drm_rect_width(_pstate->src) >> 16; > > @@ -2923,15 +2925,20 @@ skl_plane_relative_data_rate(const struct > > intel_crtc_state *cstate, > > /* for planar format */ > > if (fb->pixel_format == DRM_FORMAT_NV12) { > > if (y) /* y-plane data rate */ > > - return width * height * > > + data_rate = width * height * > > drm_format_plane_cpp(fb->pixel_format, 0); > > else/* uv-plane data rate */ > > - return (width / 2) * (height / 2) * > > + data_rate = (width / 2) * (height / 2) * > > drm_format_plane_cpp(fb->pixel_format, 1); > > - } > > + } else > > + /* for packed formats */ > > + data_rate = width * height * > > + drm_format_plane_cpp(fb->pixel_format, 0); > > According to the coding style, I believe we're supposed to use braces > for both branches if either one of them needs braces. Yes, and definitely when the other branch spans more than 1 line. -Daniel > > Aside from that, > > Reviewed-by: Matt Roper > > > + > > + down_scale_amount = skl_plane_downscale_amount(intel_plane); > > + > > + return DIV_ROUND_UP((data_rate * down_scale_amount), 1000); > > > > - /* for packed formats */ > > - return width * height * drm_format_plane_cpp(fb->pixel_format, 0); > > } > > > > /* > > -- > > 2.5.0 > > > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gen9: Check for DC state mismatch
The DMC can incorrectly run off and allow DC states on it's own. We don't know the root-cause for this yet but this patch makes it more visible. Signed-off-by: Patrik Jakobsson--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_csr.c| 2 ++ drivers/gpu/drm/i915/intel_runtime_pm.c | 8 3 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e11eef1..7e33454 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -746,6 +746,7 @@ struct intel_csr { uint32_t mmio_count; i915_reg_t mmioaddr[8]; uint32_t mmiodata[8]; + uint32_t dc_state; }; #define DEV_INFO_FOR_EACH_FLAG(func, sep) \ diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 2a7ec31..b453fcc 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -243,6 +243,8 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv) I915_WRITE(dev_priv->csr.mmioaddr[i], dev_priv->csr.mmiodata[i]); } + + dev_priv->csr.dc_state = 0; } static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index bbca527..e79674b 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -494,10 +494,18 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state) val = I915_READ(DC_STATE_EN); DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n", val & mask, state); + + /* Check if DMC is ignoring our DC state requests */ + if ((val & mask) != dev_priv->csr.dc_state) + DRM_ERROR("DC state mismatch (0x%x -> 0x%x)\n", + dev_priv->csr.dc_state, val & mask); + val &= ~mask; val |= state; I915_WRITE(DC_STATE_EN, val); POSTING_READ(DC_STATE_EN); + + dev_priv->csr.dc_state = val & mask; } void bxt_enable_dc9(struct drm_i915_private *dev_priv) -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V3] drm/i915/skl: SKL CDCLK change on modeset tracking VCO
On Wed, Feb 10, 2016 at 05:37:20PM -0800, Marc Herbert wrote: > On 10/02/16 06:27, Ville Syrjälä wrote: > > On Tue, Feb 09, 2016 at 04:28:27PM -0800, clinton.a.tay...@intel.com wrote: > >> From: Clint Taylor> >> > >> Track VCO frequency of SKL instead of the boot CDCLK and allow modeset > >> to set cdclk based on the max required pixel clock based on VCO > >> selected. > >> > >> The vco should be tracked at the atomic level and all CRTCs updated if > >> the required vco is changed. At this time the eDP pll is configured > >> inside the encoder which has no visibility into the atomic state. > > > > Yes it does. The passed in pipe_config is the crtc's state. And > > if you want to store the thing in the top level atomic state you > > just dig up that up from the crtc state: > > to_intel_atomic_state(pipe_config->base.state) > > or something along those lines). > > Hi, I'm writing this message with Clint. We understand the following: > > - This V3 patch as it is now is fixing many use cases for hardware that has > been > on the shelves for months. > > - It does not fix all use cases with future eDP 1.4 panels. > Example: boot with external monitor and eDP 1.4 lid closed; then lid is > opened and causes a VCO change. However such cases are not supported yet > anyway! > If this happens today, the CD clock will move to some uncontrolled value > and the pipelines will NOT be reprogrammed. So this patch does *not > regress* any > use case - not even future use cases. > > - Present and future use cases can be addressed in two consecutive patches: > first > this V3 patch now; then atomic modeset VCO tracking later. This won't > increase > development complexity in any way. In other words, the second patch will > almost not > change the code of the first patch. > > - From a validation perspective this first patch can be validated today with > today's > hardware. The second patch cannot be validated yet because eDP 1.4 hardware > is not readily available yet. > > - Getting atomic modeset VCO tracking right will take additional development > time. > And... did I mention validation already? > > - This first patch is fixing many use cases for hardware that has been > on the shelves for months, including failures to reach max resolution. > > So - you saw me coming from a distance - can we agree on splitting this work > in two separate patches so we can merge this first V3 patch now? Products have > been needing this for months, thanks! Atomic is the new world, we need this thing to be atomic. Please work together with Mika Kahola (who's done all the dynamic cdclk infrastucture together with Ville). Wrt validation: That's what we have testcases and CI for, plus in-kernel consistency checks of atomic state. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Use atomic state to obtain load detection crtc.
On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote: > Instead of restoring dpms and a flag for whether a temp fb is allocated > duplicate > the old plane_state and crtc_state, and restore the members we potentially > touched. > > Signed-off-by: Maarten LankhorstDo we have the load-detect igt testcase now in the BAT set? We've broken this way too many times to not do that. Imo that should be done before merging this fix. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 128 > --- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 2 files changed, 76 insertions(+), 56 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 4d8c9f7857db..0702ce8ec36a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, > if (obj->base.size < mode->vdisplay * fb->pitches[0]) > return NULL; > > + drm_framebuffer_reference(fb); > return fb; > #else > return NULL; > @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector > *connector, > encoder->base.id, encoder->name); > > retry: > + old->old_pipe_config = NULL; > + old->old_plane_state = NULL; > + > ret = drm_modeset_lock(>connection_mutex, ctx); > if (ret) > goto fail; > @@ -10441,24 +10445,15 @@ retry: >*/ > > /* See if we already have a CRTC for this connector */ > - if (encoder->crtc) { > - crtc = encoder->crtc; > + if (connector->state->crtc) { > + crtc = connector->state->crtc; > > ret = drm_modeset_lock(>mutex, ctx); > if (ret) > goto fail; > - ret = drm_modeset_lock(>primary->mutex, ctx); > - if (ret) > - goto fail; > - > - old->dpms_mode = connector->dpms; > - old->load_detect_temp = false; > > /* Make sure the crtc and connector are running */ > - if (connector->dpms != DRM_MODE_DPMS_ON) > - connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); > - > - return true; > + goto found; > } > > /* Find an unused one (if possible) */ > @@ -10466,8 +10461,15 @@ retry: > i++; > if (!(encoder->possible_crtcs & (1 << i))) > continue; > - if (possible_crtc->state->enable) > + > + ret = drm_modeset_lock(_crtc->mutex, ctx); > + if (ret) > + goto fail; > + > + if (possible_crtc->state->enable) { > + drm_modeset_unlock(_crtc->mutex); > continue; > + } > > crtc = possible_crtc; > break; > @@ -10481,17 +10483,19 @@ retry: > goto fail; > } > > - ret = drm_modeset_lock(>mutex, ctx); > - if (ret) > - goto fail; > +found: > + intel_crtc = to_intel_crtc(crtc); > + > ret = drm_modeset_lock(>primary->mutex, ctx); > if (ret) > goto fail; > > - intel_crtc = to_intel_crtc(crtc); > - old->dpms_mode = connector->dpms; > - old->load_detect_temp = true; > - old->release_fb = NULL; > + old->old_pipe_config = intel_crtc_duplicate_state(crtc); > + old->old_plane_state = intel_plane_duplicate_state(crtc->primary); > + if (!old->old_pipe_config || !old->old_plane_state) { > + ret = -ENOMEM; > + goto fail; > + } > > state = drm_atomic_state_alloc(dev); > if (!state) > @@ -10505,7 +10509,9 @@ retry: > goto fail; > } > > - connector_state->crtc = crtc; > + ret = drm_atomic_set_crtc_for_connector(connector_state, crtc); > + if (ret) > + goto fail; > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > if (IS_ERR(crtc_state)) { > @@ -10529,7 +10535,6 @@ retry: > if (fb == NULL) { > DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); > fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); > - old->release_fb = fb; > } else > DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); > if (IS_ERR(fb)) { > @@ -10541,15 +10546,16 @@ retry: > if (ret) > goto fail; > > - drm_mode_copy(_state->base.mode, mode); > + drm_framebuffer_unreference(fb); > + > + ret = drm_atomic_set_mode_for_crtc(_state->base, mode); > + if (ret) > + goto fail; > > if (drm_atomic_commit(state)) { > DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); > - if (old->release_fb) > -
Re: [Intel-gfx] [PATCH 2/3] tests/kms_hdmi_inject: Test to make use of HDMI injection capabilities.
On Tue, Feb 02, 2016 at 03:03:37PM +0200, Marius Vlad wrote: > Signed-off-by: Marius Vlad> --- > tests/Makefile.sources | 1 + > tests/kms_hdmi_inject.c | 165 > > 2 files changed, 166 insertions(+) > create mode 100644 tests/kms_hdmi_inject.c > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index d431ebf..13c2552 100644 Some of this should be bat tests I think, and then probably in kms_force_connector_basic. -Daniel > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -170,6 +170,7 @@ TESTS_progs = \ > gen3_render_tiledy_blits \ > gen7_forcewake_mt \ > kms_3d \ > + kms_hdmi_inject \ > kms_fence_pin_leak \ > kms_force_connector_basic \ > kms_pwrite_crc \ > diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c > new file mode 100644 > index 000..8f75116 > --- /dev/null > +++ b/tests/kms_hdmi_inject.c > @@ -0,0 +1,165 @@ > +/* > + * Copyright © 2016 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 "igt.h" > + > +#define HDISPLAY_4K 3840 > +#define VDISPLAY_4K 2160 > + > +IGT_TEST_DESCRIPTION("Tests 4K and audio HDMI injection."); > + > +static drmModeConnector * > +get_connector(int drm_fd, drmModeRes *res) > +{ > + int i; > + drmModeConnector *connector; > + > + for (i = 0; i < res->count_connectors; i++) { > + > + connector = > + drmModeGetConnectorCurrent(drm_fd, res->connectors[i]); > + > + if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA && > + connector->connection == DRM_MODE_DISCONNECTED) > + break; > + > + drmModeFreeConnector(connector); > + connector = NULL; > + } > + > + return connector; > +} > + > +static void > +hdmi_inject_4k(int drm_fd, drmModeConnector *connector) > +{ > + unsigned char *edid; > + size_t length; > + struct kmstest_connector_config config; > + int ret, cid, i, crtc_mask = -1; > + int fb_id; > + struct igt_fb fb; > + uint8_t found_4k_mode = 0; > + uint32_t devid; > + > + devid = intel_get_drm_devid(drm_fd); > + > + /* 4K requires at least HSW */ > + igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid)); > + > + kmstest_edid_add_4k(igt_kms_get_base_edid(), EDID_LENGTH, , > + ); > + > + kmstest_force_edid(drm_fd, connector, edid, length); > + > + if (!kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_ON)) > + igt_skip("Could not force connector on\n"); > + > + cid = connector->connector_id; > + > + connector = drmModeGetConnectorCurrent(drm_fd, cid); > + > + for (i = 0; i < connector->count_modes; i++) { > + if (connector->modes[i].hdisplay == HDISPLAY_4K && > + connector->modes[i].vdisplay == VDISPLAY_4K) { > + found_4k_mode++; > + break; > + } > + } > + > + igt_assert(found_4k_mode); > + > + /* create a configuration */ > + ret = kmstest_get_connector_config(drm_fd, cid, crtc_mask, ); > + igt_assert(ret); > + > + igt_info(" "); > + kmstest_dump_mode(>modes[i]); > + > + /* create framebuffer */ > + fb_id = igt_create_fb(drm_fd, connector->modes[i].hdisplay, > + connector->modes[i].vdisplay, > + DRM_FORMAT_XRGB, > + LOCAL_DRM_FORMAT_MOD_NONE, ); > + > + ret = drmModeSetCrtc(drm_fd, config.crtc->crtc_id, fb_id, 0, 0, > + >connector_id, 1, > + >modes[i]); > + > + igt_assert(ret == 0); > + > + igt_remove_fb(drm_fd, ); > + > + kmstest_force_connector(drm_fd,
Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4
On Thu, Feb 11, 2016 at 10:54:08AM +0200, Oleksandr Natalenko wrote: > Daniel, > > I've already tried Ville's patch you've mentioned with no luck. > > Kindly find unpatched v4.5-rc3 dmesg with drm debug enabled here: [1] > > [1] https://gist.github.com/efb44b7c6bc325978b80 That's an IVB. So no wonder my patch doesn't help. Can you grab another dmesg after disconnecting and reconnecting the HDMI cable? > > 11.02.2016 10:21, Daniel Vetter wrote: > > Please boot with drm.debug=0xe and attach the dmesg. Also please test > > the > > patch later on in this thread: > > > > https://lkml.org/lkml/2016/2/9/587 > > > > Ville is working on a proper fix for this specific case. But could be > > that > > you're hitting yet another issue. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4
On Thu, Feb 11, 2016 at 09:45:17AM +0200, Oleksandr Natalenko wrote: > Daniel, > > I do confirm that this hacky patch: > > https://lkml.org/lkml/2016/1/19/637 > > works around my issue. I understand that this is improper fix, so let me > know how could I debug my issue further. Please boot with drm.debug=0xe and attach the dmesg. Also please test the patch later on in this thread: https://lkml.org/lkml/2016/2/9/587 Ville is working on a proper fix for this specific case. But could be that you're hitting yet another issue. Thanks, Daniel > > Thanks. > > 09.02.2016 12:11, Daniel Vetter wrote: > >Can you please retest with latest -rc? There's been some bugs in the HDMI > >detection changes, which should be fixed now. > > > >If that doesn't help please try to bisect which exact change caused the > >regression. > > > >Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 0/6] Check pixel clock when setting mode
On Tue, Feb 02, 2016 at 06:25:39PM +0200, Ville Syrjälä wrote: > On Tue, Feb 02, 2016 at 03:16:37PM +0200, Mika Kahola wrote: > > From EDID we can read and request higher pixel clock than > > our HW can support. This set of patches add checks if > > requested pixel clock is lower than the one supported by the HW. > > The requested mode is discarded if we cannot support the requested > > pixel clock. For example for Cherryview > > > > 'cvt 2560 1600 60' gives > > > > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz > > Modeline "2560x1600_60.00" 348.50 2560 2760 3032 3504 1600 1603 1609 > > 1658 -hsync +vsync > > > > where pixel clock 348.50 MHz is higher than the supported 304 MHz. > > > > The missing mode validity checks for DisplayPort, HDMI, DP-MST, SDVO, CRT, > > and TV. > > > > V2: > > - The maximum DOT clock frequency is added to debugfs i915_frequency_info. > > - max dotclock cached in dev_priv structure > > - moved computation of max dotclock to 'intel_display.c' > > > > V3: > > - intel_update_max_dotclk() renamed as intel_compute_max_dotclk() > > - for GEN9 and above the max dotclock frequency is equal to CD clock > > frequency > > - for older generations the dot clock frequency is limited to 90% of the > > CD clock frequency > > - For Cherryview the dot clock is limited to 95% of CD clock frequency > > - for GEN2/3 the maximum dot clock frequency is limited to 90% of the > > 2X CD clock frequency as we have on option to use double wide mode > > - cleanup > > > > V4: > > - renaming of max_dotclk as max_dotclk_freq in dev_priv (i915_drv.h) > > caused changes to all patches in my series even though some of them has > > been r-b'd by Ville > > - for consistency the max_pixclk variable is renamed as max_dotclk > > throughout > > the whole series > > > > Mika Kahola (6): > > drm/i915: DisplayPort pixel clock check > > drm/i915: HDMI pixel clock check > > drm/i915: DisplayPort-MST pixel clock check > > drm/i915: SDVO pixel clock check > > drm/i915: CRT pixel clock check > > drm/i915: TV pixel clock check > > I think I've r-b'd these in the past, but just in case I had another > look and it still looks OK to me. So, for the series: > Reviewed-by: Ville Syrjälä> > Now, apparently there are some actual bugs out there that could be > fixed by this. Eg: > https://bugzilla.redhat.com/show_bug.cgi?id=1279797 > > We had one bug in fdo too, but somehow that got closed before we added > any of the relevant checks. I think it was this one: > https://bugs.freedesktop.org/show_bug.cgi?id=85621 > > I'd be tempted to put cc:stable on this stuff actually, to get those > bugs fixed. Looks max_dotclock_freq got merged in 4.4 so we can't > backport beyond that. Hm, I was less risky, so pushed them all to dinq without cc: something. We'll see how it goes, and can cherry-pick later on. -Daniel > > > > > drivers/gpu/drm/i915/intel_crt.c| 4 > > drivers/gpu/drm/i915/intel_dp.c | 3 ++- > > drivers/gpu/drm/i915/intel_dp_mst.c | 5 + > > drivers/gpu/drm/i915/intel_hdmi.c | 8 > > drivers/gpu/drm/i915/intel_sdvo.c | 4 > > drivers/gpu/drm/i915/intel_tv.c | 4 > > 6 files changed, 27 insertions(+), 1 deletion(-) > > > > -- > > 1.9.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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Support HDMI EDID injection
On Tue, Feb 02, 2016 at 01:05:07PM +0200, Marius Vlad wrote: > Use the drm_property_blob data for EDID when an blob EDID > has been supplied over the debugfs interface. > > Signed-off-by: Marius Vladv2 of patches must have an in-patch changelog of what (and why) stuff changed. -Daniel > --- > drivers/gpu/drm/i915/intel_hdmi.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 8698a64..241305b 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1336,7 +1336,9 @@ intel_hdmi_unset_edid(struct drm_connector *connector) > intel_hdmi->has_audio = false; > intel_hdmi->rgb_quant_range_selectable = false; > > - kfree(to_intel_connector(connector)->detect_edid); > + /* only free if we haven't injected EDID */ > + if (!connector->override_edid) > + kfree(to_intel_connector(connector)->detect_edid); > to_intel_connector(connector)->detect_edid = NULL; > } > > @@ -1355,6 +1357,10 @@ intel_hdmi_set_edid(struct drm_connector *connector, > bool force) > intel_gmbus_get_adapter(dev_priv, > intel_hdmi->ddc_bus)); > > + /* for injected EDID */ > + if (!edid && connector->override_edid) > + edid = (struct edid *) connector->edid_blob_ptr->data; > + > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); > } > > -- > 2.5.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Support HDMI EDID injection
On Mon, Feb 01, 2016 at 06:35:33PM +0200, Marius Vlad wrote: > Allow HDMI EDID injection by making a copy of edid_blob_ptr. When > disconnecting > the connector, or forcing a disconnect, the copy will free'd by > intel_hdmi_unset_edid(). > > Signed-off-by: Marius Vlad> --- > drivers/gpu/drm/i915/intel_hdmi.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 8698a64..4725e8d1 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1355,6 +1355,16 @@ intel_hdmi_set_edid(struct drm_connector *connector, > bool force) > intel_gmbus_get_adapter(dev_priv, > intel_hdmi->ddc_bus)); > > + /* for injected EDIDs */ > + if (!edid && connector->override_edid) { > + edid = kzalloc(connector->edid_blob_ptr->length, > + GFP_KERNEL); > + if (edid) { > + memcpy(edid, connector->edid_blob_ptr->data, > + > connector->edid_blob_ptr->length); > + } > + } > + You're sure this does anything? When forcing the edid, ->detect isn't called. And the problem with hdmi injection isn't the edid, but that our audio detection code is wrong. See how has_audio is set in intel_hdmi_set_edid is handled. Similarly we need to fixup the eld code to correctly update drm_connector->eld. Finally this needs a testcase which: 1. injects hdmi edid without audio, then sets mode. 2. injects hdmi edid with audio, then sets mode. 3. injects hdmi edid without audio, then sets mode. Just to exercise the code paths. Bonus points if you check through alsa that the hdmi audio state indeed matches our expectations (compare the eld you can grab from alsa with what you injected to make sure you look at the right screen). But given all our other oustanding troubles around CI I'd suggest we deprioritize this task for now. Please add this comment to JIRA so it wont' get lost. -Daniel > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); > } > > -- > 2.5.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] dmr/i915: Use appropriate spinlock flavour
On Mon, Feb 01, 2016 at 11:00:07AM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > We know this never runs from interrupt context so > don't need to use the flags variant. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_gem.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index a928823507c5..faa9def96917 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2970,11 +2970,9 @@ i915_gem_retire_requests(struct drm_device *dev) > i915_gem_retire_requests_ring(ring); > idle &= list_empty(>request_list); > if (i915.enable_execlists) { > - unsigned long flags; > - > - spin_lock_irqsave(>execlist_lock, flags); > + spin_lock_irq(>execlist_lock); > idle &= list_empty(>execlist_queue); > - spin_unlock_irqrestore(>execlist_lock, flags); > + spin_unlock_irq(>execlist_lock); > > intel_execlists_retire_requests(ring); > } > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix hpd live status bits for g4x
On Thu, 11 Feb 2016, Daniel Vetterwrote: > On Wed, Feb 10, 2016 at 07:59:05PM +0200, ville.syrj...@linux.intel.com wrote: >> From: Ville Syrjälä >> >> Looks like g4x hpd live status bits actually agree with the spec. At >> least they do on the machine I have, and apparently on Nick Bowler's >> g4x as well. >> >> So gm45 may be the only platform where they don't agree. At least >> that seems to be the case based on the (somewhat incomplete) >> logs/dumps in [1], and Daniel has also tested this on his gm45 >> sometime in the past. >> >> So let's change the bits to match the spec on g4x. That actually makes >> the g4x bits identical to vlv/chv so we can just share the code >> between those platforms, leaving gm45 as the special case. >> >> [1] https://bugzilla.kernel.org/show_bug.cgi?id=52361 >> >> Cc: Shashank Sharma >> Cc: Sonika Jindal >> Cc: Daniel Vetter >> Cc: Jani Nikula >> Cc: Nick Bowler >> References: >> https://lists.freedesktop.org/archives/dri-devel/2016-February/100382.html >> Reported-by: Nick Bowler >> Cc: sta...@vger.kernel.org >> Fixes: 237ed86c693d ("drm/i915: Check live status before reading edid") >> Signed-off-by: Ville Syrjälä > > Yeah I'm hopeful this will work. Reviewed-by: Daniel Vetter > > > Since CI is down and this is super restricted impact and fixing a > regression I'm voting that we'll pick it up right away. Jani, are you ok > with that? Ack. > -Daniel > >> --- >> drivers/gpu/drm/i915/i915_reg.h | 15 --- >> drivers/gpu/drm/i915/intel_dp.c | 14 +++--- >> 2 files changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 188ad5de020f..678faa957e75 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -3296,19 +3296,20 @@ enum skl_disp_power_wells { >> >> #define PORT_HOTPLUG_STAT _MMIO(dev_priv->info.display_mmio_offset + >> 0x61114) >> /* >> - * HDMI/DP bits are gen4+ >> + * HDMI/DP bits are g4x+ >> * >> * WARNING: Bspec for hpd status bits on gen4 seems to be completely >> confused. >> * Please check the detailed lore in the commit message for for experimental >> * evidence. >> */ >> -#define PORTD_HOTPLUG_LIVE_STATUS_G4X (1 << 29) >> +/* Bspec says GM45 should match G4X/VLV/CHV, but reality disagrees */ >> +#define PORTD_HOTPLUG_LIVE_STATUS_GM45(1 << 29) >> +#define PORTC_HOTPLUG_LIVE_STATUS_GM45(1 << 28) >> +#define PORTB_HOTPLUG_LIVE_STATUS_GM45(1 << 27) >> +/* G4X/VLV/CHV DP/HDMI bits again match Bspec */ >> +#define PORTD_HOTPLUG_LIVE_STATUS_G4X (1 << 27) >> #define PORTC_HOTPLUG_LIVE_STATUS_G4X (1 << 28) >> -#define PORTB_HOTPLUG_LIVE_STATUS_G4X (1 << 27) >> -/* VLV DP/HDMI bits again match Bspec */ >> -#define PORTD_HOTPLUG_LIVE_STATUS_VLV (1 << 27) >> -#define PORTC_HOTPLUG_LIVE_STATUS_VLV (1 << 28) >> -#define PORTB_HOTPLUG_LIVE_STATUS_VLV (1 << 29) >> +#define PORTB_HOTPLUG_LIVE_STATUS_G4X (1 << 29) >> #define PORTD_HOTPLUG_INT_STATUS (3 << 21) >> #define PORTD_HOTPLUG_INT_LONG_PULSE (2 << 21) >> #define PORTD_HOTPLUG_INT_SHORT_PULSE (1 << 21) >> diff --git a/drivers/gpu/drm/i915/intel_dp.c >> b/drivers/gpu/drm/i915/intel_dp.c >> index a073f04a5330..bbe18996efe6 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4490,20 +4490,20 @@ static bool g4x_digital_port_connected(struct >> drm_i915_private *dev_priv, >> return I915_READ(PORT_HOTPLUG_STAT) & bit; >> } >> >> -static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *port) >> +static bool gm45_digital_port_connected(struct drm_i915_private *dev_priv, >> +struct intel_digital_port *port) >> { >> u32 bit; >> >> switch (port->port) { >> case PORT_B: >> -bit = PORTB_HOTPLUG_LIVE_STATUS_VLV; >> +bit = PORTB_HOTPLUG_LIVE_STATUS_GM45; >> break; >> case PORT_C: >> -bit = PORTC_HOTPLUG_LIVE_STATUS_VLV; >> +bit = PORTC_HOTPLUG_LIVE_STATUS_GM45; >> break; >> case PORT_D: >> -bit = PORTD_HOTPLUG_LIVE_STATUS_VLV; >> +bit = PORTD_HOTPLUG_LIVE_STATUS_GM45; >> break; >> default: >> MISSING_CASE(port->port); >> @@ -4555,8 +4555,8 @@ bool intel_digital_port_connected(struct >> drm_i915_private *dev_priv, >> return cpt_digital_port_connected(dev_priv, port); >> else if (IS_BROXTON(dev_priv)) >>
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave - Intel fixes all around, mostly display. Sorry I'm late this week. BR, Jani. The following changes since commit 388f7b1d6e8ca06762e2454d28d6c3c55ad0fe95: Linux 4.5-rc3 (2016-02-07 15:38:30 -0800) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2016-02-12 for you to fetch changes up to ed3f9fd1e865975ceefdb2a43b453e090b1fd787: drm/i915: fix error path in intel_setup_gmbus() (2016-02-10 18:11:34 +0200) Chris Wilson (1): drm/i915: Allow i915_gem_object_get_page() on userptr as well Jani Nikula (5): drm/i915/dsi: defend gpio table against out of bounds access drm/i915/dsi: don't pass arbitrary data to sideband drm/i915/dsi: skip gpio element execution when not supported drm/i915/dp: abstract training pattern selection drm/i915/dp: reduce missing TPS3 support errors to debug logging Lyude (2): drm/i915/skl: Don't skip mst encoders in skl_ddi_pll_select() drm/i915/skl: Fix typo in DPLL_CFGCR1 definition Matt Roper (2): drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3) drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2) Rasmus Villemoes (1): drm/i915: fix error path in intel_setup_gmbus() drivers/gpu/drm/i915/i915_drv.h | 4 +++ drivers/gpu/drm/i915/i915_gem.c | 3 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 3 +- drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/i915_suspend.c | 4 +-- drivers/gpu/drm/i915/intel_ddi.c | 3 +- drivers/gpu/drm/i915/intel_dp_link_training.c | 45 +++ drivers/gpu/drm/i915/intel_dsi_panel_vbt.c| 21 - drivers/gpu/drm/i915/intel_i2c.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 14 ++--- 10 files changed, 75 insertions(+), 26 deletions(-) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4
Ville, I've applied patch you've provided and did couple of replugging with intel_reg in between. Here are the results. I used additional VGA cable to see what actually I type in console :). Both HDMI and VGA cables plugged: [1] Both HDMI and VGA cables unplugged: [2] Only HDMI cable plugged: [3] Only VGA cable plugged: [4] And here goes dmesg with all the stuff logged: [5] Hope this helps. [1] https://gist.github.com/58a0eb50dcf84e104555 [2] https://gist.github.com/7e8749a3e2cc58ea8aac [3] https://gist.github.com/9d76930da7380634b845 [4] https://gist.github.com/c0d2e2f64242ad4f01f2 [5] https://gist.github.com/fda3b9fed3ca4d31cd20 11.02.2016 16:01, Ville Syrjälä wrote: OK, so the hpd interrupt does happen, and yet the live status supposedly claims that nothing is there. Port C live status definitely works here on my IVB, so not sure what the deal is. Can you grab intel-gpu-tools and run intel_reg read 0xc4000 0xc4004 0xc4008 0xc400c 0xc4030 a couple of times after plugging the monitor in, and also run it when nothing is plugged in. Also you could try something like the following patch so we might observe the live status with a bit more detail. Though the fact that it doesn't seem to work for you even when the monitor was already plugged in is somewhat troubling: --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1392,12 +1392,17 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - for (try = 0; !live_status && try < 9; try++) { - if (try) - msleep(10); - live_status = intel_digital_port_connected(dev_priv, + printk("port %c live status\n ", port_name(hdmi_to_dig_port(intel_hdmi)->port)); + for (try = 0; try < 250; try++) { + bool status = intel_digital_port_connected(dev_priv, hdmi_to_dig_port(intel_hdmi)); + live_status |= status; + printk("%c", status ? '#' : '_'); + if (try % 50 == 49) + printk("\n "); + usleep_range(1000, 1000); } + printk("\n"); if (!live_status) DRM_DEBUG_KMS("Live status not up!"); -- 2.4.10 Oh, and if you have another cable you can try, might be a good idea to see if it behaves any better. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 08/12] drm/i915: Fix struct mutex vs. RPS lock inversion
On Tue, Feb 02, 2016 at 02:46:58PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > RPS lock must be taken before the struct_mutex to avoid > locking inversion. So stop grabbing it for the whole > powersave initialization and instead only take it during > the sections which need it. > > Also, struct_mutex is not needed any more since dedicated > RPS lock was added in: > >commit 4fc688ce79772496503d22263d61b071a8fb596e >Author: Jesse Barnes >Date: Fri Nov 2 11:14:01 2012 -0700 > >drm/i915: protect RPS/RC6 related accesses (including PCU) with a new > mutex > > Based on prototype patch by Chris Wilson and a subsequent > mailing list discussion involving Ville, Imre, Chris and > Daniel. > > v2: More details in the commit. > > v3: Use drm_gem_object_unreference_unlocked. (Chris Wilson) > > Signed-off-by: Tvrtko Ursulin > Cc: Chris Wilson > Cc: Ville Syrjälä > Cc: Imre Deak > Cc: Daniel Vetter Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/12] drm/i915: GEM operations need to be done under the big lock
On Tue, Feb 02, 2016 at 11:06:25AM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > VMA creation and GEM list management need the big lock. > > v2: > > Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall) > > Not to mention drm_gem_object_unreference was there in existing > code with no mutex held. > > v3: > > Some callers of i915_gem_object_create_stolen_for_preallocated > already hold the lock so move the mutex into the other caller > as well. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21
On Wed, 10 Feb 2016, "Rogers, Martin"wrote: > Hi Jani, > > I have not submitted a bug yet, because no kernel msgs were produced the > moment I recreated the issue. > I used : drm.debug=0x14 > > Can you suggest something else ? We want the log anyway to properly identify the machine, outputs, whatnot even if it doesn't appear to have any symptoms of the issue you're seeing. BR, Jani. > TIA, > Martin > > > -Original Message- > From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > Sent: Wednesday, February 03, 2016 6:41 AM > To: Rogers, Martin; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21 > > On Tue, 02 Feb 2016, "Rogers, Martin" wrote: >> I'm seeing weird video glitches on the Intel BayLey Bay -i CRB board, >> with Fedora 21, XFCE, and I hope someone can help. > > Please file a bug at > https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel > > Add drm.debug=14 module parameter, and attach dmesg to the bug, from boot to > the problem. > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center > > ___ > This e-mail and any files transmitted with it are proprietary and intended > solely for the use of the individual or entity to whom they are addressed. If > you have reason to believe that you have received this e-mail in error, > please notify the sender and destroy this email and any attached files. > Please note that any views or opinions presented in this e-mail are solely > those of the author and do not necessarily represent those of the > Curtiss-Wright Corporation or any of its subsidiaries. Documents attached > hereto may contain technology subject to government export regulations. > Recipient is solely responsible for ensuring that any re-export, transfer or > disclosure of this information is in accordance with applicable government > export regulations. The recipient should check this e-mail and any > attachments for the presence of viruses. Curtiss-Wright Corporation and its > subsidiaries accept no liability for any damage caused by any virus > transmitted by this e-mail. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present
On 04/02/16 09:30, ankitprasad.r.sha...@intel.com wrote: From: Ankitprasad SharmaThe BIOS RapidStartTechnology may corrupt the stolen memory across S3 suspend due to unalarmed hibernation, in which case we will not be able to preserve the User data stored in the stolen region. Hence this patch tries to identify presence of the RST device on the ACPI bus, and disables use of stolen memory (for persistent data) if found. v2: Updated comment, updated/corrected new functions private to driver (Chris/Tvrtko) v3: Disabling stolen by default, wait till required acpi changes to detect device presence are pulled in (Ankit) v4: Enabled stolen by default as required acpi changes are merged (Ankit) Signed-off-by: Ankitprasad Sharma --- drivers/gpu/drm/i915/i915_drv.h| 11 +++ drivers/gpu/drm/i915/i915_gem.c| 8 drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++ drivers/gpu/drm/i915/intel_acpi.c | 10 ++ 4 files changed, 43 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 16f2f94..9d67097 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1349,6 +1349,16 @@ struct i915_gem_mm { */ bool busy; + /** +* Stolen will be lost upon hibernate (as the memory is unpowered). +* Across resume, we expect stolen to be intact - however, it may +* also be utililised by third parties (e.g. Intel RapidStart +* Technology) and if so we have to assume that any data stored in +* stolen across resume is lost and we set this flag to indicate that +* the stolen memory is volatile. +*/ + bool nonvolatile_stolen; I agree with a point made by Lukas that volatile_stolen would be better. Even the comment above suggests so. :) + /* the indicator for dispatch video commands on two BSD rings */ unsigned int bsd_ring_dispatch_index; @@ -3465,6 +3475,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state) #endif /* intel_acpi.c */ +bool intel_detect_acpi_rst(void); #ifdef CONFIG_ACPI extern void intel_register_dsm_handler(void); extern void intel_unregister_dsm_handler(void); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0cd57d4..63dab63 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -396,8 +396,16 @@ static struct drm_i915_gem_object * i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size) { struct drm_i915_gem_object *obj; + struct drm_i915_private *dev_priv = dev->dev_private; int ret; + if (!dev_priv->mm.nonvolatile_stolen) { + /* Stolen may be overwritten by external parties +* so unsuitable for persistent user data. +*/ + return ERR_PTR(-ENODEV); + } + mutex_lock(>struct_mutex); obj = i915_gem_object_create_stolen(dev, size); if (IS_ERR(obj)) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 335a1ef..4f44531 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev) */ drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size); + /* If the stolen region can be modified behind our backs upon suspend, +* then we cannot use it to store nonvolatile contents (i.e user data) +* as it will be corrupted upon resume. +*/ + dev_priv->mm.nonvolatile_stolen = true; +#ifdef CONFIG_SUSPEND + if (intel_detect_acpi_rst()) { + /* BIOSes using RapidStart Technology have been reported +* to overwrite stolen across S3, not just S4. +*/ + dev_priv->mm.nonvolatile_stolen = false; + } +#endif I also agree with the IS_ENABLED(CONFIG_SUSPEND) suggestion. + return 0; } diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c index eb638a1..67dc9b2 100644 --- a/drivers/gpu/drm/i915/intel_acpi.c +++ b/drivers/gpu/drm/i915/intel_acpi.c @@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = { 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c }; +static const struct acpi_device_id irst_ids[] = { + {"INT3392", 0}, + {"", 0} +}; + static char *intel_dsm_port_name(u8 id) { switch (id) { @@ -162,3 +167,8 @@ void intel_register_dsm_handler(void) void intel_unregister_dsm_handler(void) { } + +bool intel_detect_acpi_rst(void) +{ + return acpi_dev_present(irst_ids[0].id); +} I also agree with Lukas'es suggestion to get rid of this and just do: dev_priv->mm.volatile_stolen = IS_ENABLED(CONFIG_SUSPEND) &&
Re: [Intel-gfx] [PATCH v3 0/1] Add a lib for power management helpers
On Thu, Feb 11, 2016 at 09:25:35AM +0200, David Weinehall wrote: > This patch aims to create a separate lib for power management related > helpers. Initially it only contains code that modify settings for > external components (to handle components with default settings that > prevents entering deeper sleep states), but moving i915-related > power management helpers to this lib would probably make sense too. > > v2: Change name of library to igt_pm > Namespace all exported functions with igt_pm_ > > v3: Include igt_pm.xml in intel-gpu-tools-docs.xml > Free pm_data > Fixed a few typos > > David Weinehall (1): > lib/igt_pm: Lib for power management When resending anew complete please don't in-reply-to but start a new thread. In-reply-to is just for new versions of single patches to the existing series. Wrt the patch: If Paulo is ok, just push. lgmt. -Daniel > > .../intel-gpu-tools/intel-gpu-tools-docs.xml | 1 + > lib/Makefile.sources | 2 + > lib/igt.h | 1 + > lib/igt_aux.c | 15 +- > lib/igt_pm.c | 233 > + > lib/igt_pm.h | 31 +++ > tests/pm_lpsp.c| 25 +-- > tests/pm_rpm.c | 40 +--- > 8 files changed, 281 insertions(+), 67 deletions(-) > create mode 100644 lib/igt_pm.c > create mode 100644 lib/igt_pm.h > > -- > 2.7.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
On Wed, Feb 03, 2016 at 06:05:01AM -0800, Matt Roper wrote: > On Wed, Feb 03, 2016 at 01:16:38PM +0200, Ville Syrjälä wrote: > > On Tue, Feb 02, 2016 at 10:06:51PM -0800, Matt Roper wrote: > > > Due to our lack of two-step watermark programming, our driver has > > > historically pretended that the cursor plane is always on for the > > > purpose of watermark calculations; this helps avoid serious flickering > > > when the cursor turns off/on (e.g., when the user moves the mouse > > > pointer to a different screen). That workaround was accidentally > > > dropped as we started working toward atomic watermark updates. Since we > > > still aren't quite there yet with two-stage updates, we need to > > > resurrect the workaround and treat the cursor as always active. > > > > > > v2: Tweak cursor width calculations slightly to more closely match the > > > logic we used before the atomic overhaul began. (Ville) > > > > > > Cc: simde...@outlook.com > > > Cc: manfred.kitzbich...@gmail.com > > > Reported-by: simde...@outlook.com > > > Reported-by: manfred.kitzbich...@gmail.com > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892 > > > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from > > > ILK-style WM code (v2)") > > > Signed-off-by: Matt Roper> > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 15 ++- > > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 31bc4ea..5888632 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct > > > intel_crtc_state *cstate, > > > const struct intel_plane_state *pstate, > > > uint32_t mem_value) > > > { > > > - int cpp = pstate->base.fb ? > > > - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0; > > > + /* > > > + * We treat the cursor plane as always-on for the purposes of watermark > > > + * calculation. Until we have two-stage watermark programming merged, > > > + * this is necessary to avoid flickering. > > > + */ > > > + int cpp = 4; > > > + int width = pstate->visible ? pstate->base.crtc_w : 64; > > > > > > - if (!cstate->base.active || !pstate->visible) > > > + if (!cstate->base.active) > > > return 0; > > > > > > + > > > > Stray whitespace. > > > > Otherwise lgtm > > Reviewed-by: Ville Syrjälä > > Whitespace fixed and pushed to dinq. Also added a Cc: for -fixes since > this is relevant to 4.5. > > Thanks for the review. Hm, I don't see the CI result for v2 of this patch anywhere. Is my mailer lossy? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Use atomic state in intel_fb_initial_config.
On Mon, Feb 01, 2016 at 02:44:02PM +0100, Maarten Lankhorst wrote: > This is another step in removing legacy state. > > Signed-off-by: Maarten LankhorstPatches 2,3,4,6 lgtm. Reviewed-by: Ville Syrjälä Though I'd still prefer some locking asserts to avoid having to go double check all the callers. > --- > drivers/gpu/drm/i915/intel_fbdev.c | 17 +++-- > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > b/drivers/gpu/drm/i915/intel_fbdev.c > index 09840f4380f9..97a91e631915 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -406,8 +406,8 @@ retry: > continue; > } > > - encoder = connector->encoder; > - if (!encoder || WARN_ON(!encoder->crtc)) { > + encoder = connector->state->best_encoder; > + if (!encoder || WARN_ON(!connector->state->crtc)) { > if (connector->force > DRM_FORCE_OFF) > goto bail; > > @@ -420,7 +420,7 @@ retry: > > num_connectors_enabled++; > > - new_crtc = intel_fb_helper_crtc(fb_helper, encoder->crtc); > + new_crtc = intel_fb_helper_crtc(fb_helper, > connector->state->crtc); > > /* >* Make sure we're not trying to drive multiple connectors > @@ -466,17 +466,22 @@ retry: >* usually contains. But since our current >* code puts a mode derived from the post-pfit timings >* into crtc->mode this works out correctly. > + * > + * This is crtc->mode and not crtc->state->mode for the > + * fastboot check to work correctly. crtc_state->mode > has > + * I915_MODE_FLAG_INHERITED, which we clear to force > check > + * state. >*/ > DRM_DEBUG_KMS("looking for current mode on connector > %s\n", > connector->name); > - modes[i] = >crtc->mode; > + modes[i] = >state->crtc->mode; > } > crtcs[i] = new_crtc; > > DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n", > connector->name, > - pipe_name(to_intel_crtc(encoder->crtc)->pipe), > - encoder->crtc->base.id, > + > pipe_name(to_intel_crtc(connector->state->crtc)->pipe), > + connector->state->crtc->base.id, > modes[i]->hdisplay, modes[i]->vdisplay, > modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" > :""); > > -- > 2.1.0 > > ___ > 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 https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
On Thu, Feb 11, 2016 at 09:50:45AM +0100, Daniel Vetter wrote: > On Wed, Feb 03, 2016 at 06:05:01AM -0800, Matt Roper wrote: > > On Wed, Feb 03, 2016 at 01:16:38PM +0200, Ville Syrjälä wrote: > > > On Tue, Feb 02, 2016 at 10:06:51PM -0800, Matt Roper wrote: > > > > Due to our lack of two-step watermark programming, our driver has > > > > historically pretended that the cursor plane is always on for the > > > > purpose of watermark calculations; this helps avoid serious flickering > > > > when the cursor turns off/on (e.g., when the user moves the mouse > > > > pointer to a different screen). That workaround was accidentally > > > > dropped as we started working toward atomic watermark updates. Since we > > > > still aren't quite there yet with two-stage updates, we need to > > > > resurrect the workaround and treat the cursor as always active. > > > > > > > > v2: Tweak cursor width calculations slightly to more closely match the > > > > logic we used before the atomic overhaul began. (Ville) > > > > > > > > Cc: simde...@outlook.com > > > > Cc: manfred.kitzbich...@gmail.com > > > > Reported-by: simde...@outlook.com > > > > Reported-by: manfred.kitzbich...@gmail.com > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892 > > > > Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters > > > > from ILK-style WM code (v2)") > > > > Signed-off-by: Matt Roper> > > > --- > > > > drivers/gpu/drm/i915/intel_pm.c | 15 ++- > > > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > index 31bc4ea..5888632 100644 > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > @@ -1799,16 +1799,21 @@ static uint32_t ilk_compute_cur_wm(const struct > > > > intel_crtc_state *cstate, > > > >const struct intel_plane_state > > > > *pstate, > > > >uint32_t mem_value) > > > > { > > > > - int cpp = pstate->base.fb ? > > > > - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) > > > > : 0; > > > > + /* > > > > +* We treat the cursor plane as always-on for the purposes of > > > > watermark > > > > +* calculation. Until we have two-stage watermark programming > > > > merged, > > > > +* this is necessary to avoid flickering. > > > > +*/ > > > > + int cpp = 4; > > > > + int width = pstate->visible ? pstate->base.crtc_w : 64; > > > > > > > > - if (!cstate->base.active || !pstate->visible) > > > > + if (!cstate->base.active) > > > > return 0; > > > > > > > > + > > > > > > Stray whitespace. > > > > > > Otherwise lgtm > > > Reviewed-by: Ville Syrjälä > > > > Whitespace fixed and pushed to dinq. Also added a Cc: for -fixes since > > this is relevant to 4.5. > > > > Thanks for the review. > > Hm, I don't see the CI result for v2 of this patch anywhere. Is my mailer > lossy? > -Daniel Looks like they were here: https://lists.freedesktop.org/archives/intel-gfx/2016-February/086953.html Matt > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add missing 'else' to intel_digital_port_connected()
From: Ville Syrjäläintel_digital_port_connected() lacks one 'else'. There's no actual harm in not having it since each branch has an unconditional return, so it can't accidentally end up in taking two branches instead of just the one. But let's be consistent and add the 'else' anyway. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index bbe18996efe6..040b86b9797f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4551,7 +4551,7 @@ bool intel_digital_port_connected(struct drm_i915_private *dev_priv, { if (HAS_PCH_IBX(dev_priv)) return ibx_digital_port_connected(dev_priv, port); - if (HAS_PCH_SPLIT(dev_priv)) + else if (HAS_PCH_SPLIT(dev_priv)) return cpt_digital_port_connected(dev_priv, port); else if (IS_BROXTON(dev_priv)) return bxt_digital_port_connected(dev_priv, port); -- 2.4.10 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 6/6] drm/i915: fix context/engine cleanup order
Chris Wilsonwrites: > On Sat, Jan 30, 2016 at 11:17:07AM +, Chris Wilson wrote: >> On Fri, Jan 29, 2016 at 07:19:31PM +, Dave Gordon wrote: >> > From: Nick Hoath >> > >> > Swap the order of context & engine cleanup, so that contexts are cleaned >> > up first, and *then* engines. This is a more sensible order anyway, but >> > in particular has become necessary since the 'intel_ring_initialized() >> > must be simple and inline' patch, which now uses ring->dev as an >> > 'initialised' flag, so it can now be NULL after engine teardown. This in >> > turn can cause a problem in the context code, which (used to) check the >> > ring->dev->struct_mutex -- causing a fault if ring->dev was NULL. >> > >> > Also rename the cleanup function to reflect what it actually does >> > (cleanup engines, not a ringbuffer), and fix an annoying whitespace >> > issue. >> > >> > v2: Also make the fix in i915_load_modeset_init, not just in >> > i915_driver_unload (Chris Wilson) >> > v3: Had extra stuff in it. >> > v4: Reverted extra stuff (so we're back to v2). >> > Rebased and updated commentary above (Dave Gordon). >> > >> > Signed-off-by: Nick Hoath >> > Signed-off-by: David Gordon >> > Reviewed-by: Chris Wilson >> >> Have to drop that, with the recent context changes. >> >> You have to move the gpu-reset now for execlists. >> >> Basically pull it out into: >> >> static void i915_unload_gem(struct drm_device *dev) >> { >>/* >> * Neither the BIOS, ourselves or any other kernel >> * expects the system to be in execlists mode on startup, >> * so we need to reset the GPU back to legacy mode. And the only >> * known way to disable logical contexts is through a GPU reset. >> * >> * So in order to leave the system in a known default configration, >> * always reset the GPU upon unload. This also cleans up the GEM >> * state tracking, flushing off the requests and leaving the system >> * idle. >> * >> * Note that is of the upmost importance that the GPU is idle and >> * all stray writes are flushed *before* we dismantle the backing >> * storage for the pinned objects. >> */ >>i915_reset(dev); >> >>mutex_lock(>struct_mutex); >>i915_gem_context_fini(dev); >>i915_gem_cleanup_ringbuffer(dev); >>mutex_unlock(>struct_mutex); >> } >> >> and then kill the intel_gpu_reset along both the cleanup pathsh > > It appears this patch was applied without dropping my r-b for the issue > I pointed out above. Now causes a splat in intel_logical_ring_cleanup when unloading module. Best to revert and rework on top of Dave's cleanup set? -Mika [ 58.170369] BUG: unable to handle kernel NULL pointer dereference at (null) [ 58.170374] IP: [] intel_logical_ring_cleanup+0x83/0x100 [i915] [ 58.170389] PGD 0 [ 58.170391] Oops: [#1] PREEMPT SMP [ 58.170394] Modules linked in: x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel i915(-) mei_me mei i2c_hid e1000e ptp pps_core [ 58.170404] CPU: 0 PID: 5766 Comm: rmmod Not tainted 4.5.0-rc3+ #43 [ 58.170407] Hardware name: System manufacturer System Product Name/Z170M-PLUS, BIOS 0505 11/16/2015 [ 58.170410] task: 880036aab380 ti: 8802374c task.ti: 8802374c [ 58.170412] RIP: 0010:[] [] intel_logical_ring_cleanup+0x83/0x100 [i915] [ 58.170424] RSP: 0018:8802374c3d30 EFLAGS: 00010282 [ 58.170425] RAX: RBX: 880237203788 RCX: 0001 [ 58.170428] RDX: 87654321 RSI: 000d RDI: 8802372037c0 [ 58.170430] RBP: 8802374c3d40 R08: R09: 000ad856c000 [ 58.170432] R10: R11: 0001 R12: 88023720 [ 58.170434] R13: 880237209638 R14: 880237323000 R15: 558a770bd090 [ 58.170438] FS: 7f8b439ff700() GS:88023980() knlGS: [ 58.170442] CS: 0010 DS: ES: CR0: 80050033 [ 58.170444] CR2: CR3: 00023532d000 CR4: 003406f0 [ 58.170447] DR0: DR1: DR2: [ 58.170450] DR3: DR6: 4ff0 DR7: 0400 [ 58.170453] Stack: [ 58.170455] 880237203788 88023720 8802374c3d70 a00d0ed4 [ 58.170460] 88023720 880237323000 880237323060 a0194360 [ 58.170465] 8802374c3d98 a0154520 880237323000 880237323000 [ 58.170469] Call Trace: [ 58.170479] [] i915_gem_cleanup_engines+0x34/0x60 [i915] [ 58.170493] [] i915_driver_unload+0x140/0x220 [i915] [ 58.170497] [] drm_dev_unregister+0x24/0xa0 [ 58.170501] [] drm_put_dev+0x1e/0x60 [ 58.170506] []
[Intel-gfx] [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format
From: Deepak MThe bpp value which is used while calulating the txbyteclkhs values should be wrt the pixel format value. Currently bpp is coming from pipe config to calculate txbyteclkhs. Fix it in this patch. V2: dsi_pixel_format_bpp is used to retrieve the bpp from pixel_format [Review: Jani] Signed-off-by: Deepak M Signed-off-by: Yogesh Mohan Marimuthu Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/intel_dsi.c |5 ++--- drivers/gpu/drm/i915/intel_dsi.h |2 ++ drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |5 + drivers/gpu/drm/i915/intel_dsi_pll.c |2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 91cef35..ce94342 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder, { struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); enum port port; - unsigned int bpp = intel_crtc->config->pipe_bpp; + unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format); unsigned int lane_count = intel_dsi->lane_count; u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); const struct drm_display_mode *adjusted_mode = _crtc->config->base.adjusted_mode; enum port port; - unsigned int bpp = intel_crtc->config->pipe_bpp; + unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format); u32 val, tmp; u16 mode_hdisplay; diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index de7be7f..92f3922 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -34,6 +34,8 @@ #define DSI_DUAL_LINK_FRONT_BACK 1 #define DSI_DUAL_LINK_PIXEL_ALT2 +int dsi_pixel_format_bpp(int pixel_format); + struct intel_dsi_host; struct intel_dsi { diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index 1d43e6f..23c0f67 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -420,10 +420,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id) intel_dsi->dual_link = mipi_config->dual_link; intel_dsi->pixel_overlap = mipi_config->pixel_overlap; - if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666) - bits_per_pixel = 18; - else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565) - bits_per_pixel = 16; + bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format); intel_dsi->operation_mode = mipi_config->is_cmd_mode; intel_dsi->video_mode_format = mipi_config->video_transfer_mode; diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c index bb5e95a..70883c5 100644 --- a/drivers/gpu/drm/i915/intel_dsi_pll.c +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c @@ -30,7 +30,7 @@ #include "i915_drv.h" #include "intel_dsi.h" -static int dsi_pixel_format_bpp(int pixel_format) +int dsi_pixel_format_bpp(int pixel_format) { int bpp; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3 V3] drm/i915/BXT: Fixed COS blanking issue
From: Uma ShankarDuring Charging OS mode, mipi display was blanking.This is because during driver load, though encoder, connector were active but crtc returned inactive. This caused sanitize function to disable the DSI panel. In AOS, this is fine since HWC will do a modeset and crtc, connector, encoder mapping will be restored. But in COS, no modeset is called, it just calls DPMS ON/OFF. This is fine on BYT/CHT since transcoder is common b/w all encoders. But for BXT, there is a separate mipi transcoder. Hence this needs special handling for BXT. V2: pipe_config->has_dsi_encoder is set for BXT [By Jani] V3: Timing parameters are retrieved from VBT. [By Jani] Signed-off-by: Uma Shankar Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/intel_display.c | 106 -- 1 file changed, 100 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a66220a..e47a75c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -34,6 +34,7 @@ #include #include #include "intel_drv.h" +#include "intel_dsi.h" #include #include "i915_drv.h" #include "i915_trace.h" @@ -7814,6 +7815,53 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc) (intel_crtc->config->pipe_src_h - 1)); } +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc, + struct intel_crtc_state *pipe_config) +{ + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_display_mode *vbt_dsi_mode = + dev_priv->vbt.lfp_lvds_vbt_mode; + struct intel_encoder *encoder; + uint32_t tmp; + + pipe_config->base.adjusted_mode.crtc_hdisplay = vbt_dsi_mode->hdisplay; + pipe_config->base.adjusted_mode.crtc_htotal = vbt_dsi_mode->htotal; + pipe_config->base.adjusted_mode.crtc_hblank_start = + vbt_dsi_mode->hdisplay; + pipe_config->base.adjusted_mode.crtc_hblank_end = vbt_dsi_mode->htotal; + pipe_config->base.adjusted_mode.crtc_hsync_start = + vbt_dsi_mode->hsync_start; + pipe_config->base.adjusted_mode.crtc_hsync_end = + vbt_dsi_mode->hsync_end; + + pipe_config->base.adjusted_mode.crtc_vdisplay = vbt_dsi_mode->vdisplay; + pipe_config->base.adjusted_mode.crtc_vtotal = vbt_dsi_mode->vtotal; + pipe_config->base.adjusted_mode.crtc_vblank_start = + vbt_dsi_mode->vdisplay; + pipe_config->base.adjusted_mode.crtc_vblank_end = vbt_dsi_mode->vtotal; + pipe_config->base.adjusted_mode.crtc_vsync_start = + vbt_dsi_mode->vsync_start; + pipe_config->base.adjusted_mode.crtc_vsync_end = + vbt_dsi_mode->vsync_end; + + for_each_encoder_on_crtc(dev, >base, encoder) { + if (encoder->type == INTEL_OUTPUT_DSI) { + struct intel_dsi *intel_dsi = + enc_to_intel_dsi(>base); + pipe_config->pipe_bpp = + dsi_pixel_format_bpp(intel_dsi->pixel_format); + } + } + + tmp = I915_READ(PIPESRC(crtc->pipe)); + pipe_config->pipe_src_h = (tmp & 0x) + 1; + pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1; + + pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h; + pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w; +} + static void intel_get_pipe_timings(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config) { @@ -9962,6 +10010,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc, } } +static void bxt_get_pipe_config_dsi(struct intel_crtc *crtc, + struct intel_crtc_state *pipe_config) +{ + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + enum port port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) + ? PORT_A : PORT_C; + uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port)); + uint32_t tmp; + + tmp = I915_READ(BXT_MIPI_PORT_CTRL(port)); + if (tmp & DPI_ENABLE) { + enum pipe trans_dsi_pipe; + + switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) { + default: + WARN(1, "unknown pipe linked to dsi transcoder\n"); + return; + case BXT_PIPE_SELECT(PIPE_A): + trans_dsi_pipe = PIPE_A; + break; +
Re: [Intel-gfx] [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue
Hi, Thanks for the review comments. Addressing them in next version. On Thursday 04 February 2016 07:24 PM, Jani Nikula wrote: On Wed, 03 Feb 2016, Ramalingam Cwrote: From: Uma Shankar During Charging OS mode, mipi display was blanking.This is because during driver load, though encoder, connector were active but crtc returned inactive. This caused sanitize function to disable the DSI panel. In AOS, this is fine since HWC will do a modeset and crtc, connector, encoder mapping will be restored. But in COS, no modeset is called, it just calls DPMS ON/OFF. This is fine on BYT/CHT since transcoder is common b/w all encoders. But for BXT, there is a separate mipi transcoder. Hence this needs special handling for BXT. Signed-off-by: Uma Shankar Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/intel_display.c | 115 -- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a66220a..58d2cd9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -34,6 +34,7 @@ #include #include #include "intel_drv.h" +#include "intel_dsi.h" #include #include "i915_drv.h" #include "i915_trace.h" @@ -7814,6 +7815,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc) (intel_crtc->config->pipe_src_h - 1)); } +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc, + struct intel_crtc_state *pipe_config) +{ + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; Hum, how does this work for DSI transcoders. + struct intel_encoder *encoder; + uint32_t tmp; + + tmp = I915_READ(HTOTAL(cpu_transcoder)); + pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0x) + 1; + pipe_config->base.adjusted_mode.crtc_htotal = + ((tmp >> 16) & 0x) + 1; + tmp = I915_READ(HBLANK(cpu_transcoder)); + pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0x) + 1; + pipe_config->base.adjusted_mode.crtc_hblank_end = + ((tmp >> 16) & 0x) + 1; + tmp = I915_READ(HSYNC(cpu_transcoder)); + pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0x) + 1; + pipe_config->base.adjusted_mode.crtc_hsync_end = + ((tmp >> 16) & 0x) + 1; + + tmp = I915_READ(VBLANK(cpu_transcoder)); + pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0x) + 1; + pipe_config->base.adjusted_mode.crtc_vblank_end = + ((tmp >> 16) & 0x) + 1; + tmp = I915_READ(VSYNC(cpu_transcoder)); + pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0x) + 1; + pipe_config->base.adjusted_mode.crtc_vsync_end = + ((tmp >> 16) & 0x) + 1; + + if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) { + pipe_config->base.adjusted_mode.flags |= + DRM_MODE_FLAG_INTERLACE; + pipe_config->base.adjusted_mode.crtc_vtotal += 1; + pipe_config->base.adjusted_mode.crtc_vblank_end += 1; + } + + + for_each_encoder_on_crtc(dev, >base, encoder) { + struct intel_dsi *intel_dsi = + enc_to_intel_dsi(>base); + enum port port; + + pipe_config->pipe_bpp = intel_dsi->dsi_bpp; + for_each_dsi_port(port, intel_dsi->ports) { + pipe_config->base.adjusted_mode.crtc_hdisplay = + I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); + pipe_config->base.adjusted_mode.crtc_vdisplay = + I915_READ(BXT_MIPI_TRANS_VACTIVE(port)); + pipe_config->base.adjusted_mode.crtc_vtotal = + I915_READ(BXT_MIPI_TRANS_VTOTAL(port)); + } + } Since you already figure out the port/pipe in bxt_get_pipe_config_dsi, I feel it would be better to use that instead of doing the for loops here and semi-blindly casting the encoder to intel_dsi. + + tmp = I915_READ(PIPESRC(crtc->pipe)); + pipe_config->pipe_src_h = (tmp & 0x) + 1; + pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1; + + pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h; + pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w; +} What's the point of duplicating most of intel_get_pipe_timings() when you could just call that
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format
On Thursday 04 February 2016 06:43 PM, Jani Nikula wrote: On Wed, 03 Feb 2016, Ramalingam Cwrote: From: Deepak M The bpp value which is used while calulating the txbyteclkhs values should be wrt the pixel format value. Currently bpp is coming from pipe config to calculate txbyteclkhs. Fix it in this patch. Signed-off-by: Deepak M Signed-off-by: Yogesh Mohan Marimuthu Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/intel_dsi.c |5 ++--- drivers/gpu/drm/i915/intel_dsi.h |1 + drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 91cef35..aa11293 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder, { struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); enum port port; - unsigned int bpp = intel_crtc->config->pipe_bpp; + unsigned int bpp = intel_dsi->dsi_bpp; unsigned int lane_count = intel_dsi->lane_count; u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp; @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder); const struct drm_display_mode *adjusted_mode = _crtc->config->base.adjusted_mode; enum port port; - unsigned int bpp = intel_crtc->config->pipe_bpp; + unsigned int bpp = intel_dsi->dsi_bpp; u32 val, tmp; u16 mode_hdisplay; diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index de7be7f..9bf6fa1d 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -64,6 +64,7 @@ struct intel_dsi { /* video mode pixel format for MIPI_DSI_FUNC_PRG register */ u32 pixel_format; + u32 dsi_bpp; Please never add extra state for things that can trivially be derived from existing information. Given the dsi_pixel_format_bpp() in intel_dsi_pll.c, this should always hold, right: intel_dsi->dsi_bpp == dsi_pixel_format_bpp(intel_dsi->pixel_format) Please just make dsi_pixel_format_bpp() available and use it. As a nice bonus, this becomes self-documenting code on why pipe config is not used where the bpp based on pixel format is used. Agreed. Implemented in the next version. Thanks BR, Jani. /* video mode format for MIPI_VIDEO_MODE_FORMAT register */ u32 video_mode_format; diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index 1d43e6f..bf266cb 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id) intel_dsi->bw_timer = mipi_config->dbi_bw_timer; intel_dsi->video_frmt_cfg_bits = mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0; + intel_dsi->dsi_bpp = bits_per_pixel; pclk = mode->clock; -- Thanks, --Ram ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI
In case of BXT DSI we are updating the CPU_TRANSCODER with appropriate value. Signed-off-by: Ramalingam CSigned-off-by: Uma Shankar --- drivers/gpu/drm/i915/i915_drv.h |2 ++ drivers/gpu/drm/i915/intel_display.c |5 + 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 65a2cd0..ef4b376 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -122,6 +122,8 @@ enum transcoder { TRANSCODER_B, TRANSCODER_C, TRANSCODER_EDP, + TRANSCODER_MIPI_A, + TRANSCODER_MIPI_C, I915_MAX_TRANSCODERS }; #define transcoder_name(t) ((t) + 'A') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e47a75c..9715056 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc, enc_to_intel_dsi(>base); pipe_config->pipe_bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format); + + if (intel_dsi->ports & (1 << PORT_A)) + pipe_config->cpu_transcoder = TRANSCODER_MIPI_A; + else + pipe_config->cpu_transcoder = TRANSCODER_MIPI_C; } } -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 00/22] Add support for GuC-based SLPC
Em Ter, 2016-02-09 às 12:33 +0530, Kamble, Sagar A escreveu: > Hi Paulo, > > Thanks for comments. > 1. Will make change related to #define for number of pipes and > remove > the unnecessary ones. > 2. vrefresh is almost same as "clock/(htotal*vtotal) if we round up > later. Will keep it vrefresh for now. I was under the impression that the VSyncFTUsec field wanted the more precise solution. Maybe Tom or someone else could clarify this to us. > > Thanks > Sagar > > > On 2/4/2016 1:55 AM, Zanoni, Paulo R wrote: > > Em Qua, 2016-01-20 às 18:26 -0800, tom.orou...@intel.com escreveu: > > > From: Tom O'Rourke> > > > > > SLPC (Single Loop Power Controller) is a replacement for > > > some host-based power management features. The SLPC > > > implemenation runs in firmware on GuC. > > > > > > This series is a first request for comments. This series > > > is not expected to be merged. After changes based on > > > comments, a later patch series will be sent for merging. > > > > > > This series has been tested with SKL guc firmware > > > versions 4.3 and 4.7. The graphics power management > > > features in SLPC in those versions are DFPS (Dynamic FPS), > > > Turbo, and DCC (Duty Cycle Control). DFPS adjusts > > > requested graphics frequency to maintain target framerate. > > > Turbo adjusts requested graphics frequency to maintain > > > target GT busyness. DCC adjusts requested graphics > > > frequency and stalls guc-scheduler to maintain actual > > > graphics frequency in efficient range. > > > > > > Patch 1/22 is included ihere for convenience and should be > > > part of an earlier series. SLPC assumes guc firmware has > > > been loaded and GuC submission is enabled. > > > > > > Patch 22/22 sets the flag to enable SLPC on SKL. Without > > > this patch, the previous patches should have no effect. > > > > > > VIZ-6773, VIZ-6889 > > Hi > > > > Some high-level comments for the whole series: > > > > In many places there are 8 spaces instead of tabs. > > > > There are also some lines containing just a tab character. > > > > Lots of Yoda conditions: if (constant == variable). > > > > Some functions would get much simpler if they had early returns. > > > > I certainly wouldn't complain if you also extracted the relevant > > rps/powersave code out of intel_pm.c to its own file with a nice > > documentation. Of course, this could be done after the series. > > > > Lots of ignored return values. It seems the inner functions all > > check > > for errors and return them to their callers, but the top-most > > functions > > added by the series just ignore the errors. See my previous comment > > on > > patch 14 about this for suggestions. > > > > There are no checks for GuC version here. We know that the SLPC ABI > > is > > not stable and can change in new firmware versions, so I expect the > > SLPC code to only run if it finds the specific _whitelisted_ GuC > > versions. No "if (version >= num) use_slpc=true;", please. > > > > I'm not 100% sure, but from what I could understand, it seems I'll > > get > > a broken machine with no RPS at all if I don't have the GuC > > firmware > > files - or if the GuC version is not the expected. IMHO this is a > > regression since I currently don't have any firmware files and my > > SKL > > machine works. > > > > I see most of the functions are protected by "HAS_SLPC". Usually > > HAS_SOMETHING is used for hardware features and is expected to be > > constant on platforms. I suggest you to add a possible driver > > parameter > > such as i915.enable_slpc, and also add a new "intel_slpc_enabled()" > > or > > "intel_using_slpc()" function and replace all the HAS_SLPC checks > > with > > these. This way, we'll be able to easily disable SLPC in case we > > don't > > want it (such as due to a regression) or if there's no firmware or > > incorrect firmware version, and revert back to the old (current) > > behavior of driver-based turbo. The only HAS_SLPC check should be > > during SLPC initialization. Having an easy way to enable/disable > > SLPC > > will also be immensely helpful when we start running workloads to > > decide if we want to enable it. > > > > You could even go further and make the i915.enable_slpc param be a > > mask > > where it's possible to select each sub-feature individually (dfps, > > turbo, dcc). > > > > Some of the checks for shared_data_obj could also become calls to > > an > > inline function with a nice name such as intel_slpc_enabled() or > > something else. > > > > I see there's a specific pattern on the places that call > > host2guc_action(). Perhaps we could move that common code to its > > own > > function? It would also be nice to add a name to the 0xFF mask that > > we > > return - and that gets ignored at the end of the call chains. > > > > Patch 5 needs a commit message. Actually, when reviewing patch 4 I > > thought it had broken RC6, until I saw patch 5, so maybe you could > > just > > squash both commits
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present
Hi Ankitprasad, On Thu, Feb 04, 2016 at 05:43:17PM +0100, Lukas Wunner wrote: > On Thu, Feb 04, 2016 at 04:05:04PM +, Chris Wilson wrote: > > We could #define INTEL_RAPID_START "INT3392" for > > if (IS_ENABLED(CONFIG_SUSPEND) && acpi_dev_present(INTEL_RAPID_START)) > > but Anki wanted to keep the acpi details themselves out of stolen (hence > > the current split). > > Less code is almost always better, it's more work for a reader to > follow the logic if things are split across multiple files. > > If you absolutely positively want to keep the current split, > the "static const struct acpi_device_id irst_ids[]" data structure > should be replaced by a "static const char*" in order to not waste > memory. As I've learned the hard way yesterday, acpi_dev_present() is undefined if the kernel is compiled without CONFIG_ACPI, so you made the right call splitting that out into intel_acpi.c and my suggested simplification was wrong. Sorry for the noise. :-/ I'd still suggest to invoke acpi_dev_present() with the string literal "INT3392" in intel_acpi.c:intel_detect_acpi_rst() though, or at least replace "static const struct acpi_device_id irst_ids[]" with "static const char*". Best regards, Lukas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Introduce i915_gem_object_get_dma_address()
On 04/02/16 09:30, ankitprasad.r.sha...@intel.com wrote: From: Chris WilsonThis utility function is a companion to i915_gem_object_get_page() that uses the same cached iterator for the scatterlist to perform fast sequential lookup of the dma address associated with any page within the object. Signed-off-by: Chris Wilson Signed-off-by: Ankitprasad Sharma --- drivers/gpu/drm/i915/i915_drv.h | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 65a2cd0..e4c25c6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2947,6 +2947,23 @@ static inline int __sg_page_count(struct scatterlist *sg) struct page * i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, int n); +static inline dma_addr_t +i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj, int n) +{ + if (n < obj->get_page.last) { + obj->get_page.sg = obj->pages->sgl; + obj->get_page.last = 0; + } + + while (obj->get_page.last + __sg_page_count(obj->get_page.sg) <= n) { + obj->get_page.last += __sg_page_count(obj->get_page.sg++); + if (unlikely(sg_is_chain(obj->get_page.sg))) + obj->get_page.sg = sg_chain_ptr(obj->get_page.sg); + } + + return sg_dma_address(obj->get_page.sg) + ((n - obj->get_page.last) << PAGE_SHIFT); +} + static inline struct page * i915_gem_object_get_page(struct drm_i915_gem_object *obj, int n) { Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [IGT PATCH 5/6] kms_force_connector_basic: Add force-load-detect test
Op 11-02-16 om 09:59 schreef Daniel Vetter: > On Mon, Feb 01, 2016 at 02:44:01PM +0100, Maarten Lankhorst wrote: >> Signed-off-by: Maarten Lankhorst>> --- >> diff --git a/tests/kms_force_connector_basic.c >> b/tests/kms_force_connector_basic.c >> index bd80caeffd82..f827d0008f7b 100644 >> --- a/tests/kms_force_connector_basic.c >> +++ b/tests/kms_force_connector_basic.c >> @@ -52,6 +52,8 @@ static void reset_connectors(void) >> >> drmModeFreeConnector(connector); >> } >> + >> +igt_set_module_param_int("load_detect_test", 0); >> } >> >> static int opt_handler(int opt, int opt_index, void *data) >> @@ -108,6 +110,17 @@ int main(int argc, char **argv) >> igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED); >> } >> >> +igt_subtest("force-load-detect") { >> +igt_set_module_param_int("load_detect_test", 1); > Ah here's the testcase. We need to unset this module parameter again at > the end of the testcase - atm the magic exithandler stuff only works on > binary exit, not when leaving a subtests (for things set within a > subtest). > > With that done Reviewed-by: Daniel Vetter > > We should probably fix that issue in the exit handler code, but that's > another task really. > Yeah, but pushed the fixed version for now. Thanks! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21
On Wed, Feb 10, 2016 at 07:16:03PM +, Rogers, Martin wrote: > Hi Jani, > > I have not submitted a bug yet, because no kernel msgs were produced the > moment I recreated the issue. > I used : drm.debug=0x14 That should be drm.debug=14 or drm.debug=0xe > > Can you suggest something else ? > TIA, > Martin > > > -Original Message- > From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > Sent: Wednesday, February 03, 2016 6:41 AM > To: Rogers, Martin; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21 > > On Tue, 02 Feb 2016, "Rogers, Martin"wrote: > > I'm seeing weird video glitches on the Intel BayLey Bay -i CRB board, > > with Fedora 21, XFCE, and I hope someone can help. > > Please file a bug at > https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel > > Add drm.debug=14 module parameter, and attach dmesg to the bug, from boot to > the problem. > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center > > ___ > This e-mail and any files transmitted with it are proprietary and intended > solely for the use of the individual or entity to whom they are addressed. If > you have reason to believe that you have received this e-mail in error, > please notify the sender and destroy this email and any attached files. > Please note that any views or opinions presented in this e-mail are solely > those of the author and do not necessarily represent those of the > Curtiss-Wright Corporation or any of its subsidiaries. Documents attached > hereto may contain technology subject to government export regulations. > Recipient is solely responsible for ensuring that any re-export, transfer or > disclosure of this information is in accordance with applicable government > export regulations. The recipient should check this e-mail and any > attachments for the presence of viruses. Curtiss-Wright Corporation and its > subsidiaries accept no liability for any damage caused by any virus > transmitted by this e-mail. > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 1/3] tests/gem_concurrent_blit: rename gem_concurrent_all
We'll both rename gem_concurrent_all over gem_concurrent_blit and change gem_concurrent_blit in this changeset. To make this easier to follow we first do the the rename. Signed-off-by: David Weinehall--- tests/gem_concurrent_blit.c | 1548 ++- 1 file changed, 1540 insertions(+), 8 deletions(-) diff --git a/tests/gem_concurrent_blit.c b/tests/gem_concurrent_blit.c index 513de4a1b719..9b7ef8700e31 100644 --- a/tests/gem_concurrent_blit.c +++ b/tests/gem_concurrent_blit.c @@ -1,8 +1,1540 @@ -/* This test is just a duplicate of gem_concurrent_all. */ -/* However the executeable will be gem_concurrent_blit. */ -/* The main function examines argv[0] and, in the case */ -/* of gem_concurent_blit runs only a subset of the */ -/* available subtests. This avoids the use of */ -/* non-standard command line parameters which can cause */ -/* problems for automated testing */ -#include "gem_concurrent_all.c" +/* + * Copyright © 2009,2012,2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Eric Anholt + *Chris Wilson + *Daniel Vetter + * + */ + +/** @file gem_concurrent.c + * + * This is a test of pread/pwrite/mmap behavior when writing to active + * buffers. + * + * Based on gem_gtt_concurrent_blt. + */ + +#include "igt.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "intel_bufmgr.h" + +IGT_TEST_DESCRIPTION("Test of pread/pwrite/mmap behavior when writing to active" +" buffers."); + +#define LOCAL_I915_GEM_USERPTR 0x33 +#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr) +struct local_i915_gem_userptr { + uint64_t user_ptr; + uint64_t user_size; + uint32_t flags; + uint32_t handle; +}; + +int fd, devid, gen; +struct intel_batchbuffer *batch; +int all; +int pass; + +struct buffers { + const struct access_mode *mode; + drm_intel_bufmgr *bufmgr; + drm_intel_bo **src, **dst; + drm_intel_bo *snoop, *spare; + uint32_t *tmp; + int width, height, size; + int count; +}; + +#define MIN_BUFFERS 3 + +static void blt_copy_bo(struct buffers *b, drm_intel_bo *dst, drm_intel_bo *src); + +static void +nop_release_bo(drm_intel_bo *bo) +{ + drm_intel_bo_unreference(bo); +} + +static void +prw_set_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) +{ + for (int i = 0; i < b->size; i++) + b->tmp[i] = val; + drm_intel_bo_subdata(bo, 0, 4*b->size, b->tmp); +} + +static void +prw_cmp_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) +{ + uint32_t *vaddr; + + vaddr = b->tmp; + do_or_die(drm_intel_bo_get_subdata(bo, 0, 4*b->size, vaddr)); + for (int i = 0; i < b->size; i++) + igt_assert_eq_u32(vaddr[i], val); +} + +#define pixel(y, width) ((y)*(width) + (((y) + pass)%(width))) + +static void +partial_set_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) +{ + for (int y = 0; y < b->height; y++) + do_or_die(drm_intel_bo_subdata(bo, 4*pixel(y, b->width), 4, )); +} + +static void +partial_cmp_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) +{ + for (int y = 0; y < b->height; y++) { + uint32_t buf; + do_or_die(drm_intel_bo_get_subdata(bo, 4*pixel(y, b->width), 4, )); + igt_assert_eq_u32(buf, val); + } +} + +static drm_intel_bo * +create_normal_bo(drm_intel_bufmgr *bufmgr, uint64_t size) +{ + drm_intel_bo *bo; + + bo = drm_intel_bo_alloc(bufmgr, "bo", size, 0); + igt_assert(bo); + + return bo; +} + +static bool can_create_normal(void) +{ + return true;
[Intel-gfx] [PATCH i-g-t v4 0/3] Unify slow/combinatorial test handling
Until now we've had no unified way to handle slow/combinatorial tests. Most of the time we don't want to run slow/combinatorial tests, so this should remain the default, but when we do want to run such tests, it has been handled differently in different tests. This patch adds an --all command line option to igt_core, changes gem_concurrent_blit and kms_frontbuffer_tracking to use this instead of their own methods, and removes gem_concurrent_all in the process, since it's now unnecessary. Test cases that have subtests that should not be run by default should use the igt_subtest_flags() / ugt_subtest_flags_f() functions and pass the subtest types as part of the flags parameter. v2: Incorporate various suggestions from reviewers. v3: Rewrite to provide a generic mechanism for categorising the subtests v4: Refreshed against a more recent version of i-g-t David Weinehall (3): Copy gem_concurrent_all to gem_concurrent_blit Unify handling of slow/combinatorial tests Remove superfluous gem_concurrent_all.c David Weinehall (3): tests/gem_concurrent_blit: rename gem_concurrent_all lib/igt_core: Unify handling of slow/combinatorial tests tests/gem_concurrent_all: Remove gem_concurrent_all.c lib/igt_core.c | 43 +- lib/igt_core.h | 42 ++ tests/Makefile.sources |1 - tests/gem_concurrent_all.c | 1540 - tests/gem_concurrent_blit.c | 1548 +- tests/kms_frontbuffer_tracking.c | 186 +++-- 6 files changed, 1725 insertions(+), 1635 deletions(-) delete mode 100644 tests/gem_concurrent_all.c -- 2.7.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 0/6] Check pixel clock when setting mode
> -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Thursday, February 11, 2016 11:21 AM > To: Kahola, Mika> Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v4 0/6] Check pixel clock when setting mode > > On Tue, Feb 02, 2016 at 03:16:37PM +0200, Mika Kahola wrote: > > From EDID we can read and request higher pixel clock than our HW can > > support. This set of patches add checks if requested pixel clock is > > lower than the one supported by the HW. > > The requested mode is discarded if we cannot support the requested > > pixel clock. For example for Cherryview > > > > 'cvt 2560 1600 60' gives > > > > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz > > Modeline "2560x1600_60.00" 348.50 2560 2760 3032 3504 1600 1603 > > 1609 1658 -hsync +vsync > > > > where pixel clock 348.50 MHz is higher than the supported 304 MHz. > > > > The missing mode validity checks for DisplayPort, HDMI, DP-MST, SDVO, > CRT, and TV. > > > > V2: > > - The maximum DOT clock frequency is added to debugfs > i915_frequency_info. > > - max dotclock cached in dev_priv structure > > - moved computation of max dotclock to 'intel_display.c' > > > > V3: > > - intel_update_max_dotclk() renamed as intel_compute_max_dotclk() > > - for GEN9 and above the max dotclock frequency is equal to CD clock > > frequency > > - for older generations the dot clock frequency is limited to 90% of the > > CD clock frequency > > - For Cherryview the dot clock is limited to 95% of CD clock frequency > > - for GEN2/3 the maximum dot clock frequency is limited to 90% of the > > 2X CD clock frequency as we have on option to use double wide mode > > - cleanup > > > > V4: > > - renaming of max_dotclk as max_dotclk_freq in dev_priv (i915_drv.h) > > caused changes to all patches in my series even though some of them has > > been r-b'd by Ville > > - for consistency the max_pixclk variable is renamed as max_dotclk > throughout > > the whole series > > > > Mika Kahola (6): > > drm/i915: DisplayPort pixel clock check > > drm/i915: HDMI pixel clock check > > drm/i915: DisplayPort-MST pixel clock check > > drm/i915: SDVO pixel clock check > > drm/i915: CRT pixel clock check > > drm/i915: TV pixel clock check > > Note that the title of your series is wrong I think - mode_valid is _only_ > called > at probe time, to filter the list of modes userspace can see. > Userspace can still try to set a mode not in that list which would be above > that dotclk. From a quick look around I don't see that addressed anywhere. > Indeed, the title is a bit misleading as the mode is checked at probe time. > If that's true then I think we also need an igt which exercises this code by > trying to set a dotclock that's way too high (but for a resolution that the > display actually supports). Let's put this on my to-do list Cheers, Mika > -Daniel > > > > > > drivers/gpu/drm/i915/intel_crt.c| 4 > > drivers/gpu/drm/i915/intel_dp.c | 3 ++- > > drivers/gpu/drm/i915/intel_dp_mst.c | 5 + > > drivers/gpu/drm/i915/intel_hdmi.c | 8 > > drivers/gpu/drm/i915/intel_sdvo.c | 4 > > drivers/gpu/drm/i915/intel_tv.c | 4 > > 6 files changed, 27 insertions(+), 1 deletion(-) > > > > -- > > 1.9.1 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 07/12] drm/i915: GEM operations need to be done under the big lock
On Tue, Feb 02, 2016 at 02:46:19PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > VMA creation and GEM list management need the big lock. > > v2: > > Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall) > > Not to mention drm_gem_object_unreference was there in existing > code with no mutex held. > > v3: > > Some callers of i915_gem_object_create_stolen_for_preallocated > already hold the lock so move the mutex into the other caller > as well. > > v4: > > Changed to lockdep_assert_held. (Chris Wilson) > > Signed-off-by: Tvrtko Ursulin > Cc: Chris Wilson Also Reviewed-by: Chris Wilson :) Daniel, what did you think of having a config option to turn lockdep_assert_held into WARN_ON(!mutex_is_locked()) when lockdep is not fully-enabled? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Fix some invalid requests cancellations
On 29/01/16 16:49, Chris Wilson wrote: As we add the VMA to the request early, it may be cancelled during execbuf reservation. This will leave the context object pointing to a I don't get it, request is created after the reservation. dangling request; i915_wait_request() simply skips the wait and so we may unbind the object whilst it is still active. However, if at any point we make a change to the hardware (and equally importantly our bookkeeping in the driver), we cannot cancel the request as what has already been written must be submitted. Submitting a partial request is far easier than trying to unwind the incomplete change. Unfortunately this patch undoes the excess breadcrumb usage that olr prevented, e.g. if we interrupt batchbuffer submission then we submit the requests along with the memory writes and interrupt (even though we do no real work). Disassociating requests from breadcrumbs (and semaphores) is a topic for a past/future series, but now much more important. > v2: Rebase Note that igt/gem_concurrent_blit tiggers both misrendering and a GPF that is fixed by this patch. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93907 Testcase: igt/gem_concurrent_blit Signed-off-by: Chris WilsonCc: Daniel Vetter Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_drv.h| 1 - drivers/gpu/drm/i915/i915_gem.c| 7 ++- drivers/gpu/drm/i915/i915_gem_context.c| 21 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 +--- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_lrc.c | 1 - 6 files changed, 17 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a2d2f08b7515..f828a7ffed37 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2823,7 +2823,6 @@ int i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); void i915_gem_execbuffer_move_to_active(struct list_head *vmas, struct drm_i915_gem_request *req); -void i915_gem_execbuffer_retire_commands(struct i915_execbuffer_params *params); int i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, struct drm_i915_gem_execbuffer2 *args, struct list_head *vmas); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index e9b19bca1383..f764f33580fc 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3407,12 +3407,9 @@ int i915_gpu_idle(struct drm_device *dev) return PTR_ERR(req); ret = i915_switch_context(req); - if (ret) { - i915_gem_request_cancel(req); - return ret; - } - i915_add_request_no_flush(req); + if (ret) + return ret; } ret = intel_ring_idle(ring); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 83a097c94911..5da7adc3f7b2 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -652,7 +652,6 @@ static int do_switch(struct drm_i915_gem_request *req) struct drm_i915_private *dev_priv = ring->dev->dev_private; struct intel_context *from = ring->last_context; u32 hw_flags = 0; - bool uninitialized = false; int ret, i; if (from != NULL && ring == _priv->ring[RCS]) { @@ -759,6 +758,15 @@ static int do_switch(struct drm_i915_gem_request *req) to->remap_slice &= ~(1<legacy_hw_ctx.initialized) { + if (ring->init_context) { + ret = ring->init_context(req); + if (ret) + goto unpin_out; + } + to->legacy_hw_ctx.initialized = true; + } + /* The backing object for the context is done after switching to the * *next* context. Therefore we cannot retire the previous context until * the next context has already started running. In fact, the below code @@ -782,21 +790,10 @@ static int do_switch(struct drm_i915_gem_request *req) i915_gem_context_unreference(from); } - uninitialized = !to->legacy_hw_ctx.initialized; - to->legacy_hw_ctx.initialized = true; - done: i915_gem_context_reference(to); ring->last_context = to; - if (uninitialized) { - if (ring->init_context) { - ret = ring->init_context(req); - if (ret) -
[Intel-gfx] [PATCH 2/5] drm/i915: GEM operations need to be done under the big lock
From: Tvrtko UrsulinVMA creation and GEM list management need the big lock. v2: Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall) Not to mention drm_gem_object_unreference was there in existing code with no mutex held. v3: Some callers of i915_gem_object_create_stolen_for_preallocated already hold the lock so move the mutex into the other caller as well. v4: Changed to lockdep_assert_held. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_stolen.c | 2 ++ drivers/gpu/drm/i915/intel_display.c | 8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index ba1a00d815d3..feec0f80d8ef 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -638,6 +638,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (!drm_mm_initialized(_priv->mm.stolen)) return NULL; + lockdep_assert_held(>struct_mutex); + DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n", stolen_offset, gtt_offset, size); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 836bbdc239b6..57e465ed088b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2551,12 +2551,16 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc, if (size_aligned * 2 > dev_priv->gtt.stolen_usable_size) return false; + mutex_lock(>struct_mutex); + obj = i915_gem_object_create_stolen_for_preallocated(dev, base_aligned, base_aligned, size_aligned); - if (!obj) + if (!obj) { + mutex_unlock(>struct_mutex); return false; + } obj->tiling_mode = plane_config->tiling; if (obj->tiling_mode == I915_TILING_X) @@ -2569,12 +2573,12 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc, mode_cmd.modifier[0] = fb->modifier[0]; mode_cmd.flags = DRM_MODE_FB_MODIFIERS; - mutex_lock(>struct_mutex); if (intel_framebuffer_init(dev, to_intel_framebuffer(fb), _cmd, obj)) { DRM_DEBUG_KMS("intel fb init failed\n"); goto out_unref_obj; } + mutex_unlock(>struct_mutex); DRM_DEBUG_KMS("initial plane fb obj %p\n", obj); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: Fix struct mutex vs. RPS lock inversion
From: Tvrtko UrsulinRPS lock must be taken before the struct_mutex to avoid locking inversion. So stop grabbing it for the whole powersave initialization and instead only take it during the sections which need it. Also, struct_mutex is not needed any more since dedicated RPS lock was added in: commit 4fc688ce79772496503d22263d61b071a8fb596e Author: Jesse Barnes Date: Fri Nov 2 11:14:01 2012 -0700 drm/i915: protect RPS/RC6 related accesses (including PCU) with a new mutex Based on prototype patch by Chris Wilson and a subsequent mailing list discussion involving Ville, Imre, Chris and Daniel. v2: More details in the commit. v3: Use drm_gem_object_unreference_unlocked. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Cc: Ville Syrjälä Cc: Imre Deak Cc: Daniel Vetter Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 4 drivers/gpu/drm/i915/intel_pm.c | 9 - 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 57e465ed088b..a0f3e399a2d9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15976,9 +15976,7 @@ void intel_modeset_gem_init(struct drm_device *dev) struct drm_i915_gem_object *obj; int ret; - mutex_lock(>struct_mutex); intel_init_gt_powersave(dev); - mutex_unlock(>struct_mutex); intel_modeset_init_hw(dev); @@ -16058,9 +16056,7 @@ void intel_modeset_cleanup(struct drm_device *dev) intel_cleanup_overlay(dev); - mutex_lock(>struct_mutex); intel_cleanup_gt_powersave(dev); - mutex_unlock(>struct_mutex); intel_teardown_gmbus(dev); } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 379eabe093cb..8e869f183404 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5229,8 +5229,6 @@ static void cherryview_setup_pctx(struct drm_device *dev) u32 pcbr; int pctx_size = 32*1024; - WARN_ON(!mutex_is_locked(>struct_mutex)); - pcbr = I915_READ(VLV_PCBR); if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) { DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n"); @@ -5252,7 +5250,7 @@ static void valleyview_setup_pctx(struct drm_device *dev) u32 pcbr; int pctx_size = 24*1024; - WARN_ON(!mutex_is_locked(>struct_mutex)); + mutex_lock(>struct_mutex); pcbr = I915_READ(VLV_PCBR); if (pcbr) { @@ -5280,7 +5278,7 @@ static void valleyview_setup_pctx(struct drm_device *dev) pctx = i915_gem_object_create_stolen(dev, pctx_size); if (!pctx) { DRM_DEBUG("not enough stolen space for PCTX, disabling\n"); - return; + goto out; } pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start; @@ -5289,6 +5287,7 @@ static void valleyview_setup_pctx(struct drm_device *dev) out: DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR)); dev_priv->vlv_pctx = pctx; + mutex_unlock(>struct_mutex); } static void valleyview_cleanup_pctx(struct drm_device *dev) @@ -5298,7 +5297,7 @@ static void valleyview_cleanup_pctx(struct drm_device *dev) if (WARN_ON(!dev_priv->vlv_pctx)) return; - drm_gem_object_unreference(_priv->vlv_pctx->base); + drm_gem_object_unreference_unlocked(_priv->vlv_pctx->base); dev_priv->vlv_pctx = NULL; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915/guc: Do not wait for firmware load atomically
From: Tvrtko UrsulinIt does not look like this code needs to wait atomically? Higher in the call chain it calls the GEM API and I do not see that the section is under any spin locks or such. Signed-off-by: Tvrtko Ursulin Cc: Alex Dai Reviewed-by: Dave Gordon --- drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 3accd914490f..82a3c03fbc0e 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -199,7 +199,7 @@ static void set_guc_init_params(struct drm_i915_private *dev_priv) * the value matches either of two values representing completion * of the GuC boot process. * - * This is used for polling the GuC status in a wait_for_atomic() + * This is used for polling the GuC status in a wait_for() * loop below. */ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv, @@ -259,14 +259,14 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv) I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA)); /* -* Spin-wait for the DMA to complete & the GuC to start up. +* Wait for the DMA to complete & the GuC to start up. * NB: Docs recommend not using the interrupt for completion. * Measurements indicate this should take no more than 20ms, so a * timeout here indicates that the GuC has failed and is unusable. * (Higher levels of the driver will attempt to fall back to * execlist mode if this happens.) */ - ret = wait_for_atomic(guc_ucode_response(dev_priv, ), 100); + ret = wait_for(guc_ucode_response(dev_priv, ), 100); DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n", I915_READ(DMA_CTRL), status); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915/ilk: Move register read under spinlock
From: Tvrtko UrsulinCode does read-modify-write but the read was outside the lock. It is fine since the caller holds struct mutex, but if we correct this we open up the opportunity for decreasing the mutex duration time since the call to ironlake_enable_drps does not need it any longer since it is covered by the mchdev_lock lock. Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/intel_pm.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8e869f183404..b63cdb23b222 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4116,11 +4116,13 @@ bool ironlake_set_drps(struct drm_device *dev, u8 val) static void ironlake_enable_drps(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - u32 rgvmodectl = I915_READ(MEMMODECTL); + u32 rgvmodectl; u8 fmax, fmin, fstart, vstart; spin_lock_irq(_lock); + rgvmodectl = I915_READ(MEMMODECTL); + /* Enable temp reporting */ I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN); I915_WRITE16(TSC1, I915_READ(TSC1) | TSE); @@ -6240,8 +6242,8 @@ void intel_enable_gt_powersave(struct drm_device *dev) return; if (IS_IRONLAKE_M(dev)) { - mutex_lock(>struct_mutex); ironlake_enable_drps(dev); + mutex_lock(>struct_mutex); intel_init_emon(dev); mutex_unlock(>struct_mutex); } else if (INTEL_INFO(dev)->gen >= 6) { -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] Assorted reviewed patches for CI run
From: Tvrtko UrsulinAs the subject says, just to trigger a CI run for some assorted patches which are all already R-Bed. Tvrtko Ursulin (5): drm/i915: Use appropriate spinlock flavour drm/i915: GEM operations need to be done under the big lock drm/i915: Fix struct mutex vs. RPS lock inversion drm/i915/guc: Do not wait for firmware load atomically drm/i915/ilk: Move register read under spinlock drivers/gpu/drm/i915/i915_gem.c | 6 ++ drivers/gpu/drm/i915/i915_gem_stolen.c | 2 ++ drivers/gpu/drm/i915/intel_display.c| 12 ++-- drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++--- drivers/gpu/drm/i915/intel_pm.c | 15 --- 5 files changed, 21 insertions(+), 20 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Use appropriate spinlock flavour
From: Tvrtko UrsulinWe know this never runs from interrupt context so don't need to use the flags variant. Signed-off-by: Tvrtko Ursulin Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index de57e7f0be0f..ef53bb314639 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2970,11 +2970,9 @@ i915_gem_retire_requests(struct drm_device *dev) i915_gem_retire_requests_ring(ring); idle &= list_empty(>request_list); if (i915.enable_execlists) { - unsigned long flags; - - spin_lock_irqsave(>execlist_lock, flags); + spin_lock_irq(>execlist_lock); idle &= list_empty(>execlist_queue); - spin_unlock_irqrestore(>execlist_lock, flags); + spin_unlock_irq(>execlist_lock); intel_execlists_retire_requests(ring); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V3] drm/i915/skl: SKL CDCLK change on modeset tracking VCO
On Tue, Feb 09, 2016 at 04:28:27PM -0800, clinton.a.tay...@intel.com wrote: > From: Clint Taylor> > Track VCO frequency of SKL instead of the boot CDCLK and allow modeset > to set cdclk based on the max required pixel clock based on VCO > selected. > > The vco should be tracked at the atomic level and all CRTCs updated if > the required vco is changed. At this time the eDP pll is configured > inside the encoder which has no visibility into the atomic state. When > eDP v1.4 panel that require the 8640 vco are available this may need > to be investigated. > > V1: initial version > V2: add vco tracking in intel_dp_compute_config(), rename > skl_boot_cdclk. > V3: rebase, V2 feedback not possible as encoders are not aware of > atomic. > > Signed-off-by: Clint Taylor > Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= BTW I think we might want a patch for stable that just removes the max_cdclk setup for SKL/KBL (ie. just set max_cdclk=current cdclk). We'd put that in first, then put in the final version of this patch, and finally revert the first patch. Without that, backporting any of the DP/HDMI max_dotclock check patches to stable won't actually help SKL. Should we decide to backport those that is. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4
Ville, here is another dmesg: [1] I've reconnected HDMI cable three times. Forgot to note, it is HDMI monitor plugged into machine's DVI with HDMI-DVI cable. I guess this should matter as well. [1] https://gist.github.com/7057ea8512b9aa7ee5bd 11.02.2016 11:26, Ville Syrjälä написав: On Thu, Feb 11, 2016 at 10:54:08AM +0200, Oleksandr Natalenko wrote: Daniel, I've already tried Ville's patch you've mentioned with no luck. Kindly find unpatched v4.5-rc3 dmesg with drm debug enabled here: [1] [1] https://gist.github.com/efb44b7c6bc325978b80 That's an IVB. So no wonder my patch doesn't help. Can you grab another dmesg after disconnecting and reconnecting the HDMI cable? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4 2/3] lib/igt_core: Unify handling of slow/combinatorial tests
On Thu, Feb 11, 2016 at 01:09:33PM +0200, David Weinehall wrote: > +enum { > + /* The set of tests run if nothing else is specified */ > + SUBTEST_TYPE_NORMAL = 1 << 0, > + /* Basic Acceptance Testing set */ > + SUBTEST_TYPE_BASIC = 1 << 1, > + /* Tests that are very slow */ > + SUBTEST_TYPE_SLOW = 1 << 2, I still feel that slow isn't a useful descriminant for tests. Off the opt of my head, I would like HANG, SWAP, STRESS (this is too also undefined) that should cover the majority of the GEM cases. Looking at kms_flip should also offer a few categories. As for gem_concurrent_blit, I basically expect to have h->subtest_flags | p->subtest_flags | subtest_flags. I also expect QA to run the full gem_concurrent_blit anyway since it is showing up regressions from over a year ago. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix hpd live status bits for g4x
On Wed, Feb 10, 2016 at 07:59:05PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > Looks like g4x hpd live status bits actually agree with the spec. At > least they do on the machine I have, and apparently on Nick Bowler's > g4x as well. > > So gm45 may be the only platform where they don't agree. At least > that seems to be the case based on the (somewhat incomplete) > logs/dumps in [1], and Daniel has also tested this on his gm45 > sometime in the past. > > So let's change the bits to match the spec on g4x. That actually makes > the g4x bits identical to vlv/chv so we can just share the code > between those platforms, leaving gm45 as the special case. > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=52361 > > Cc: Shashank Sharma > Cc: Sonika Jindal > Cc: Daniel Vetter > Cc: Jani Nikula > Cc: Nick Bowler > References: > https://lists.freedesktop.org/archives/dri-devel/2016-February/100382.html > Reported-by: Nick Bowler > Cc: sta...@vger.kernel.org > Fixes: 237ed86c693d ("drm/i915: Check live status before reading edid") > Signed-off-by: Ville Syrjälä Yeah I'm hopeful this will work. Reviewed-by: Daniel Vetter Since CI is down and this is super restricted impact and fixing a regression I'm voting that we'll pick it up right away. Jani, are you ok with that? -Daniel > --- > drivers/gpu/drm/i915/i915_reg.h | 15 --- > drivers/gpu/drm/i915/intel_dp.c | 14 +++--- > 2 files changed, 15 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 188ad5de020f..678faa957e75 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3296,19 +3296,20 @@ enum skl_disp_power_wells { > > #define PORT_HOTPLUG_STAT_MMIO(dev_priv->info.display_mmio_offset + > 0x61114) > /* > - * HDMI/DP bits are gen4+ > + * HDMI/DP bits are g4x+ > * > * WARNING: Bspec for hpd status bits on gen4 seems to be completely > confused. > * Please check the detailed lore in the commit message for for experimental > * evidence. > */ > -#define PORTD_HOTPLUG_LIVE_STATUS_G4X (1 << 29) > +/* Bspec says GM45 should match G4X/VLV/CHV, but reality disagrees */ > +#define PORTD_HOTPLUG_LIVE_STATUS_GM45 (1 << 29) > +#define PORTC_HOTPLUG_LIVE_STATUS_GM45 (1 << 28) > +#define PORTB_HOTPLUG_LIVE_STATUS_GM45 (1 << 27) > +/* G4X/VLV/CHV DP/HDMI bits again match Bspec */ > +#define PORTD_HOTPLUG_LIVE_STATUS_G4X (1 << 27) > #define PORTC_HOTPLUG_LIVE_STATUS_G4X (1 << 28) > -#define PORTB_HOTPLUG_LIVE_STATUS_G4X (1 << 27) > -/* VLV DP/HDMI bits again match Bspec */ > -#define PORTD_HOTPLUG_LIVE_STATUS_VLV (1 << 27) > -#define PORTC_HOTPLUG_LIVE_STATUS_VLV (1 << 28) > -#define PORTB_HOTPLUG_LIVE_STATUS_VLV (1 << 29) > +#define PORTB_HOTPLUG_LIVE_STATUS_G4X (1 << 29) > #define PORTD_HOTPLUG_INT_STATUS (3 << 21) > #define PORTD_HOTPLUG_INT_LONG_PULSE (2 << 21) > #define PORTD_HOTPLUG_INT_SHORT_PULSE (1 << 21) > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index a073f04a5330..bbe18996efe6 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4490,20 +4490,20 @@ static bool g4x_digital_port_connected(struct > drm_i915_private *dev_priv, > return I915_READ(PORT_HOTPLUG_STAT) & bit; > } > > -static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv, > -struct intel_digital_port *port) > +static bool gm45_digital_port_connected(struct drm_i915_private *dev_priv, > + struct intel_digital_port *port) > { > u32 bit; > > switch (port->port) { > case PORT_B: > - bit = PORTB_HOTPLUG_LIVE_STATUS_VLV; > + bit = PORTB_HOTPLUG_LIVE_STATUS_GM45; > break; > case PORT_C: > - bit = PORTC_HOTPLUG_LIVE_STATUS_VLV; > + bit = PORTC_HOTPLUG_LIVE_STATUS_GM45; > break; > case PORT_D: > - bit = PORTD_HOTPLUG_LIVE_STATUS_VLV; > + bit = PORTD_HOTPLUG_LIVE_STATUS_GM45; > break; > default: > MISSING_CASE(port->port); > @@ -4555,8 +4555,8 @@ bool intel_digital_port_connected(struct > drm_i915_private *dev_priv, > return cpt_digital_port_connected(dev_priv, port); > else if (IS_BROXTON(dev_priv)) > return bxt_digital_port_connected(dev_priv, port); > - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - return
[Intel-gfx] [RFC] drm/i915/guc: Keep the previous context pinned until the next one has been completed
From: Tvrtko UrsulinIn GuC mode submitting requests is only putting them into the GuC queue with the actual submission to hardware following at some future point. This makes the per engine last context tracking insufficient for closing the premature context unpin race. Instead we need to make requests track and pin the previous context on the same engine, and only unpin them when they themselves are retired. This will ensure context remain pinned until the are fully saved by the GPU in all cases. This patch adds overhead to the request retire path which could be avoided by refactoring LRC pinning to make it track the VMA and cache the kmap in the VMA as suggested by Chris Wilson. It is also not tested in GuC mode at all so just speculative. Signed-off-by: Tvrtko Ursulin Cc: Alex Dai Cc: Dave Gordon Cc: Nick Hoath Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/i915_gem.c | 5 + drivers/gpu/drm/i915/intel_lrc.c | 15 +++ 3 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e11eef1e5134..090172de81a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2313,6 +2313,12 @@ struct drm_i915_gem_request { /** Execlists no. of times this request has been sent to the ELSP */ int elsp_submitted; + /** +* Context of the request precedeing this one on the same engine. +* Can be NULL. +*/ + struct intel_context *previous_ctx; + }; struct drm_i915_gem_request * __must_check diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ef53bb314639..c63c3072a8a9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1428,6 +1428,11 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) list_del_init(>list); i915_gem_request_remove_from_client(request); + if (request->previous_ctx) { + intel_lr_context_unpin(request->previous_ctx, request->ring); + request->previous_ctx = NULL; + } + i915_gem_request_unreference(request); } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 3a03646e343d..89eb892df4ae 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -783,6 +783,21 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) if (intel_ring_stopped(engine)) return 0; + /* +* Pin the previous context on this engine to ensure it is not +* prematurely unpinned in GuC mode. +* Previous context will be unpinned when this request is retired, +* ensuring the GPU has switched out from the previous context and into +* a new context at that point. +*/ + if (i915.enable_guc_submission && engine->last_context) { + request->previous_ctx = engine->last_context; + intel_lr_context_pin(request->previous_ctx, engine); + } + + /* +* Track and pin the last context submitted on an engine. +*/ if (engine->last_context != request->ctx) { if (engine->last_context) intel_lr_context_unpin(engine->last_context, engine); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] agp/intel-gtt: Only register fake agp driver for gen1
On Thu, Feb 11, 2016 at 12:31:45PM +0200, Ville Syrjälä wrote: > On Wed, Jan 27, 2016 at 02:38:00PM +0100, Daniel Vetter wrote: > > The fake agp driver for the intel graphics gart is only needed for ums > > support. And we ditched that a long time ago: > > > > commit 03dae59c72d8ef6e005f48ba356c863e0587 > > Author: Daniel Vetter> > Date: Wed Jul 23 16:27:25 2014 +0200 > > > > drm/i915: Ditch UMS config option > > > > With this there's no longer the problem that 2 drivers (fake agp > > driver and the drm/i915 driver) fight over the same piece, which fixes > > apparent dma leaks detected by CONFIG_DMA_API_DEBUG. > > > > Note that the leak isn't real since intel-gtt refcounts and will tear > > down eventually. But the debug code assumes that when the i915 driver > > unbinds from the pci device everything should be gone. Which isn't the > > case if we have intel-agp enabled - userspace might need it. But by > > ditching this intel-gtt setup and teardown is completely tied to the > > livetime of the "real" driver. > > > > While at it untangle the init ordering a bit - the fake agp wouldn't > > be initialized correctly if i915.ko loads first. Which isn't a problem > > since when i915 loads in kms mode you won't need the fake agp support > > needed by the ums driver ... > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93793 > > Signed-off-by: Daniel Vetter > > --- > > drivers/char/agp/intel-gtt.c | 24 > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > > index e657f989745e..aef87fdbd187 100644 > > --- a/drivers/char/agp/intel-gtt.c > > +++ b/drivers/char/agp/intel-gtt.c > > @@ -1348,16 +1348,6 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, > > struct pci_dev *gpu_pdev, > > { > > int i, mask; > > > > - /* > > -* Can be called from the fake agp driver but also directly from > > -* drm/i915.ko. Hence we need to check whether everything is set up > > -* already. > > -*/ > > - if (intel_private.driver) { > > - intel_private.refcount++; > > - return 1; > > - } > > - > > for (i = 0; intel_gtt_chipsets[i].name != NULL; i++) { > > if (gpu_pdev) { > > if (gpu_pdev->device == > > @@ -1378,16 +1368,26 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, > > struct pci_dev *gpu_pdev, > > if (!intel_private.driver) > > return 0; > > > > - intel_private.refcount++; > > - > > #if IS_ENABLED(CONFIG_AGP_INTEL) > > if (bridge) { > > + if (INTEL_GTT_GEN > 1) > > + return 0; > > So if this happens first, we set up intel_private.driver but leave the > refcount at 0. Then i915 loads and we set up intel_private.driver again, > and bump the refcount to 0. Well, we should end up pointing > intel_privatee.driver at the same thing both times so not really a > problem I suppose. > > So yeah, I think this ought to work > Reviewed-by: Ville Syrjälä Thanks for the review, applied. > I guess we could also trim the gen>=2 gmch pci ids from the agp > driver's pci id table, to avoid even probing it. Hm yeah, forgot about that. I'll try to remember and do a follow-up patch. -Daniel > > > + > > bridge->driver = _fake_agp_driver; > > bridge->dev_private_data = _private; > > bridge->dev = bridge_pdev; > > } > > #endif > > > > + > > + /* > > +* Can be called from the fake agp driver but also directly from > > +* drm/i915.ko. Hence we need to check whether everything is set up > > +* already. > > +*/ > > + if (intel_private.refcount++) > > + return 1; > > + > > intel_private.bridge_dev = pci_dev_get(bridge_pdev); > > > > dev_info(_pdev->dev, "Intel %s Chipset\n", > > intel_gtt_chipsets[i].name); > > -- > > 2.5.0 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Add support for mapping an object page by page
On 04/02/16 09:30, ankitprasad.r.sha...@intel.com wrote: From: Chris WilsonIntroduced a new vm specfic callback insert_page() to program a single pte in ggtt or ppgtt. This allows us to map a single page in to the mappable aperture space. This can be iterated over to access the whole object by using space as meagre as page size. v2: Added low level rpm assertions to insert_page routines (Chris) Signed-off-by: Chris Wilson Signed-off-by: Ankitprasad Sharma --- drivers/char/agp/intel-gtt.c| 9 + drivers/gpu/drm/i915/i915_gem_gtt.c | 65 + drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +++ include/drm/intel-gtt.h | 3 ++ 4 files changed, 82 insertions(+) diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index 1341a94..7c68576 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -838,6 +838,15 @@ static bool i830_check_flags(unsigned int flags) return false; } +void intel_gtt_insert_page(dma_addr_t addr, + unsigned int pg, + unsigned int flags) +{ + intel_private.driver->write_entry(addr, pg, flags); + wmb(); +} +EXPORT_SYMBOL(intel_gtt_insert_page); + void intel_gtt_insert_sg_entries(struct sg_table *st, unsigned int pg_start, unsigned int flags) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 715a771..a64018f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2341,6 +2341,28 @@ static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) #endif } +static void gen8_ggtt_insert_page(struct i915_address_space *vm, + dma_addr_t addr, + uint64_t offset, + enum i915_cache_level level, + u32 unused) +{ + struct drm_i915_private *dev_priv = to_i915(vm->dev); + gen8_pte_t __iomem *pte = + (gen8_pte_t __iomem *)dev_priv->gtt.gsm + + (offset >> PAGE_SHIFT); + int rpm_atomic_seq; + + rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv); + + gen8_set_pte(pte, gen8_pte_encode(addr, level, true)); + wmb(); gen8_ggtt_insert_entries does a read-back of the PTE after having written it with a big fat comment talking about how that could be really important. This is not needed in this path? + + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN); Why is the posting read not required here as in gen8_ggtt_insert_entries? + + assert_rpm_atomic_end(dev_priv, rpm_atomic_seq); +} + static void gen8_ggtt_insert_entries(struct i915_address_space *vm, struct sg_table *st, uint64_t start, @@ -2412,6 +2434,28 @@ static void gen8_ggtt_insert_entries__BKL(struct i915_address_space *vm, stop_machine(gen8_ggtt_insert_entries__cb, , NULL); } +static void gen6_ggtt_insert_page(struct i915_address_space *vm, + dma_addr_t addr, + uint64_t offset, + enum i915_cache_level level, + u32 flags) +{ + struct drm_i915_private *dev_priv = to_i915(vm->dev); + gen6_pte_t __iomem *pte = + (gen6_pte_t __iomem *)dev_priv->gtt.gsm + + (offset >> PAGE_SHIFT); + int rpm_atomic_seq; + + rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv); + + iowrite32(vm->pte_encode(addr, level, true, flags), pte); + wmb(); + + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN); + + assert_rpm_atomic_end(dev_priv, rpm_atomic_seq); +} Same questions as for the gen8 version. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 136/190] drm/i915: Move ioremap_wc tracking onto VMA
On 11/01/16 10:45, Chris Wilson wrote: By tracking the iomapping on the VMA itself, we can share that area between multiple users. Also by only revoking the iomapping upon unbinding from the mappable portion of the GGTT, we can keep that iomap across multiple invocations (e.g. execlists context pinning). Between the lines and from some IRC discussion it seems the goal of this is to fix an address space memory leak with fbcon? But I don't know fbdev so can't find who would do the unpin on the VMA to allow unbind to eventually unmap it? Regards, Tvrtko Signed-off-by: Chris Wilson--- drivers/gpu/drm/i915/i915_gem.c | 5 + drivers/gpu/drm/i915/i915_gem_gtt.c | 33 + drivers/gpu/drm/i915/i915_gem_gtt.h | 4 drivers/gpu/drm/i915/intel_fbdev.c | 8 +++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0c4e8e1aeeff..5bb21b20c36a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2699,6 +2699,11 @@ int i915_vma_unbind(struct i915_vma *vma) if (ret) return ret; + if (vma->iomap) { + iounmap(vma->iomap); + vma->iomap = NULL; + } + vma->map_and_fenceable = false; } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index b8af904ad12c..3fcf2fd73453 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3575,3 +3575,36 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, return 0; } + +void *i915_vma_iomap(struct drm_i915_private *dev_priv, +struct i915_vma *vma) +{ + if (WARN_ON(!vma->map_and_fenceable)) + return ERR_PTR(-ENODEV); + + GEM_BUG_ON(!vma->is_ggtt); + GEM_BUG_ON((vma->bound & GLOBAL_BIND) == 0); + + if (vma->iomap == NULL) { + u32 base = dev_priv->gtt.mappable_base + vma->node.start; + void *ptr; + + ptr = ioremap_wc(base, vma->size); + if (ptr == NULL) { + int ret; + + /* Too many areas already allocated? */ + ret = i915_gem_evict_vm(vma->vm, true); + if (ret) + return ERR_PTR(ret); + + ptr = ioremap_wc(base, vma->size); + if (ptr == NULL) + return ERR_PTR(-ENOMEM); + } + + vma->iomap = ptr; + } + + return vma->iomap; +} diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 6b0f557982d5..0e0570e13a68 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -181,6 +181,7 @@ struct i915_vma { struct drm_mm_node node; struct drm_i915_gem_object *obj; struct i915_address_space *vm; + void *iomap; u64 size; struct i915_gem_active last_read[I915_NUM_RINGS]; @@ -579,4 +580,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev); int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj); + +void *i915_vma_iomap(struct drm_i915_private *dev_priv, +struct i915_vma *vma); #endif diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 7decbca25dbb..8e7c341951fd 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -248,12 +248,10 @@ static int intelfb_create(struct drm_fb_helper *helper, info->fix.smem_start = dev->mode_config.fb_base + vma->node.start; info->fix.smem_len = vma->node.size; - info->screen_base = - ioremap_wc(dev_priv->gtt.mappable_base + vma->node.start, - vma->node.size); - if (!info->screen_base) { + info->screen_base = i915_vma_iomap(dev_priv, vma); + if (IS_ERR(info->screen_base)) { DRM_ERROR("Failed to remap framebuffer into virtual memory\n"); - ret = -ENOSPC; + ret = PTR_ERR(info->screen_base); goto out_destroy_fbi; } info->screen_size = vma->node.size; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 136/190] drm/i915: Move ioremap_wc tracking onto VMA
On 11/02/16 13:29, Chris Wilson wrote: On Thu, Feb 11, 2016 at 01:20:46PM +, Tvrtko Ursulin wrote: On 11/01/16 10:45, Chris Wilson wrote: By tracking the iomapping on the VMA itself, we can share that area between multiple users. Also by only revoking the iomapping upon unbinding from the mappable portion of the GGTT, we can keep that iomap across multiple invocations (e.g. execlists context pinning). Between the lines and from some IRC discussion it seems the goal of this is to fix an address space memory leak with fbcon? The goal is to prevent an issue from hastily dropping iomappings (and vmappings elsewhere) when unpinning contexts. That comes into play when we track the ring->vma (not just track ring->vma as we do now, but can rely on the vma being persistent). Fixing a leak on driver unload is an interesting side-effect. But I don't know fbdev so can't find who would do the unpin on the VMA to allow unbind to eventually unmap it? That actually gets fixed in another patch when we teach intel_fbdev to actually track the VMA it allocates, as right now we deliberately leak it to keep the code simple. Ok in that case ack on the patch. It will need to assert on VMA being pinned I think and some minor changes when you rebase it on top of nightly as standalone so I can properly review it then. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 136/190] drm/i915: Move ioremap_wc tracking onto VMA
On Thu, Feb 11, 2016 at 01:20:46PM +, Tvrtko Ursulin wrote: > > > On 11/01/16 10:45, Chris Wilson wrote: > >By tracking the iomapping on the VMA itself, we can share that area > >between multiple users. Also by only revoking the iomapping upon > >unbinding from the mappable portion of the GGTT, we can keep that iomap > >across multiple invocations (e.g. execlists context pinning). > > Between the lines and from some IRC discussion it seems the goal of > this is to fix an address space memory leak with fbcon? The goal is to prevent an issue from hastily dropping iomappings (and vmappings elsewhere) when unpinning contexts. That comes into play when we track the ring->vma (not just track ring->vma as we do now, but can rely on the vma being persistent). Fixing a leak on driver unload is an interesting side-effect. > But I don't know fbdev so can't find who would do the unpin on the > VMA to allow unbind to eventually unmap it? That actually gets fixed in another patch when we teach intel_fbdev to actually track the VMA it allocates, as right now we deliberately leak it to keep the code simple. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/6] drm/i915: tidy up initialisation failure paths (legacy)
On Mon, Feb 01, 2016 at 09:38:02AM +, Dave Gordon wrote: > On 30/01/16 10:50, Chris Wilson wrote: > >On Fri, Jan 29, 2016 at 07:19:26PM +, Dave Gordon wrote: > >>1. Fix intel_cleanup_ring_buffer() to handle the error cleanup > >>case where the ringbuffer has been allocated but map-and-pin > >>failed. Unpin it iff it's previously been mapped-and-pinned. > >> > >>2. Fix the error path in intel_init_ring_buffer(), which already > >>called intel_destroy_ringbuffer_obj(), but failed to free the > >>actual ringbuffer structure. Calling intel_ringbuffer_free() > >>instead does both in one go. > >> > >>3. With the above change, intel_destroy_ringbuffer_obj() is only > >>called in one place (intel_ringbuffer_free()), so flatten it > >>into that function. > >> > >>4. move low-level register accesses from intel_cleanup_ring_buffer() > >>(which calls intel_stop_ring_buffer(ring) which calls stop_ring()) > >>down into stop_ring() itself), which is already doing low-level > >>register accesses. Then, intel_cleanup_ring_buffer() no longer > >>needs 'dev_priv'. > >> > >>v3: > >>Make intel_unpin_ringbuffer_obj() return early if ringbuffer is > >>not mapped, simplifying matters for the caller. [Chris Wilson, > >>Daniel Vetter] > >>Don't bother to clear a pointer in an object about to be freed. > >>[Chris Wilson] > >> > >>Signed-off-by: Dave Gordon> >>Cc: Chris Wilson > >>--- > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 49 > >> - > >> 1 file changed, 23 insertions(+), 26 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>index 6f5b511..db02893 100644 > >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>@@ -551,6 +551,8 @@ static bool stop_ring(struct intel_engine_cs *ring) > >>I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING)); > >>} > >> > >>+ WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0); > > > >This warn was guarding the release of the backing pages, to verify that > >we had stopped the rings first. > > It's still effectively at the same point in the execution sequence, > just not intruding into a function that has no business peeking at > registers. It's now at the end of the called subfunctions, rather > than just after the call at the top-level, where it was a layering > violation. Nope, semantically it is an important difference and the assertion that the hardware is not reading from the object as we return the pages back to the system is very, very important documentation. > Or we could put it in a separate function, and say > WARN_ON(!ring_is_idle(ring)); > > >>+ > >>return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0; > >> } > >> > >>@@ -2055,6 +2057,9 @@ static int init_phys_status_page(struct > >>intel_engine_cs *ring) > >> > >> void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) > >> { > >>+ if (!ringbuf->virtual_start) > >>+ return; > >>+ > >>if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) > >>vunmap(ringbuf->virtual_start); > >>else > >>@@ -2132,12 +2137,6 @@ int intel_pin_and_map_ringbuffer_obj(struct > >>drm_device *dev, > >>return 0; > >> } > >> > >>-static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf) > >>-{ > >>- drm_gem_object_unreference(>obj->base); > >>- ringbuf->obj = NULL; > >>-} > >>- > >> static int intel_alloc_ringbuffer_obj(struct drm_device *dev, > >> struct intel_ringbuffer *ringbuf) > >> { > >>@@ -2200,11 +2199,13 @@ struct intel_ringbuffer * > >> } > >> > >> void > >>-intel_ringbuffer_free(struct intel_ringbuffer *ring) > >>+intel_ringbuffer_free(struct intel_ringbuffer *ringbuf) > >> { > >>- intel_destroy_ringbuffer_obj(ring); > >>- list_del(>link); > >>- kfree(ring); > >>+ if (ringbuf->obj) > >>+ drm_gem_object_unreference(>obj->base); > > > >No, let's not add duplicate code. If you worry, > >replace to_intel_bo() with > > I suspect the above doesn't actually duplicate the test, 'cos > drm_gem_object_unreference() is inline, and common subexpression > elimination should determine that ringbuf->obj and > >obj->base are the same thing and then the compiler can > collapse the explicit test here with the one inside > drm_gem_object_unreference(). So why did you write it? Since it clearly duplicates the existing code, the only reason I could think of was if you did not trust >obj == >obj->base. > >static inline struct drm_i915_gem_object * > >to_intel_bo(struct drm_gem_object *gem_obj) > >{ > > BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base)); > > return container_of(gem_obj, struct drm_i915_gem_object, base); > >} > > I would quite like to get rid of the "&(...)->base" that
Re: [Intel-gfx] [PATCH v4 6/6] drm/i915: fix context/engine cleanup order
On Sat, Jan 30, 2016 at 11:17:07AM +, Chris Wilson wrote: > On Fri, Jan 29, 2016 at 07:19:31PM +, Dave Gordon wrote: > > From: Nick Hoath> > > > Swap the order of context & engine cleanup, so that contexts are cleaned > > up first, and *then* engines. This is a more sensible order anyway, but > > in particular has become necessary since the 'intel_ring_initialized() > > must be simple and inline' patch, which now uses ring->dev as an > > 'initialised' flag, so it can now be NULL after engine teardown. This in > > turn can cause a problem in the context code, which (used to) check the > > ring->dev->struct_mutex -- causing a fault if ring->dev was NULL. > > > > Also rename the cleanup function to reflect what it actually does > > (cleanup engines, not a ringbuffer), and fix an annoying whitespace > > issue. > > > > v2: Also make the fix in i915_load_modeset_init, not just in > > i915_driver_unload (Chris Wilson) > > v3: Had extra stuff in it. > > v4: Reverted extra stuff (so we're back to v2). > > Rebased and updated commentary above (Dave Gordon). > > > > Signed-off-by: Nick Hoath > > Signed-off-by: David Gordon > > Reviewed-by: Chris Wilson > > Have to drop that, with the recent context changes. > > You have to move the gpu-reset now for execlists. > > Basically pull it out into: > > static void i915_unload_gem(struct drm_device *dev) > { >/* > * Neither the BIOS, ourselves or any other kernel > * expects the system to be in execlists mode on startup, > * so we need to reset the GPU back to legacy mode. And the only > * known way to disable logical contexts is through a GPU reset. > * > * So in order to leave the system in a known default configration, > * always reset the GPU upon unload. This also cleans up the GEM > * state tracking, flushing off the requests and leaving the system > * idle. > * > * Note that is of the upmost importance that the GPU is idle and > * all stray writes are flushed *before* we dismantle the backing > * storage for the pinned objects. > */ >i915_reset(dev); > >mutex_lock(>struct_mutex); >i915_gem_context_fini(dev); >i915_gem_cleanup_ringbuffer(dev); >mutex_unlock(>struct_mutex); > } > > and then kill the intel_gpu_reset along both the cleanup pathsh It appears this patch was applied without dropping my r-b for the issue I pointed out above. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION] i915: No HDMI output with 4.4
On Thu, Feb 11, 2016 at 01:16:53PM +0200, Oleksandr Natalenko wrote: > Ville, > > here is another dmesg: [1] > > I've reconnected HDMI cable three times. > > Forgot to note, it is HDMI monitor plugged into machine's DVI with > HDMI-DVI cable. I guess this should matter as well. Shouldn't really matter. HDMI and DVI are identical at this level. > > [1] https://gist.github.com/7057ea8512b9aa7ee5bd OK, so the hpd interrupt does happen, and yet the live status supposedly claims that nothing is there. Port C live status definitely works here on my IVB, so not sure what the deal is. Can you grab intel-gpu-tools and run intel_reg read 0xc4000 0xc4004 0xc4008 0xc400c 0xc4030 a couple of times after plugging the monitor in, and also run it when nothing is plugged in. Also you could try something like the following patch so we might observe the live status with a bit more detail. Though the fact that it doesn't seem to work for you even when the monitor was already plugged in is somewhat troubling: --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1392,12 +1392,17 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - for (try = 0; !live_status && try < 9; try++) { - if (try) - msleep(10); - live_status = intel_digital_port_connected(dev_priv, + printk("port %c live status\n ", port_name(hdmi_to_dig_port(intel_hdmi)->port)); + for (try = 0; try < 250; try++) { + bool status = intel_digital_port_connected(dev_priv, hdmi_to_dig_port(intel_hdmi)); + live_status |= status; + printk("%c", status ? '#' : '_'); + if (try % 50 == 49) + printk("\n "); + usleep_range(1000, 1000); } + printk("\n"); if (!live_status) DRM_DEBUG_KMS("Live status not up!"); -- 2.4.10 Oh, and if you have another cable you can try, might be a good idea to see if it behaves any better. > > 11.02.2016 11:26, Ville Syrjälä написав: > > On Thu, Feb 11, 2016 at 10:54:08AM +0200, Oleksandr Natalenko wrote: > >> Daniel, > >> > >> I've already tried Ville's patch you've mentioned with no luck. > >> > >> Kindly find unpatched v4.5-rc3 dmesg with drm debug enabled here: [1] > >> > >> [1] https://gist.github.com/efb44b7c6bc325978b80 > > > > That's an IVB. So no wonder my patch doesn't help. > > > > Can you grab another dmesg after disconnecting and reconnecting the > > HDMI cable? -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4 2/3] lib/igt_core: Unify handling of slow/combinatorial tests
On Thu, Feb 11, 2016 at 01:04:14PM +, Chris Wilson wrote: > On Thu, Feb 11, 2016 at 01:09:33PM +0200, David Weinehall wrote: > > +enum { > > + /* The set of tests run if nothing else is specified */ > > + SUBTEST_TYPE_NORMAL = 1 << 0, > > + /* Basic Acceptance Testing set */ > > + SUBTEST_TYPE_BASIC = 1 << 1, > > + /* Tests that are very slow */ > > + SUBTEST_TYPE_SLOW = 1 << 2, > > I still feel that slow isn't a useful descriminant for tests. Off the > opt of my head, I would like > > HANG, > SWAP, > STRESS (this is too also undefined) > > that should cover the majority of the GEM cases. Looking at kms_flip > should also offer a few categories. As I said the first time I posted this patch, I'm open to both alternative names and help with categorising the subtests. > As for gem_concurrent_blit, I basically expect to > have h->subtest_flags | p->subtest_flags | subtest_flags. > > I also expect QA to run the full gem_concurrent_blit anyway since it is > showing up regressions from over a year ago. Yeah, I did a partial run of the full set yesterday and spotted about 150 failed tests within an hour of so. After that I had to abort the test to continue with other work on that machine. But it's fairly obvious that the limited set is a lot better tested. Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Hide GEM shutdown in i915_gem_fini
On Thu, Feb 11, 2016 at 03:39:25PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > As we have i915_gem_init, do the reverse in new i915_gem_fini so > the fragile order of doing things is hidden from the outside and > only at one place. > > Signed-off-by: Tvrtko Ursulin > Cc: Nick Hoath > Cc: David Gordon > Cc: Chris Wilson > Cc: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_dma.c | 6 ++ > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_gem.c | 40 > 3 files changed, 27 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 2df2fac04708..beda7dea6814 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -444,8 +444,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > > cleanup_gem: > mutex_lock(>struct_mutex); > - i915_gem_context_fini(dev); > - i915_gem_cleanup_engines(dev); > + i915_gem_fini(dev); > mutex_unlock(>struct_mutex); > cleanup_irq: > intel_guc_ucode_fini(dev); > @@ -1256,8 +1255,7 @@ int i915_driver_unload(struct drm_device *dev) > > intel_guc_ucode_fini(dev); > mutex_lock(>struct_mutex); > - i915_gem_context_fini(dev); > - i915_gem_cleanup_engines(dev); > + i915_gem_fini(dev); > mutex_unlock(>struct_mutex); > intel_fbc_cleanup_cfb(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 090172de81a0..d004dc806670 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3060,11 +3060,11 @@ static inline bool i915_stop_ring_allow_warn(struct > drm_i915_private *dev_priv) > void i915_gem_reset(struct drm_device *dev); > bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force); > int __must_check i915_gem_init(struct drm_device *dev); > +void __must_check i915_gem_fini(struct drm_device *dev); > int i915_gem_init_rings(struct drm_device *dev); > int __must_check i915_gem_init_hw(struct drm_device *dev); > int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice); > void i915_gem_init_swizzling(struct drm_device *dev); > -void i915_gem_cleanup_engines(struct drm_device *dev); > int __must_check i915_gpu_idle(struct drm_device *dev); > int __must_check i915_gem_suspend(struct drm_device *dev); > void __i915_add_request(struct drm_i915_gem_request *req, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c63c3072a8a9..f7e7ab432341 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4834,6 +4834,26 @@ cleanup_render_ring: > return ret; > } > > +static void > +i915_gem_cleanup_engines(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_engine_cs *ring; > + int i; > + > + for_each_ring(ring, dev_priv, i) > + dev_priv->gt.cleanup_ring(ring); > + > + if (i915.enable_execlists) { > + /* > + * Neither the BIOS, ourselves or any other kernel > + * expects the system to be in execlists mode on startup, > + * so we need to reset the GPU back to legacy mode. > + */ > + intel_gpu_reset(dev); > + } This needs to be before contex-fini. If you move this outside of the mutex, we can then use i915_gpu_reset() and do the full state cleanup. Similarly that also moves the mutex handling inside i915_gem_fini as well. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Hide GEM shutdown in i915_gem_fini
From: Tvrtko UrsulinAs we have i915_gem_init, do the reverse in new i915_gem_fini so the fragile order of doing things is hidden from the outside and only at one place. Signed-off-by: Tvrtko Ursulin Cc: Nick Hoath Cc: David Gordon Cc: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_dma.c | 6 ++ drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 40 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2df2fac04708..beda7dea6814 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -444,8 +444,7 @@ static int i915_load_modeset_init(struct drm_device *dev) cleanup_gem: mutex_lock(>struct_mutex); - i915_gem_context_fini(dev); - i915_gem_cleanup_engines(dev); + i915_gem_fini(dev); mutex_unlock(>struct_mutex); cleanup_irq: intel_guc_ucode_fini(dev); @@ -1256,8 +1255,7 @@ int i915_driver_unload(struct drm_device *dev) intel_guc_ucode_fini(dev); mutex_lock(>struct_mutex); - i915_gem_context_fini(dev); - i915_gem_cleanup_engines(dev); + i915_gem_fini(dev); mutex_unlock(>struct_mutex); intel_fbc_cleanup_cfb(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 090172de81a0..d004dc806670 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3060,11 +3060,11 @@ static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv) void i915_gem_reset(struct drm_device *dev); bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force); int __must_check i915_gem_init(struct drm_device *dev); +void __must_check i915_gem_fini(struct drm_device *dev); int i915_gem_init_rings(struct drm_device *dev); int __must_check i915_gem_init_hw(struct drm_device *dev); int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice); void i915_gem_init_swizzling(struct drm_device *dev); -void i915_gem_cleanup_engines(struct drm_device *dev); int __must_check i915_gpu_idle(struct drm_device *dev); int __must_check i915_gem_suspend(struct drm_device *dev); void __i915_add_request(struct drm_i915_gem_request *req, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c63c3072a8a9..f7e7ab432341 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4834,6 +4834,26 @@ cleanup_render_ring: return ret; } +static void +i915_gem_cleanup_engines(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_engine_cs *ring; + int i; + + for_each_ring(ring, dev_priv, i) + dev_priv->gt.cleanup_ring(ring); + + if (i915.enable_execlists) { + /* +* Neither the BIOS, ourselves or any other kernel +* expects the system to be in execlists mode on startup, +* so we need to reset the GPU back to legacy mode. +*/ + intel_gpu_reset(dev); + } +} + int i915_gem_init_hw(struct drm_device *dev) { @@ -5011,24 +5031,12 @@ out_unlock: return ret; } -void -i915_gem_cleanup_engines(struct drm_device *dev) +void i915_gem_fini(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_engine_cs *ring; - int i; - - for_each_ring(ring, dev_priv, i) - dev_priv->gt.cleanup_ring(ring); + lockdep_assert_held(>struct_mutex); - if (i915.enable_execlists) { - /* -* Neither the BIOS, ourselves or any other kernel -* expects the system to be in execlists mode on startup, -* so we need to reset the GPU back to legacy mode. -*/ - intel_gpu_reset(dev); - } + i915_gem_context_fini(dev); + i915_gem_cleanup_engines(dev); } static void -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)
On Thu, Feb 11, 2016 at 08:05:26AM -0800, Matt Roper wrote: > On Thu, Feb 11, 2016 at 12:00:50PM +0200, Ville Syrjälä wrote: > > On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote: > > > Gen9 platforms allow CRTC's to be programmed with a background/canvas > > > color below the programmable planes. Let's expose this as a property to > > > allow userspace to program a desired value. > > > > > > This patch is based on earlier work by Chandra Konduru; unfortunately > > > the driver has evolved so much since his patches were written (in the > > > pre-atomic era) that the functionality had to be pretty much completely > > > rewritten for the new i915 atomic internals. > > > > > > v2: > > > - Set initial background color (black) via proper helper function (Bob) > > > - Fix debugfs output > > > - General rebasing > > > > > > Cc: Chandra Konduru> > > Cc: Bob Paauwe > > > Cc: dri-de...@lists.freedesktop.org > > > Signed-off-by: Matt Roper > > > --- > > > Documentation/DocBook/gpu.tmpl | 10 +++- > > > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++ > > > drivers/gpu/drm/i915/i915_reg.h | 9 +++ > > > drivers/gpu/drm/i915/intel_display.c | 46 > > > > > > 4 files changed, 72 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/DocBook/gpu.tmpl > > > b/Documentation/DocBook/gpu.tmpl > > > index fe6b36a..9e003cd 100644 > > > --- a/Documentation/DocBook/gpu.tmpl > > > +++ b/Documentation/DocBook/gpu.tmpl > > > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > > > TBD > > > > > > > > > - i915 > > > + i915 > > > Generic > > > "Broadcast RGB" > > > ENUM > > > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > > > TBD > > > > > > > > > + CRTC > > > + “background_color” > > > + RGBA > > > + > > > + CRTC > > > + Background color of regions not covered by a > > > plane > > > + > > > + > > > SDVO-TV > > > “mode” > > > ENUM > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > index ec0c2a05e..e7352fc 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, > > > void *unused) > > > intel_scaler_info(m, crtc); > > > intel_plane_info(m, crtc); > > > } > > > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > > > + struct drm_rgba background = > > > pipe_config->base.background_color; > > > + > > > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x > > > b=%x\n", > > > +DRM_RGBA_REDBITS(background, 10), > > > +DRM_RGBA_GREENBITS(background, 10), > > > +DRM_RGBA_BLUEBITS(background, 10)); > > > + } > > > > > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > > > yesno(!crtc->cpu_fifo_underrun_disabled), > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 144586e..b0b014d 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > > > #define PIPE_CSC_POSTOFF_ME(pipe)_MMIO_PIPE(pipe, > > > _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > > > #define PIPE_CSC_POSTOFF_LO(pipe)_MMIO_PIPE(pipe, > > > _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > > > > > +/* Skylake pipe bottom color */ > > > +#define _PIPE_BOTTOM_COLOR_A0x70034 > > > +#define _PIPE_BOTTOM_COLOR_B0x71034 > > > +#define _PIPE_BOTTOM_COLOR_C0x72034 > > > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > > > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > > > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFF > > > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, > > > _PIPE_BOTTOM_COLOR_B) > > > + > > > /* MIPI DSI registers */ > > > > > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and > > > C only */ > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 836bbdc..a616ac42 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct > > > intel_crtc *crtc, > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > struct intel_crtc_state *pipe_config = > > > to_intel_crtc_state(crtc->base.state); > > > + struct drm_rgba background = pipe_config->base.background_color; > > > + uint32_t val; > > > > > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > > >
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)
On Thu, Feb 11, 2016 at 12:00:50PM +0200, Ville Syrjälä wrote: > On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote: > > Gen9 platforms allow CRTC's to be programmed with a background/canvas > > color below the programmable planes. Let's expose this as a property to > > allow userspace to program a desired value. > > > > This patch is based on earlier work by Chandra Konduru; unfortunately > > the driver has evolved so much since his patches were written (in the > > pre-atomic era) that the functionality had to be pretty much completely > > rewritten for the new i915 atomic internals. > > > > v2: > > - Set initial background color (black) via proper helper function (Bob) > > - Fix debugfs output > > - General rebasing > > > > Cc: Chandra Konduru> > Cc: Bob Paauwe > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Matt Roper > > --- > > Documentation/DocBook/gpu.tmpl | 10 +++- > > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++ > > drivers/gpu/drm/i915/i915_reg.h | 9 +++ > > drivers/gpu/drm/i915/intel_display.c | 46 > > > > 4 files changed, 72 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > > index fe6b36a..9e003cd 100644 > > --- a/Documentation/DocBook/gpu.tmpl > > +++ b/Documentation/DocBook/gpu.tmpl > > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > > TBD > > > > > > - i915 > > + i915 > > Generic > > "Broadcast RGB" > > ENUM > > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > > TBD > > > > > > + CRTC > > + “background_color” > > + RGBA > > + > > + CRTC > > + Background color of regions not covered by a > > plane > > + > > + > > SDVO-TV > > “mode” > > ENUM > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index ec0c2a05e..e7352fc 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, > > void *unused) > > intel_scaler_info(m, crtc); > > intel_plane_info(m, crtc); > > } > > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > > + struct drm_rgba background = > > pipe_config->base.background_color; > > + > > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x > > b=%x\n", > > + DRM_RGBA_REDBITS(background, 10), > > + DRM_RGBA_GREENBITS(background, 10), > > + DRM_RGBA_BLUEBITS(background, 10)); > > + } > > > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > >yesno(!crtc->cpu_fifo_underrun_disabled), > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 144586e..b0b014d 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, > > _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, > > _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > > > +/* Skylake pipe bottom color */ > > +#define _PIPE_BOTTOM_COLOR_A0x70034 > > +#define _PIPE_BOTTOM_COLOR_B0x71034 > > +#define _PIPE_BOTTOM_COLOR_C0x72034 > > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFF > > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, > > _PIPE_BOTTOM_COLOR_B) > > + > > /* MIPI DSI registers */ > > > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and > > C only */ > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 836bbdc..a616ac42 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct > > intel_crtc *crtc, > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct intel_crtc_state *pipe_config = > > to_intel_crtc_state(crtc->base.state); > > + struct drm_rgba background = pipe_config->base.background_color; > > + uint32_t val; > > > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > > crtc->base.mode = crtc->base.state->mode; > > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct > > intel_crtc *crtc, > > else if (old_crtc_state->pch_pfit.enabled) > >
Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21
Submitted bug # 94104. https://bugs.freedesktop.org/show_bug.cgi?id=94104 Thanks, Martin -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Thursday, February 11, 2016 5:34 AM To: Rogers, Martin Cc: Jani Nikula; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21 On Wed, Feb 10, 2016 at 07:16:03PM +, Rogers, Martin wrote: > Hi Jani, > > I have not submitted a bug yet, because no kernel msgs were produced the > moment I recreated the issue. > I used : drm.debug=0x14 That should be drm.debug=14 or drm.debug=0xe > > Can you suggest something else ? > TIA, > Martin > > > -Original Message- > From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > Sent: Wednesday, February 03, 2016 6:41 AM > To: Rogers, Martin; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] Video glitches E3845 ATOM, Fedora 21 > > On Tue, 02 Feb 2016, "Rogers, Martin"wrote: > > I'm seeing weird video glitches on the Intel BayLey Bay -i CRB > > board, with Fedora 21, XFCE, and I hope someone can help. > > Please file a bug at > https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/I > ntel > > Add drm.debug=14 module parameter, and attach dmesg to the bug, from boot to > the problem. > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ This e-mail and any files transmitted with it are proprietary and intended solely for the use of the individual or entity to whom they are addressed. If you have reason to believe that you have received this e-mail in error, please notify the sender and destroy this email and any attached files. Please note that any views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of the Curtiss-Wright Corporation or any of its subsidiaries. Documents attached hereto may contain technology subject to government export regulations. Recipient is solely responsible for ensuring that any re-export, transfer or disclosure of this information is in accordance with applicable government export regulations. The recipient should check this e-mail and any attachments for the presence of viruses. Curtiss-Wright Corporation and its subsidiaries accept no liability for any damage caused by any virus transmitted by this e-mail. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Avoid vblank counter for gen9+
Framecounter register is read-only so DMC cannot restore it after exiting DC5 and DC6. Easiest way to go is to avoid the counter and use vblank interruptions for this platform and for all the following ones since DMC came to stay. At least while we can't change this register to read-write. Signed-off-by: Rodrigo Vivi--- drivers/gpu/drm/i915/i915_irq.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 25a8937..c294a4b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4556,7 +4556,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv) pm_qos_add_request(_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE); - if (IS_GEN2(dev_priv)) { + if (INTEL_INFO(dev_priv)->gen >= 9) { + dev->max_vblank_count = 0; + dev->driver->get_vblank_counter = g4x_get_vblank_counter; + } else if (IS_GEN2(dev_priv)) { dev->max_vblank_count = 0; dev->driver->get_vblank_counter = i8xx_get_vblank_counter; } else if (IS_G4X(dev_priv) || INTEL_INFO(dev_priv)->gen >= 5) { @@ -4572,7 +4575,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv) * Gen2 doesn't have a hardware frame counter and so depends on * vblank interrupts to produce sane vblank seuquence numbers. */ - if (!IS_GEN2(dev_priv)) + if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) dev->vblank_disable_immediate = true; dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp; -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V4] drm/i915/skl: SKL CDCLK change on modeset tracking VCO
[I'm cheating and doing this code review with the author watching over my shoulder] On 11/02/16 15:22, clinton.a.tay...@intel.com wrote: > From: Clint Taylor> > Track VCO frequency of SKL instead of the boot CDCLK and allow modeset > to set cdclk based on the max required pixel clock based on VCO > selected. Nit: the main point shouldn't come second. > The vco should be tracked at the atomic level and all CRTCs updated if > the required vco is changed. At this time the eDP pll is configured > inside the encoder which has no visibility into the atomic state. should be -> is > When eDP v1.4 panel that require the 8640 vco are available this may need > to be investigated. Just say that 8640 can't be tested yet. > V1: initial version > V2: add vco tracking in intel_dp_compute_config(), rename > skl_boot_cdclk. > V3: rebase, V2 feedback not possible as encoders are not aware of > atomic. > V4: track target vco is atomic state. modeset all CRTCs if vco changes > > Signed-off-by: Clint Taylor > Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= > --- > drivers/gpu/drm/i915/i915_drv.h |2 +- > drivers/gpu/drm/i915/intel_ddi.c |2 +- > drivers/gpu/drm/i915/intel_display.c | 97 > +- > drivers/gpu/drm/i915/intel_dp.c | 10 ++-- > drivers/gpu/drm/i915/intel_drv.h |4 ++ > 5 files changed, 97 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8216665..f65dd1a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1822,7 +1822,7 @@ struct drm_i915_private { > int num_fence_regs; /* 8 on pre-965, 16 otherwise */ > > unsigned int fsb_freq, mem_freq, is_ddr3; > - unsigned int skl_boot_cdclk; > + unsigned int skl_vco_freq; > unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq; > unsigned int max_dotclk_freq; > unsigned int hpll_freq; > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 6d5b09f..285adab 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2958,7 +2958,7 @@ void intel_ddi_pll_init(struct drm_device *dev) > int cdclk_freq; > > cdclk_freq = dev_priv->display.get_display_clock_speed(dev); > - dev_priv->skl_boot_cdclk = cdclk_freq; > + dev_priv->skl_vco_freq = skl_cdclk_get_vco(cdclk_freq); - skl_cdclk_get_vco() and skl_cdclk_frequencies[] should probably be renamed to: + skl_get_bios_cdclk_vco() and skl_bios_cdclk_frequencies[] to avoid confusion with the (different) mapping used in the new skl_modeset_calc_cdclk() function below. > if (skl_sanitize_cdclk(dev_priv)) > DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n"); > if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 9e2273b..ef4ac34 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5663,7 +5663,7 @@ static unsigned int skl_cdclk_decimal(unsigned int freq) > return (freq - 1000) / 500; > } > > -static unsigned int skl_cdclk_get_vco(unsigned int freq) > +unsigned int skl_cdclk_get_vco(unsigned int freq) > { > unsigned int i; > > @@ -5821,17 +5821,17 @@ void skl_uninit_cdclk(struct drm_i915_private > *dev_priv) > > void skl_init_cdclk(struct drm_i915_private *dev_priv) > { > - unsigned int required_vco; > - > /* DPLL0 not enabled (happens on early BIOS versions) */ > if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) { > /* enable DPLL0 */ > - required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk); > - skl_dpll0_enable(dev_priv, required_vco); > + if (dev_priv->skl_vco_freq != 8640) { > + dev_priv->skl_vco_freq = 8100; > + } > + skl_dpll0_enable(dev_priv, dev_priv->skl_vco_freq); > } > > /* set CDCLK to the frequency the BIOS chose */ This comment needs to be updated. Maybe something like: "initialize to the lowest/most economical freq for now; will modeset very soon anyway". > - skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk); > + skl_set_cdclk(dev_priv, (dev_priv->skl_vco_freq == 8100) ? 337500 : > 308570 ); > > /* enable DBUF power */ > I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST); > @@ -5847,7 +5847,7 @@ int skl_sanitize_cdclk(struct drm_i915_private > *dev_priv) > { > uint32_t lcpll1 = I915_READ(LCPLL1_CTL); > uint32_t cdctl = I915_READ(CDCLK_CTL); > - int freq = dev_priv->skl_boot_cdclk; > + int freq = dev_priv->cdclk_freq; > > /* >* check if the pre-os intialized
Re: [Intel-gfx] [PATCH] drm/i915: Avoid vblank counter for gen9+
Hi Rodrigo, [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on v4.5-rc3 next-20160211] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Rodrigo-Vivi/drm-i915-Avoid-vblank-counter-for-gen9/20160212-090608 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-x011-201606 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): drivers/gpu/drm/i915/i915_irq.c: In function 'intel_irq_init': >> drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: comparison of constant '9' >> with boolean expression is always false [-Wbool-compare] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: logical not is only >> applied to the left hand side of comparison [-Wlogical-not-parentheses] vim +/9 +4578 drivers/gpu/drm/i915/i915_irq.c 4562 } else if (IS_GEN2(dev_priv)) { 4563 dev->max_vblank_count = 0; 4564 dev->driver->get_vblank_counter = i8xx_get_vblank_counter; 4565 } else if (IS_G4X(dev_priv) || INTEL_INFO(dev_priv)->gen >= 5) { 4566 dev->max_vblank_count = 0x; /* full 32 bit counter */ 4567 dev->driver->get_vblank_counter = g4x_get_vblank_counter; 4568 } else { 4569 dev->driver->get_vblank_counter = i915_get_vblank_counter; 4570 dev->max_vblank_count = 0xff; /* only 24 bits of frame count */ 4571 } 4572 4573 /* 4574 * Opt out of the vblank disable timer on everything except gen2. 4575 * Gen2 doesn't have a hardware frame counter and so depends on 4576 * vblank interrupts to produce sane vblank seuquence numbers. 4577 */ > 4578 if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) 4579 dev->vblank_disable_immediate = true; 4580 4581 dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp; 4582 dev->driver->get_scanout_position = i915_get_crtc_scanoutpos; 4583 4584 if (IS_CHERRYVIEW(dev_priv)) { 4585 dev->driver->irq_handler = cherryview_irq_handler; 4586 dev->driver->irq_preinstall = cherryview_irq_preinstall; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Execlist irq handler micro optimisations
On Thu, Feb 11, 2016 at 06:03:10PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > Assorted changes most likely without any practical effect > apart from a tiny reduction in generated code for the interrupt > handler and request submission. > > * Remove needless initialization. > * Improve cache locality by reorganizing code and/or using >branch hints to keep unexpected or error conditions out >of line. > * Favor busy submit path vs. empty queue. > * Less branching in hot-paths. Sadly there are panics here to be taken care of before optimisations. :| > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/intel_lrc.c| 91 > - > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + > 2 files changed, 44 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 951f1e6af947..589d6404b5ca 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -269,6 +269,9 @@ logical_ring_init_platform_invariants(struct > intel_engine_cs *ring) > { > struct drm_device *dev = ring->dev; > > + if (IS_GEN8(dev) || IS_GEN9(dev)) > + ring->idle_lite_restore_wa = ~0; > + > ring->disable_lite_restore_wa = (IS_SKL_REVID(dev, 0, SKL_REVID_B0) || > IS_BXT_REVID(dev, 0, BXT_REVID_A1)) && > (ring->id == VCS || ring->id == VCS2); > @@ -424,7 +427,7 @@ static void execlists_submit_requests(struct > drm_i915_gem_request *rq0, > static void execlists_context_unqueue(struct intel_engine_cs *ring) > { > struct drm_i915_gem_request *req0 = NULL, *req1 = NULL; > - struct drm_i915_gem_request *cursor = NULL, *tmp = NULL; > + struct drm_i915_gem_request *cursor, *tmp; > > assert_spin_locked(>execlist_lock); > > @@ -434,9 +437,6 @@ static void execlists_context_unqueue(struct > intel_engine_cs *ring) >*/ > WARN_ON(!intel_irqs_enabled(ring->dev->dev_private)); > > - if (list_empty(>execlist_queue)) > - return; > - > /* Try to read in pairs */ > list_for_each_entry_safe(cursor, tmp, >execlist_queue, >execlist_link) { > @@ -451,37 +451,35 @@ static void execlists_context_unqueue(struct > intel_engine_cs *ring) > req0 = cursor; > } else { > req1 = cursor; > + WARN_ON(req1->elsp_submitted); > break; > } > } > > - if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) { > + if (unlikely(!req0)) > + return; > + > + if (req0->elsp_submitted & ring->idle_lite_restore_wa) { If you are going for the microptimsation, just do if (req->elsp_submitted) We unconditionally prepare the ringbuffer for the wa and currently apply it everywhere (and future hw will not be taking this path, aiui at least). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915/guc: Simplify code by keeping kmap of guc_client object
From: Alex DaiGuC client object is always pinned during its life cycle. We cache the kmap of its first page, which includes guc_process_desc and doorbell. By doing so, we can simplify the code where we read from this page to get where GuC is progressing on work queue; and the code where driver program doorbell to send work queue item to GuC. As a result, this patch removes the kmap_atomic in wq_check_space, where usleep_range could be called while kmap_atomic is held. This fixes issue below. [ 34.098798] BUG: scheduling while atomic: gem_close_race/1941/0x0002 [ 34.098822] Modules linked in: hid_generic usbhid i915 asix usbnet libphy mii i2c_algo_bit drm_kms_helper cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea drm coretemp i2c_hid hid video pinctrl_sunrisepoint pinctrl_intel acpi_pad nls_iso8859_1 e1000e ptp psmouse pps_core ahci libahci [ 34.098824] CPU: 0 PID: 1941 Comm: gem_close_race Tainted: G U 4.4.0-160121+ #123 [ 34.098824] Hardware name: Intel Corporation Skylake Client platform/Skylake AIO DDR3L RVP10, BIOS SKLSE2R1.R00.X100.B01.1509220551 09/22/2015 [ 34.098825] 00013e40 880166c27a78 81280d02 880172c13e40 [ 34.098826] 880166c27a88 810c203a 880166c27ac8 814ec808 [ 34.098827] 88016b7c6000 880166c28000 000f4240 0001 [ 34.098827] Call Trace: [ 34.098831] [] dump_stack+0x4b/0x79 [ 34.098833] [] __schedule_bug+0x41/0x4f [ 34.098834] [] __schedule+0x5a8/0x690 [ 34.098835] [] schedule+0x37/0x80 [ 34.098836] [] schedule_hrtimeout_range_clock+0xad/0x130 [ 34.098837] [] ? hrtimer_init+0x10/0x10 [ 34.098838] [] ? schedule_hrtimeout_range_clock+0xa1/0x130 [ 34.098839] [] schedule_hrtimeout_range+0xe/0x10 [ 34.098840] [] usleep_range+0x3b/0x40 [ 34.098853] [] i915_guc_wq_check_space+0x119/0x210 [i915] [ 34.098861] [] intel_logical_ring_alloc_request_extras+0x5c/0x70 [i915] [ 34.098869] [] i915_gem_request_alloc+0x91/0x170 [i915] [ 34.098875] [] i915_gem_do_execbuffer.isra.25+0xbc7/0x12a0 [i915] [ 34.098882] [] ? i915_gem_object_get_pages_gtt+0x225/0x3c0 [i915] [ 34.098889] [] ? i915_gem_pwrite_ioctl+0xd6/0x9f0 [i915] [ 34.098895] [] i915_gem_execbuffer2+0xa8/0x250 [i915] [ 34.098900] [] drm_ioctl+0x258/0x4f0 [drm] [ 34.098906] [] ? i915_gem_execbuffer+0x340/0x340 [i915] [ 34.098908] [] do_vfs_ioctl+0x2cd/0x4a0 [ 34.098909] [] ? __fget+0x72/0xb0 [ 34.098910] [] SyS_ioctl+0x3c/0x70 [ 34.098911] [] entry_SYSCALL_64_fastpath+0x12/0x6a [ 34.100208] [ cut here ] Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93847 Cc: Cc: Signed-off-by: Alex Dai --- drivers/gpu/drm/i915/i915_guc_submission.c | 39 +- drivers/gpu/drm/i915/intel_guc.h | 3 ++- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index d7543ef..d51015e 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -195,11 +195,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) struct guc_process_desc *desc; union guc_doorbell_qw db_cmp, db_exc, db_ret; union guc_doorbell_qw *db; - void *base; int attempt = 2, ret = -EAGAIN; - base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0)); - desc = base + gc->proc_desc_offset; + desc = gc->client_base + gc->proc_desc_offset; /* Update the tail so it is visible to GuC */ desc->tail = gc->wq_tail; @@ -215,7 +213,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) db_exc.cookie = 1; /* pointer of current doorbell cacheline */ - db = base + gc->doorbell_offset; + db = gc->client_base + gc->doorbell_offset; while (attempt--) { /* lets ring the doorbell */ @@ -244,10 +242,6 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) db_exc.cookie = 1; } - /* Finally, update the cached copy of the GuC's WQ head */ - gc->wq_head = desc->head; - - kunmap_atomic(base); return ret; } @@ -341,10 +335,8 @@ static void guc_init_proc_desc(struct intel_guc *guc, struct i915_guc_client *client) { struct guc_process_desc *desc; - void *base; - base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0)); - desc = base + client->proc_desc_offset; + desc = client->client_base + client->proc_desc_offset; memset(desc, 0, sizeof(*desc)); @@ -361,8 +353,6 @@ static void guc_init_proc_desc(struct intel_guc *guc, desc->wq_size_bytes = client->wq_size; desc->wq_status =
[Intel-gfx] [PATCH V4] drm/i915/skl: SKL CDCLK change on modeset tracking VCO
From: Clint TaylorTrack VCO frequency of SKL instead of the boot CDCLK and allow modeset to set cdclk based on the max required pixel clock based on VCO selected. The vco should be tracked at the atomic level and all CRTCs updated if the required vco is changed. At this time the eDP pll is configured inside the encoder which has no visibility into the atomic state. When eDP v1.4 panel that require the 8640 vco are available this may need to be investigated. V1: initial version V2: add vco tracking in intel_dp_compute_config(), rename skl_boot_cdclk. V3: rebase, V2 feedback not possible as encoders are not aware of atomic. V4: track target vco is atomic state. modeset all CRTCs if vco changes Signed-off-by: Clint Taylor Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= --- drivers/gpu/drm/i915/i915_drv.h |2 +- drivers/gpu/drm/i915/intel_ddi.c |2 +- drivers/gpu/drm/i915/intel_display.c | 97 +- drivers/gpu/drm/i915/intel_dp.c | 10 ++-- drivers/gpu/drm/i915/intel_drv.h |4 ++ 5 files changed, 97 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8216665..f65dd1a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1822,7 +1822,7 @@ struct drm_i915_private { int num_fence_regs; /* 8 on pre-965, 16 otherwise */ unsigned int fsb_freq, mem_freq, is_ddr3; - unsigned int skl_boot_cdclk; + unsigned int skl_vco_freq; unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq; unsigned int max_dotclk_freq; unsigned int hpll_freq; diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 6d5b09f..285adab 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2958,7 +2958,7 @@ void intel_ddi_pll_init(struct drm_device *dev) int cdclk_freq; cdclk_freq = dev_priv->display.get_display_clock_speed(dev); - dev_priv->skl_boot_cdclk = cdclk_freq; + dev_priv->skl_vco_freq = skl_cdclk_get_vco(cdclk_freq); if (skl_sanitize_cdclk(dev_priv)) DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n"); if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9e2273b..ef4ac34 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5663,7 +5663,7 @@ static unsigned int skl_cdclk_decimal(unsigned int freq) return (freq - 1000) / 500; } -static unsigned int skl_cdclk_get_vco(unsigned int freq) +unsigned int skl_cdclk_get_vco(unsigned int freq) { unsigned int i; @@ -5821,17 +5821,17 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv) void skl_init_cdclk(struct drm_i915_private *dev_priv) { - unsigned int required_vco; - /* DPLL0 not enabled (happens on early BIOS versions) */ if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) { /* enable DPLL0 */ - required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk); - skl_dpll0_enable(dev_priv, required_vco); + if (dev_priv->skl_vco_freq != 8640) { + dev_priv->skl_vco_freq = 8100; + } + skl_dpll0_enable(dev_priv, dev_priv->skl_vco_freq); } /* set CDCLK to the frequency the BIOS chose */ - skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk); + skl_set_cdclk(dev_priv, (dev_priv->skl_vco_freq == 8100) ? 337500 : 308570 ); /* enable DBUF power */ I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST); @@ -5847,7 +5847,7 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) { uint32_t lcpll1 = I915_READ(LCPLL1_CTL); uint32_t cdctl = I915_READ(CDCLK_CTL); - int freq = dev_priv->skl_boot_cdclk; + int freq = dev_priv->cdclk_freq; /* * check if the pre-os intialized the display @@ -5871,11 +5871,7 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) /* All well; nothing to sanitize */ return false; sanitize: - /* -* As of now initialize with max cdclk till -* we get dynamic cdclk support -* */ - dev_priv->skl_boot_cdclk = dev_priv->max_cdclk_freq; + skl_init_cdclk(dev_priv); /* we did have to sanitize */ @@ -9845,6 +9841,68 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) broadwell_set_cdclk(dev, req_cdclk); } +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->dev); + int max_pixclk =
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Do not return stale status / remove impossible WARN
On Thu, Feb 11, 2016 at 06:03:09PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > Only caller to get_context_status ensures read pointer stays in > range so the WARN is impossible. Also, if the WARN would be > triggered by a hypothetical new caller stale status would be > returned to them. > > Maybe it is better to wrap the pointer in the function itself > then to avoid both and even results in smaller code. > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/intel_lrc.c | 15 ++- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 89eb892df4ae..951f1e6af947 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -507,17 +507,16 @@ static bool execlists_check_remove_request(struct > intel_engine_cs *ring, > return false; > } > > -static void get_context_status(struct intel_engine_cs *ring, > -u8 read_pointer, > -u32 *status, u32 *context_id) > +static u32 get_context_status(struct intel_engine_cs *ring, u8 read_pointer, > + u32 *context_id) > { > struct drm_i915_private *dev_priv = ring->dev->dev_private; > > - if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES)) > - return; > + read_pointer %= GEN8_CSB_ENTRIES; > > - *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer)); > *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer)); Micro-optimising hat says not to even do the uncached, spinlocked mmio read when not required. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V5] drm/i915: edp resume/On time optimization.
On 1/22/2016 5:39 PM, Kumar, Abhay wrote: From: Abhay KumarMake resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if this time is already spent in suspend/poweron time. v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle delay calculation(Ville). v3: Addressed below comments 1. Tracking time from where last powercycle is initiated. 2. Used ktime_get_bootime() wrapper for boottime clock. 3. Used ktime_ms_delta() to get time difference. v4: Updated v3 change log in detail. v5: Removed static from panel_power_on_time(Stéphane). Cc: Ville Syrjälä Signed-off-by: Abhay Kumar Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp.c | 19 ++- drivers/gpu/drm/i915/intel_drv.h | 2 +- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e2bea710..29f21c1 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1811,12 +1811,21 @@ static void wait_panel_off(struct intel_dp *intel_dp) static void wait_panel_power_cycle(struct intel_dp *intel_dp) { + ktime_t panel_power_on_time; + s64 panel_power_off_duration; + DRM_DEBUG_KMS("Wait for panel power cycle\n"); + /* take the difference of currrent time and panel power off time +* and then make panel wait for t11_t12 if needed. */ + panel_power_on_time = ktime_get_boottime(); + panel_power_off_duration = ktime_ms_delta(panel_power_on_time, intel_dp->panel_power_off_time); + /* When we disable the VDD override bit last we have to do the manual * wait. */ - wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle, - intel_dp->panel_power_cycle_delay); + if (panel_power_off_duration < (s64)intel_dp->panel_power_cycle_delay) + wait_remaining_ms_from_jiffies(jiffies, + intel_dp->panel_power_cycle_delay - panel_power_off_duration); wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE); } @@ -1968,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp) I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg)); if ((pp & POWER_TARGET_ON) == 0) - intel_dp->last_power_cycle = jiffies; + intel_dp->panel_power_off_time = ktime_get_boottime(); power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_put(dev_priv, power_domain); @@ -2117,7 +2126,7 @@ static void edp_panel_off(struct intel_dp *intel_dp) I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); - intel_dp->last_power_cycle = jiffies; + intel_dp->panel_power_off_time = ktime_get_boottime(); wait_panel_off(intel_dp); /* We got a reference when we enabled the VDD. */ @@ -5116,7 +5125,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) { - intel_dp->last_power_cycle = jiffies; + intel_dp->panel_power_off_time = ktime_get_boottime(); intel_dp->last_power_on = jiffies; intel_dp->last_backlight_off = jiffies; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bc97012..137e40d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -770,9 +770,9 @@ struct intel_dp { int backlight_off_delay; struct delayed_work panel_vdd_work; bool want_panel_vdd; - unsigned long last_power_cycle; unsigned long last_power_on; unsigned long last_backlight_off; + ktime_t panel_power_off_time; struct notifier_block edp_notifier; Hi Daniel, can we please get this patch Queued? Thanks Regards, Abhay Kumar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid vblank counter for gen9+
Hi Rodrigo, [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on v4.5-rc3 next-20160211] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Rodrigo-Vivi/drm-i915-Avoid-vblank-counter-for-gen9/20160212-090608 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-x000-201606 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): In file included from include/uapi/linux/stddef.h:1:0, from include/linux/stddef.h:4, from include/uapi/linux/posix_types.h:4, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from include/linux/sysrq.h:18, from drivers/gpu/drm/i915/i915_irq.c:31: drivers/gpu/drm/i915/i915_irq.c: In function 'intel_irq_init': drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: comparison of constant '9' with boolean expression is always false [-Wbool-compare] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ include/linux/compiler.h:147:28: note: in definition of macro '__trace_if' if (__builtin_constant_p((cond)) ? !!(cond) : \ ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:2: note: in expansion of macro 'if' if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ include/linux/compiler.h:147:28: note: in definition of macro '__trace_if' if (__builtin_constant_p((cond)) ? !!(cond) : \ ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:2: note: in expansion of macro 'if' if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: comparison of constant '9' with boolean expression is always false [-Wbool-compare] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ include/linux/compiler.h:147:40: note: in definition of macro '__trace_if' if (__builtin_constant_p((cond)) ? !!(cond) : \ ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:2: note: in expansion of macro 'if' if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ include/linux/compiler.h:147:40: note: in definition of macro '__trace_if' if (__builtin_constant_p((cond)) ? !!(cond) : \ ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:2: note: in expansion of macro 'if' if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: comparison of constant '9' with boolean expression is always false [-Wbool-compare] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ include/linux/compiler.h:158:16: note: in definition of macro '__trace_if' __r = !!(cond); \ ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:2: note: in expansion of macro 'if' if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ drivers/gpu/drm/i915/i915_irq.c:4578:55: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses] if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ include/linux/compiler.h:158:16: note: in definition of macro '__trace_if' __r = !!(cond); \ ^ >> drivers/gpu/drm/i915/i915_irq.c:4578:2: note: in expansion of macro 'if' if (!IS_GEN2(dev_priv) && !INTEL_INFO(dev_priv)->gen >= 9) ^ vim +/if +4578 drivers/gpu/drm/i915/i915_irq.c 4562 } else if (IS_GEN2(dev_priv)) { 4563 dev->max_vblank_count = 0; 4564 dev->driver->get_vblank_counter = i8xx_get_vblank_counter; 4565 } else if (IS_G4X(dev_priv) || INTEL_INFO(dev_p
Re: [Intel-gfx] [PATCH] drm/i915: Hide GEM shutdown in i915_gem_fini
Hi Tvrtko, [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on next-20160211] [cannot apply to v4.5-rc3] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Tvrtko-Ursulin/drm-i915-Hide-GEM-shutdown-in-i915_gem_fini/20160211-234159 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-i0-201606 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/i915/i915_drv.c:34:0: >> drivers/gpu/drm/i915/i915_drv.h:3057:40: warning: 'warn_unused_result' >> attribute ignored [-Wattributes] void __must_check i915_gem_fini(struct drm_device *dev); ^ vim +/warn_unused_result +3057 drivers/gpu/drm/i915/i915_drv.h 3041 3042 static inline bool i915_stop_ring_allow_ban(struct drm_i915_private *dev_priv) 3043 { 3044 return dev_priv->gpu_error.stop_rings == 0 || 3045 dev_priv->gpu_error.stop_rings & I915_STOP_RING_ALLOW_BAN; 3046 } 3047 3048 static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv) 3049 { 3050 return dev_priv->gpu_error.stop_rings == 0 || 3051 dev_priv->gpu_error.stop_rings & I915_STOP_RING_ALLOW_WARN; 3052 } 3053 3054 void i915_gem_reset(struct drm_device *dev); 3055 bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force); 3056 int __must_check i915_gem_init(struct drm_device *dev); > 3057 void __must_check i915_gem_fini(struct drm_device *dev); 3058 int i915_gem_init_rings(struct drm_device *dev); 3059 int __must_check i915_gem_init_hw(struct drm_device *dev); 3060 int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice); 3061 void i915_gem_init_swizzling(struct drm_device *dev); 3062 int __must_check i915_gpu_idle(struct drm_device *dev); 3063 int __must_check i915_gem_suspend(struct drm_device *dev); 3064 void __i915_add_request(struct drm_i915_gem_request *req, 3065 struct drm_i915_gem_object *batch_obj, --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Execlist irq handler micro optimisations
From: Tvrtko UrsulinAssorted changes most likely without any practical effect apart from a tiny reduction in generated code for the interrupt handler and request submission. * Remove needless initialization. * Improve cache locality by reorganizing code and/or using branch hints to keep unexpected or error conditions out of line. * Favor busy submit path vs. empty queue. * Less branching in hot-paths. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_lrc.c| 91 - drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 2 files changed, 44 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 951f1e6af947..589d6404b5ca 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -269,6 +269,9 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *ring) { struct drm_device *dev = ring->dev; + if (IS_GEN8(dev) || IS_GEN9(dev)) + ring->idle_lite_restore_wa = ~0; + ring->disable_lite_restore_wa = (IS_SKL_REVID(dev, 0, SKL_REVID_B0) || IS_BXT_REVID(dev, 0, BXT_REVID_A1)) && (ring->id == VCS || ring->id == VCS2); @@ -424,7 +427,7 @@ static void execlists_submit_requests(struct drm_i915_gem_request *rq0, static void execlists_context_unqueue(struct intel_engine_cs *ring) { struct drm_i915_gem_request *req0 = NULL, *req1 = NULL; - struct drm_i915_gem_request *cursor = NULL, *tmp = NULL; + struct drm_i915_gem_request *cursor, *tmp; assert_spin_locked(>execlist_lock); @@ -434,9 +437,6 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring) */ WARN_ON(!intel_irqs_enabled(ring->dev->dev_private)); - if (list_empty(>execlist_queue)) - return; - /* Try to read in pairs */ list_for_each_entry_safe(cursor, tmp, >execlist_queue, execlist_link) { @@ -451,37 +451,35 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring) req0 = cursor; } else { req1 = cursor; + WARN_ON(req1->elsp_submitted); break; } } - if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) { + if (unlikely(!req0)) + return; + + if (req0->elsp_submitted & ring->idle_lite_restore_wa) { /* -* WaIdleLiteRestore: make sure we never cause a lite -* restore with HEAD==TAIL +* WaIdleLiteRestore: make sure we never cause a lite restore +* with HEAD==TAIL. +* +* Apply the wa NOOPS to prevent ring:HEAD == req:TAIL as we +* resubmit the request. See gen8_emit_request() for where we +* prepare the padding after the end of the request. */ - if (req0->elsp_submitted) { - /* -* Apply the wa NOOPS to prevent ring:HEAD == req:TAIL -* as we resubmit the request. See gen8_emit_request() -* for where we prepare the padding after the end of the -* request. -*/ - struct intel_ringbuffer *ringbuf; + struct intel_ringbuffer *ringbuf; - ringbuf = req0->ctx->engine[ring->id].ringbuf; - req0->tail += 8; - req0->tail &= ringbuf->size - 1; - } + ringbuf = req0->ctx->engine[ring->id].ringbuf; + req0->tail += 8; + req0->tail &= ringbuf->size - 1; } - WARN_ON(req1 && req1->elsp_submitted); - execlists_submit_requests(req0, req1); } -static bool execlists_check_remove_request(struct intel_engine_cs *ring, - u32 request_id) +static unsigned int +execlists_check_remove_request(struct intel_engine_cs *ring, u32 request_id) { struct drm_i915_gem_request *head_req; @@ -491,20 +489,21 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring, struct drm_i915_gem_request, execlist_link); - if (head_req != NULL) { - if (intel_execlists_ctx_id(head_req->ctx, ring) == request_id) { - WARN(head_req->elsp_submitted == 0, -"Never submitted head request\n"); + if (!head_req) + return 0; + + if (unlikely(intel_execlists_ctx_id(head_req->ctx, ring) != request_id)) + return 0; - if
[Intel-gfx] [PATCH 1/2] drm/i915: Do not return stale status / remove impossible WARN
From: Tvrtko UrsulinOnly caller to get_context_status ensures read pointer stays in range so the WARN is impossible. Also, if the WARN would be triggered by a hypothetical new caller stale status would be returned to them. Maybe it is better to wrap the pointer in the function itself then to avoid both and even results in smaller code. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_lrc.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 89eb892df4ae..951f1e6af947 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -507,17 +507,16 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring, return false; } -static void get_context_status(struct intel_engine_cs *ring, - u8 read_pointer, - u32 *status, u32 *context_id) +static u32 get_context_status(struct intel_engine_cs *ring, u8 read_pointer, + u32 *context_id) { struct drm_i915_private *dev_priv = ring->dev->dev_private; - if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES)) - return; + read_pointer %= GEN8_CSB_ENTRIES; - *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer)); *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer)); + + return I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer)); } /** @@ -547,9 +546,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) spin_lock(>execlist_lock); while (read_pointer < write_pointer) { - - get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES, - , _id); + status = get_context_status(ring, ++read_pointer, _id); if (status & GEN8_CTX_STATUS_IDLE_ACTIVE) continue; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)
On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote: > Gen9 platforms allow CRTC's to be programmed with a background/canvas > color below the programmable planes. Let's expose this as a property to > allow userspace to program a desired value. > > This patch is based on earlier work by Chandra Konduru; unfortunately > the driver has evolved so much since his patches were written (in the > pre-atomic era) that the functionality had to be pretty much completely > rewritten for the new i915 atomic internals. > > v2: > - Set initial background color (black) via proper helper function (Bob) > - Fix debugfs output > - General rebasing > > Cc: Chandra Konduru> Cc: Bob Paauwe > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Matt Roper > --- > Documentation/DocBook/gpu.tmpl | 10 +++- > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++ > drivers/gpu/drm/i915/i915_reg.h | 9 +++ > drivers/gpu/drm/i915/intel_display.c | 46 > > 4 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index fe6b36a..9e003cd 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > TBD > > > - i915 > + i915 > Generic > "Broadcast RGB" > ENUM > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > TBD > > > + CRTC > + “background_color” > + RGBA > + > + CRTC > + Background color of regions not covered by a > plane > + > + > SDVO-TV > “mode” > ENUM > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index ec0c2a05e..e7352fc 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void > *unused) > intel_scaler_info(m, crtc); > intel_plane_info(m, crtc); > } > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > + struct drm_rgba background = > pipe_config->base.background_color; > + > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x > b=%x\n", > +DRM_RGBA_REDBITS(background, 10), > +DRM_RGBA_GREENBITS(background, 10), > +DRM_RGBA_BLUEBITS(background, 10)); > + } > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > yesno(!crtc->cpu_fifo_underrun_disabled), > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 144586e..b0b014d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > #define PIPE_CSC_POSTOFF_ME(pipe)_MMIO_PIPE(pipe, > _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > #define PIPE_CSC_POSTOFF_LO(pipe)_MMIO_PIPE(pipe, > _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > +/* Skylake pipe bottom color */ > +#define _PIPE_BOTTOM_COLOR_A0x70034 > +#define _PIPE_BOTTOM_COLOR_B0x71034 > +#define _PIPE_BOTTOM_COLOR_C0x72034 > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFF > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, > _PIPE_BOTTOM_COLOR_B) > + > /* MIPI DSI registers */ > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and > C only */ > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 836bbdc..a616ac42 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc > *crtc, > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc_state *pipe_config = > to_intel_crtc_state(crtc->base.state); > + struct drm_rgba background = pipe_config->base.background_color; > + uint32_t val; > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > crtc->base.mode = crtc->base.state->mode; > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc > *crtc, > else if (old_crtc_state->pch_pfit.enabled) > ironlake_pfit_disable(crtc, true); > } > + > + if (INTEL_INFO(dev)->gen >= 9) { > + /* BGR 16bpc ==> RGB 10bpc */ > + val = DRM_RGBA_REDBITS(background, 10) << 20 > + |
Re: [Intel-gfx] [PATCH 3/4] agp/intel-gtt: Only register fake agp driver for gen1
On Wed, Jan 27, 2016 at 02:38:00PM +0100, Daniel Vetter wrote: > The fake agp driver for the intel graphics gart is only needed for ums > support. And we ditched that a long time ago: > > commit 03dae59c72d8ef6e005f48ba356c863e0587 > Author: Daniel Vetter> Date: Wed Jul 23 16:27:25 2014 +0200 > > drm/i915: Ditch UMS config option > > With this there's no longer the problem that 2 drivers (fake agp > driver and the drm/i915 driver) fight over the same piece, which fixes > apparent dma leaks detected by CONFIG_DMA_API_DEBUG. > > Note that the leak isn't real since intel-gtt refcounts and will tear > down eventually. But the debug code assumes that when the i915 driver > unbinds from the pci device everything should be gone. Which isn't the > case if we have intel-agp enabled - userspace might need it. But by > ditching this intel-gtt setup and teardown is completely tied to the > livetime of the "real" driver. > > While at it untangle the init ordering a bit - the fake agp wouldn't > be initialized correctly if i915.ko loads first. Which isn't a problem > since when i915 loads in kms mode you won't need the fake agp support > needed by the ums driver ... > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93793 > Signed-off-by: Daniel Vetter > --- > drivers/char/agp/intel-gtt.c | 24 > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index e657f989745e..aef87fdbd187 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -1348,16 +1348,6 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, > struct pci_dev *gpu_pdev, > { > int i, mask; > > - /* > - * Can be called from the fake agp driver but also directly from > - * drm/i915.ko. Hence we need to check whether everything is set up > - * already. > - */ > - if (intel_private.driver) { > - intel_private.refcount++; > - return 1; > - } > - > for (i = 0; intel_gtt_chipsets[i].name != NULL; i++) { > if (gpu_pdev) { > if (gpu_pdev->device == > @@ -1378,16 +1368,26 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, > struct pci_dev *gpu_pdev, > if (!intel_private.driver) > return 0; > > - intel_private.refcount++; > - > #if IS_ENABLED(CONFIG_AGP_INTEL) > if (bridge) { > + if (INTEL_GTT_GEN > 1) > + return 0; So if this happens first, we set up intel_private.driver but leave the refcount at 0. Then i915 loads and we set up intel_private.driver again, and bump the refcount to 0. Well, we should end up pointing intel_privatee.driver at the same thing both times so not really a problem I suppose. So yeah, I think this ought to work Reviewed-by: Ville Syrjälä I guess we could also trim the gen>=2 gmch pci ids from the agp driver's pci id table, to avoid even probing it. > + > bridge->driver = _fake_agp_driver; > bridge->dev_private_data = _private; > bridge->dev = bridge_pdev; > } > #endif > > + > + /* > + * Can be called from the fake agp driver but also directly from > + * drm/i915.ko. Hence we need to check whether everything is set up > + * already. > + */ > + if (intel_private.refcount++) > + return 1; > + > intel_private.bridge_dev = pci_dev_get(bridge_pdev); > > dev_info(_pdev->dev, "Intel %s Chipset\n", > intel_gtt_chipsets[i].name); > -- > 2.5.0 > > ___ > 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 https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] Use uint64_t in eviction memory subtest parameters
With large apertures we need to use uint64_t for counts and sizes. commit 0e2071411a4d4e1488a821daf522dffde2809e03 paved way for this but forgot to change the subtest parameters. v2: Pass correctly to the copy() also (Chris) References: https://bugs.freedesktop.org/show_bug.cgi?id=93849 Signed-off-by: Mika Kuoppala--- tests/gem_evict_alignment.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/gem_evict_alignment.c b/tests/gem_evict_alignment.c index 6fa70f0cd0f9..3ce1bea9dec3 100644 --- a/tests/gem_evict_alignment.c +++ b/tests/gem_evict_alignment.c @@ -53,7 +53,8 @@ IGT_TEST_DESCRIPTION("Run a couple of big batches to force the unbind on" #define WIDTH 1024 static void -copy(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo, int alignment, int error) +copy(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, + uint64_t n_bo, uint64_t alignment, int error) { uint32_t batch[12]; struct drm_i915_gem_relocation_entry reloc[2]; @@ -129,10 +130,11 @@ copy(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo, int alignme free(obj); } -static void minor_evictions(int fd, int size, int count) +static void minor_evictions(int fd, uint64_t size, uint64_t count) { uint32_t *bo, *sel; - int n, m, alignment, pass, fail; + uint64_t n, m, alignment; + int pass, fail; intel_require_memory(2 * count, size, CHECK_RAM); @@ -159,9 +161,10 @@ static void minor_evictions(int fd, int size, int count) free(bo); } -static void major_evictions(int fd, int size, int count) +static void major_evictions(int fd, uint64_t size, uint64_t count) { - int n, m, loop, alignment, max; + uint64_t n, m, alignment, max; + int loop; uint32_t *bo; intel_require_memory(count, size, CHECK_RAM); -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen9: Check for DC state mismatch
Patrik Jakobssonwrites: > The DMC can incorrectly run off and allow DC states on it's own. We > don't know the root-cause for this yet but this patch makes it more > visible. > > Signed-off-by: Patrik Jakobsson Yes, we definitely need much more state checking and hardening in this area. Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_csr.c| 2 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 8 > 3 files changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e11eef1..7e33454 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -746,6 +746,7 @@ struct intel_csr { > uint32_t mmio_count; > i915_reg_t mmioaddr[8]; > uint32_t mmiodata[8]; > + uint32_t dc_state; > }; > > #define DEV_INFO_FOR_EACH_FLAG(func, sep) \ > diff --git a/drivers/gpu/drm/i915/intel_csr.c > b/drivers/gpu/drm/i915/intel_csr.c > index 2a7ec31..b453fcc 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -243,6 +243,8 @@ void intel_csr_load_program(struct drm_i915_private > *dev_priv) > I915_WRITE(dev_priv->csr.mmioaddr[i], > dev_priv->csr.mmiodata[i]); > } > + > + dev_priv->csr.dc_state = 0; > } > > static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index bbca527..e79674b 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -494,10 +494,18 @@ static void gen9_set_dc_state(struct drm_i915_private > *dev_priv, uint32_t state) > val = I915_READ(DC_STATE_EN); > DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n", > val & mask, state); > + > + /* Check if DMC is ignoring our DC state requests */ > + if ((val & mask) != dev_priv->csr.dc_state) > + DRM_ERROR("DC state mismatch (0x%x -> 0x%x)\n", > + dev_priv->csr.dc_state, val & mask); > + > val &= ~mask; > val |= state; > I915_WRITE(DC_STATE_EN, val); > POSTING_READ(DC_STATE_EN); > + > + dev_priv->csr.dc_state = val & mask; > } > > void bxt_enable_dc9(struct drm_i915_private *dev_priv) > -- > 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 2/3] lib/igt_core: Unify handling of slow/combinatorial tests
Some subtests are not run by default, for various reasons; be it because they're only for debugging, because they're slow, or because they are not of high enough quality. This patch aims to introduce a common mechanism for categorising the subtests and introduces a flag (--all) that runs/lists all subtests instead of just the default set. Signed-off-by: David Weinehall--- lib/igt_core.c | 43 +++-- lib/igt_core.h | 42 + tests/gem_concurrent_blit.c | 50 +-- tests/kms_frontbuffer_tracking.c | 186 ++- 4 files changed, 210 insertions(+), 111 deletions(-) diff --git a/lib/igt_core.c b/lib/igt_core.c index 6b69bb780700..e6e6949ed65a 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -216,6 +216,7 @@ const char *igt_interactive_debug; /* subtests helpers */ static bool list_subtests = false; +static unsigned int subtest_types_mask = SUBTEST_TYPE_NORMAL; static char *run_single_subtest = NULL; static bool run_single_subtest_found = false; static const char *in_subtest = NULL; @@ -237,12 +238,13 @@ int test_children_sz; bool test_child; enum { - OPT_LIST_SUBTESTS, - OPT_RUN_SUBTEST, - OPT_DESCRIPTION, - OPT_DEBUG, - OPT_INTERACTIVE_DEBUG, - OPT_HELP = 'h' + OPT_LIST_SUBTESTS, + OPT_WITH_ALL_SUBTESTS, + OPT_RUN_SUBTEST, + OPT_DESCRIPTION, + OPT_DEBUG, + OPT_INTERACTIVE_DEBUG, + OPT_HELP = 'h' }; static int igt_exitcode = IGT_EXIT_SUCCESS; @@ -516,6 +518,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) fprintf(f, "Usage: %s [OPTIONS]\n", command_str); fprintf(f, " --list-subtests\n" + " --all\n" " --run-subtest \n" " --debug[=log-domain]\n" " --interactive-debug[=domain]\n" @@ -548,6 +551,7 @@ static int common_init(int *argc, char **argv, int c, option_index = 0, i, x; static struct option long_options[] = { {"list-subtests", 0, 0, OPT_LIST_SUBTESTS}, + {"all", 0, 0, OPT_WITH_ALL_SUBTESTS}, {"run-subtest", 1, 0, OPT_RUN_SUBTEST}, {"help-description", 0, 0, OPT_DESCRIPTION}, {"debug", optional_argument, 0, OPT_DEBUG}, @@ -659,6 +663,10 @@ static int common_init(int *argc, char **argv, if (!run_single_subtest) list_subtests = true; break; + case OPT_WITH_ALL_SUBTESTS: + if (!run_single_subtest) + subtest_types_mask = SUBTEST_TYPE_ALL; + break; case OPT_RUN_SUBTEST: if (!list_subtests) run_single_subtest = strdup(optarg); @@ -1667,6 +1675,29 @@ void igt_skip_on_simulation(void) igt_require(!igt_run_in_simulation()); } +/** + * igt_match_subtest_flags: + * + * This function is used to check whether the attributes of the subtest + * makes it a candidate for inclusion in the test run; this is used to + * categorise tests, for instance to exclude tests that are purely for + * debug purposes, tests that are specific to certain environments, + * or tests that are very slow. + * + * Note that a test has to have all its flags met to be run; for instance + * a subtest with the flags SUBTEST_TYPE_SLOW | SUBTEST_TYPE_DEBUG requires + * "--subtest-types=slow,debug" or "--all" to be executed + * + * @subtest_flags: The subtests to check for + * + * Returns: true if the subtest test should be run, + * false if the subtest should be skipped + */ +bool igt_match_subtest_flags(unsigned long subtest_flags) +{ + return ((subtest_flags & subtest_types_mask) == subtest_flags); +} + /* structured logging */ /** diff --git a/lib/igt_core.h b/lib/igt_core.h index 8f297e06a068..bf83de609bfa 100644 --- a/lib/igt_core.h +++ b/lib/igt_core.h @@ -193,6 +193,48 @@ bool __igt_run_subtest(const char *subtest_name); #define igt_subtest_f(f...) \ __igt_subtest_f(igt_tokencat(__tmpchar, __LINE__), f) +enum { + /* The set of tests run if nothing else is specified */ + SUBTEST_TYPE_NORMAL = 1 << 0, + /* Basic Acceptance Testing set */ + SUBTEST_TYPE_BASIC = 1 << 1, + /* Tests that are very slow */ + SUBTEST_TYPE_SLOW = 1 << 2, + /* Tests that mainly intended for debugging */ + SUBTEST_TYPE_DEBUG = 1 << 3, + SUBTEST_TYPE_ALL = ~0 +} subtest_types; + +bool igt_match_subtest_flags(unsigned long subtest_flags); + +/** + * igt_subtest_flags: + * @name: name of the subtest + * @__subtest_flags: the categories the subtest belongs to + * + * This is a wrapper around igt_subtest that will only execute the + * testcase if all of the flags passed to this function match those + * specified by the list of subtest
[Intel-gfx] [PATCH i-g-t v4 3/3] tests/gem_concurrent_all: Remove gem_concurrent_all.c
When gem_concurrent_blit was converted to use the new common framework for choosing whether or not to include slow/combinatorial tests, gem_concurrent_all became superfluous. This patch removes it. Signed-off-by: David Weinehall--- tests/Makefile.sources |1 - tests/gem_concurrent_all.c | 1540 2 files changed, 1541 deletions(-) delete mode 100644 tests/gem_concurrent_all.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index df92586a56fc..05ca603aa768 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -21,7 +21,6 @@ TESTS_progs_M = \ gem_caching \ gem_close_race \ gem_concurrent_blit \ - gem_concurrent_all \ gem_create \ gem_cs_tlb \ gem_ctx_param_basic \ diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c deleted file mode 100644 index 9b7ef8700e31.. --- a/tests/gem_concurrent_all.c +++ /dev/null @@ -1,1540 +0,0 @@ -/* - * Copyright © 2009,2012,2013 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - * - * Authors: - *Eric Anholt - *Chris Wilson - *Daniel Vetter - * - */ - -/** @file gem_concurrent.c - * - * This is a test of pread/pwrite/mmap behavior when writing to active - * buffers. - * - * Based on gem_gtt_concurrent_blt. - */ - -#include "igt.h" -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#include "intel_bufmgr.h" - -IGT_TEST_DESCRIPTION("Test of pread/pwrite/mmap behavior when writing to active" -" buffers."); - -#define LOCAL_I915_GEM_USERPTR 0x33 -#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr) -struct local_i915_gem_userptr { - uint64_t user_ptr; - uint64_t user_size; - uint32_t flags; - uint32_t handle; -}; - -int fd, devid, gen; -struct intel_batchbuffer *batch; -int all; -int pass; - -struct buffers { - const struct access_mode *mode; - drm_intel_bufmgr *bufmgr; - drm_intel_bo **src, **dst; - drm_intel_bo *snoop, *spare; - uint32_t *tmp; - int width, height, size; - int count; -}; - -#define MIN_BUFFERS 3 - -static void blt_copy_bo(struct buffers *b, drm_intel_bo *dst, drm_intel_bo *src); - -static void -nop_release_bo(drm_intel_bo *bo) -{ - drm_intel_bo_unreference(bo); -} - -static void -prw_set_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) -{ - for (int i = 0; i < b->size; i++) - b->tmp[i] = val; - drm_intel_bo_subdata(bo, 0, 4*b->size, b->tmp); -} - -static void -prw_cmp_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) -{ - uint32_t *vaddr; - - vaddr = b->tmp; - do_or_die(drm_intel_bo_get_subdata(bo, 0, 4*b->size, vaddr)); - for (int i = 0; i < b->size; i++) - igt_assert_eq_u32(vaddr[i], val); -} - -#define pixel(y, width) ((y)*(width) + (((y) + pass)%(width))) - -static void -partial_set_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) -{ - for (int y = 0; y < b->height; y++) - do_or_die(drm_intel_bo_subdata(bo, 4*pixel(y, b->width), 4, )); -} - -static void -partial_cmp_bo(struct buffers *b, drm_intel_bo *bo, uint32_t val) -{ - for (int y = 0; y < b->height; y++) { - uint32_t buf; - do_or_die(drm_intel_bo_get_subdata(bo, 4*pixel(y, b->width), 4, )); - igt_assert_eq_u32(buf, val); - } -} - -static drm_intel_bo * -create_normal_bo(drm_intel_bufmgr *bufmgr, uint64_t size) -{ - drm_intel_bo *bo; - - bo = drm_intel_bo_alloc(bufmgr, "bo", size, 0); - igt_assert(bo); - - return bo; -} - -static bool
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Support for pread/pwrite from/to non shmem backed objects
On 04/02/16 09:30, ankitprasad.r.sha...@intel.com wrote: From: Ankitprasad SharmaThis patch adds support for extending the pread/pwrite functionality for objects not backed by shmem. The access will be made through gtt interface. This will cover objects backed by stolen memory as well as other non-shmem backed objects. v2: Drop locks around slow_user_access, prefault the pages before access (Chris) v3: Rebased to the latest drm-intel-nightly (Ankit) v4: Moved page base & offset calculations outside the copy loop, corrected data types for size and offset variables, corrected if-else braces format (Tvrtko/kerneldocs) v5: Enabled pread/pwrite for all non-shmem backed objects including without tiling restrictions (Ankit) v6: Using pwrite_fast for non-shmem backed objects as well (Chris) v7: Updated commit message, Renamed i915_gem_gtt_read to i915_gem_gtt_copy, added pwrite slow path for non-shmem backed objects (Chris/Tvrtko) v8: Updated v7 commit message, mutex unlock around pwrite slow path for non-shmem backed objects (Tvrtko) v9: Corrected check during pread_ioctl, to avoid shmem_pread being called for non-shmem backed objects (Tvrtko) v10: Moved the write_domain check to needs_clflush and tiling mode check to pwrite_fast (Chris) v11: Use pwrite_fast fallback for all objects (shmem and non-shmem backed), call fast_user_write regardless of pagefault in previous iteration v12: Use page-by-page copy for slow user access too (Chris) v13: Handled EFAULT, Avoid use of WARN_ON, put_fence only if whole obj pinned (Chris) Testcase: igt/gem_stolen, igt/gem_pread, igt/gem_pwrite Signed-off-by: Ankitprasad Sharma --- drivers/gpu/drm/i915/i915_gem.c | 211 ++-- 1 file changed, 179 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ed8ae5d..40f2906 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -55,6 +55,9 @@ static bool cpu_cache_is_coherent(struct drm_device *dev, static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj) { + if (obj->base.write_domain == I915_GEM_DOMAIN_CPU) + return false; + if (!cpu_cache_is_coherent(obj->base.dev, obj->cache_level)) return true; @@ -646,6 +649,141 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, int page_length, return ret ? - EFAULT : 0; } +static inline uint64_t +slow_user_access(struct io_mapping *mapping, +uint64_t page_base, int page_offset, +char __user *user_data, +int length, bool pwrite) +{ Return type and length should be unsigned long to match __copy_to/from_user prototypes. + void __iomem *ioaddr; + void *vaddr; + uint64_t unwritten; + + ioaddr = io_mapping_map_wc(mapping, page_base); + /* We can use the cpu mem copy function because this is X86. */ + vaddr = (void __force *)ioaddr + page_offset; + if (pwrite) + unwritten = __copy_from_user(vaddr, user_data, length); + else + unwritten = __copy_to_user(user_data, vaddr, length); + + io_mapping_unmap(ioaddr); + return unwritten; +} + +static int +i915_gem_gtt_pread(struct drm_device *dev, + struct drm_i915_gem_object *obj, uint64_t size, + uint64_t data_offset, uint64_t data_ptr) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_mm_node node; + char __user *user_data; + uint64_t remain; + uint64_t offset; + int ret = 0; No need to initialize. + + ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE); + if (ret) { + ret = insert_mappable_node(dev_priv, , PAGE_SIZE); + if (ret) + goto out; + + ret = i915_gem_object_get_pages(obj); + if (ret) { + remove_mappable_node(); + goto out; + } + + i915_gem_object_pin_pages(obj); + } else { + node.start = i915_gem_obj_ggtt_offset(obj); + node.allocated = false; + ret = i915_gem_object_put_fence(obj); + if (ret) + goto out_unpin; + } + + ret = i915_gem_object_set_to_gtt_domain(obj, false); + if (ret) + goto out_unpin; + + user_data = to_user_ptr(data_ptr); + remain = size; + offset = i915_gem_obj_ggtt_offset(obj) + data_offset; This is wrong for the page-by-page (no pin) mode. + + mutex_unlock(>struct_mutex); + if (likely(!i915.prefault_disable)) { + ret = fault_in_multipages_writeable(user_data, remain); + if (ret) { + mutex_lock(>struct_mutex); + goto out_unpin; +
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Don't ERROR for an expected intel_rcs_ctx_init() interruption
On 29/01/16 16:49, Chris Wilson wrote: intel_rcs_ctx_init() can be interrupted by a signal (if it has to wait upon a full ring to advance). Don't emit an error for this. Testcase: igt/gem_concurrent_blit Signed-off-by: Chris Wilson--- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 6f5b511bdb5d..b813bbc8e41c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -746,9 +746,9 @@ static int intel_rcs_ctx_init(struct drm_i915_gem_request *req) ret = i915_gem_render_state_init(req); if (ret) - DRM_ERROR("init render state: %d\n", ret); + return ret; - return ret; + return 0; } static int wa_add(struct drm_i915_private *dev_priv, Or just "return i915_gem_render_state_init(req);", but that is way below the threshold. Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx