[Intel-gfx] [PATCH] drm/i915: Remove references to previously removed UMS config option
Commit 03dae59c72d8 ("drm/i915: Ditch UMS config option") removed CONFIG_DRM_I915_UMS from the Kconfig file, but i915_drv.c still references this option in two #ifndef statements. As an undefined config option will always be 'false', we can drop the #ifndefs alltogether and adapt the printed error message. This inconsistency was found with the undertaker tool. Signed-off-by: Andreas Ruprecht --- drivers/gpu/drm/i915/i915_drv.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8039cec..4ecf85f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1630,11 +1630,9 @@ static int __init i915_init(void) if (!(driver.driver_features & DRIVER_MODESET)) { driver.get_vblank_timestamp = NULL; -#ifndef CONFIG_DRM_I915_UMS /* Silently fail loading to not upset userspace. */ - DRM_DEBUG_DRIVER("KMS and UMS disabled.\n"); + DRM_DEBUG_DRIVER("KMS disabled.\n"); return 0; -#endif } /* @@ -1650,10 +1648,8 @@ static int __init i915_init(void) static void __exit i915_exit(void) { -#ifndef CONFIG_DRM_I915_UMS if (!(driver.driver_features & DRIVER_MODESET)) return; /* Never loaded a driver. */ -#endif drm_pci_exit(&driver, &i915_pci_driver); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] PCH fifo underrun in 3.18
On Fri, 30 Jan 2015, jon wrote: > Just updated my thinkpad (x230, ivy bridge platform) to 3.18 and boot > fails with the error 'PCH fifo underrun'. This is under fedora 21 with > the fedora kernel version 3.18.3-201 Please file a bug on [1], attach a dmesg from boot to the problem with drm.debug=14 module parameter set. Reference this thread. Thanks, Jani. [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl: Add check for minimum allocable Display Data Blocks
Fifo Underrun is observed when allocating < minimum allocable blocks for any plane, This patch calculate & checks for upper & lower DDB bound for each plane according to total allocated DDB for that Pipe. Signed-off-by: Kumar, Mahesh --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c | 48 + 2 files changed, 50 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 26ffe8b..fe51a5a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1536,6 +1536,8 @@ struct skl_ddb_allocation { struct skl_ddb_entry pipe[I915_MAX_PIPES]; struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; struct skl_ddb_entry cursor[I915_MAX_PIPES]; + uint16_t min_alloc[I915_MAX_PIPES][I915_MAX_PLANES]; + uint16_t max_alloc[I915_MAX_PIPES][I915_MAX_PLANES]; }; struct skl_wm_values { diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3c64810..d4d8994 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2482,6 +2482,43 @@ skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc, } static void +skl_calculate_allocable_blocks(struct intel_crtc *intel_crtc, + const struct skl_pipe_wm_parameters *params, + uint16_t alloc_size, struct skl_ddb_allocation *ddb) +{ + uint16_t min; + uint16_t total_min_alloc = 0; + enum pipe pipe = intel_crtc->pipe; + int plane; + + for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) { + const struct intel_plane_wm_parameters *p; + + p = ¶ms->plane[plane]; + ddb->min_alloc[pipe][plane] = 0; + + if (!p->enabled) + continue; + + /* +* TODO: Calculate PlaneMinAlloc according to X/Y-Tiling +* calculation, for now use X-Tiling PlaneMinAlloc +*/ + + min = 8; + + ddb->min_alloc[pipe][plane] = min; + total_min_alloc += min; + + } + + for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) { + ddb->max_alloc[pipe][plane] = alloc_size - total_min_alloc + + ddb->min_alloc[pipe][plane]; + } +} + +static void skl_allocate_pipe_ddb(struct drm_crtc *crtc, const struct intel_wm_config *config, const struct skl_pipe_wm_parameters *params, @@ -2519,6 +2556,8 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, total_data_rate = skl_get_total_relative_data_rate(intel_crtc, params); start = alloc->start; + + skl_calculate_allocable_blocks(intel_crtc, params, alloc_size, ddb); for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) { const struct intel_plane_wm_parameters *p; unsigned int data_rate; @@ -2537,6 +2576,15 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, plane_blocks = div_u64((uint64_t)alloc_size * data_rate, total_data_rate); + /* +* Limit plane_blocks if out of limit +*/ + + if (plane_blocks > ddb->max_alloc[pipe][plane]) + plane_blocks = ddb->max_alloc[pipe][plane]; + if (plane_blocks < ddb->min_alloc[pipe][plane]) + plane_blocks = ddb->min_alloc[pipe][plane]; + ddb->plane[pipe][plane].start = start; ddb->plane[pipe][plane].end = start + plane_blocks; -- 2.3.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/skl: Updated the gen6_rps_limits function
On Mon, Feb 09, 2015 at 10:26:33AM +0530, Akash Goel wrote: > On Fri, 2015-02-06 at 15:43 +, Chris Wilson wrote: > > On Fri, Feb 06, 2015 at 08:26:35PM +0530, akash.g...@intel.com wrote: > > > From: Akash Goel > > > > > > RP Interrupt Up/Down Frequency Limits register (A014) definition > > > has changed for SKL. Updated the gen6_rps_limits function as per that > > > > > > Signed-off-by: Akash Goel > > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 16 +++- > > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 215b200..db24b48 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -3623,7 +3623,7 @@ static void ironlake_disable_drps(struct drm_device > > > *dev) > > > * ourselves, instead of doing a rmw cycle (which might result in us > > > clearing > > > * all limits and the gpu stuck at whatever frequency it is at atm). > > > */ > > > -static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val) > > > +static u32 get_rps_limits(struct drm_i915_private *dev_priv, u8 val) > > > > Spurious name change, it doesn't seem to add anything or clear up any > > confusion with vlv. > Fine will keep the original name, thought would be better to give a > generic name to the function and abstract the platform specific > differences inside its definition. Generic would be intel_rps_limits(). I am wary of using get(), the common idiom is for a getter to return ownership as well, e.g. kref_get(), intel_uncore_forcewake_get(). Also outside of trivial getters and setters, get is such a generic verb that I don't think it adds much self-documentating value, especially when breaking established patterns. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Correct the base value while updating LP_OUTPUT_HOLD in MIPI_PORT_CTRL
On 02/06/2015 01:11 AM, shuang...@intel.com wrote: > Tested-By: PRC QA PRTS (Patch Regression Test System Contact: > shuang...@intel.com) > Task id: 5718 > -Summary- > Platform Delta drm-intel-nightly Series Applied > PNV -8 282/283 274/283 > ILK +1 316/319 317/319 > SNB 322/346 322/346 > IVB -1 382/384 381/384 > BYT 296/296 296/296 > HSW 425/428 425/428 > BDW 318/333 318/333 > -Detailed- > Platform Testdrm-intel-nightly > Series Applied > *PNV igt_gem_fence_thrash_bo-write-verify-y PASS(2, M23M7) FAIL(1, > M7) > PNV igt_gem_userptr_blits_coherency-sync CRASH(2, M7)PASS(1, M23) > CRASH(1, M7) > PNV igt_gem_userptr_blits_coherency-unsync CRASH(2, M7)PASS(1, M23) > CRASH(1, M7) > *PNV igt_gem_userptr_blits_forked-sync-interruptible PASS(2, M23M7) > DMESG_WARN(1, M7) > *PNV igt_gem_userptr_blits_forked-sync-multifd-interruptible PASS(2, > M23M7) NO_RESULT(1, M7) > PNV igt_gen3_render_linear_blits FAIL(3, M7)CRASH(1, M23)PASS(4, > M25M23) FAIL(1, M7) > PNV igt_gen3_render_mixed_blits FAIL(3, M7)PASS(1, M23) FAIL(1, > M7) > PNV igt_gen3_render_tiledx_blits FAIL(2, M7)PASS(1, M23) FAIL(1, > M7) > PNV igt_gem_tiled_pread_pwrite FAIL(2, M7)PASS(1, M23) FAIL(1, M7) > *ILK igt_drv_suspend_forcewake DMESG_WARN(5, M26M37) PASS(1, M26) > IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance > DMESG_WARN(2, M34)PASS(5, M21M34) DMESG_WARN(1, M21) > Note: You need to pay more attention to line start with '*' Not sure why this is giving issues for PNV and ILK. Patch is strictly for DSI. Are you expecting me to do anything about this ? Regards Shobhit > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
On 02/05/2015 10:44 PM, Ville Syrjälä wrote: > On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote: >> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target >> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the >> port ID should be enough to identify devices unless they are MFD. The >> SB_DevFn was intended to remove ambiguity in case of these MFD devices. >> >> For non MFD devices the recommendation for the target device IP was to >> ignore these fields, but not all of them followed the recommendation. >> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so >> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because >> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0). >> It turned out that this did not follow the recommendation and expected 0 >> in this field. >> >> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for >> all devices except target PCI devices. >> >> Signed-off-by: Shobhit Kumar > > Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW > GPIONC seems to have become more relaxed and accepts devfn 2.0 as well. > > I also tried to poke at all the other sideband units we care about and > they appear to work just as well with 2.0 and 0.0. But being consistent > seems like a good idea, so 0.0 it is for everyone. > > Reviewed-by: Ville Syrjälä Thanks Ville. Daniel this needs more testing/reviews to merge ? Regards Shobhit > >> --- >> drivers/gpu/drm/i915/intel_sideband.c | 26 +- >> 1 file changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_sideband.c >> b/drivers/gpu/drm/i915/intel_sideband.c >> index 01d841e..731b10a 100644 >> --- a/drivers/gpu/drm/i915/intel_sideband.c >> +++ b/drivers/gpu/drm/i915/intel_sideband.c >> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 >> addr) >> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >> >> mutex_lock(&dev_priv->dpio_lock); >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT, >> SB_CRRDDA_NP, addr, &val); >> mutex_unlock(&dev_priv->dpio_lock); >> >> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 >> addr, u32 val) >> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >> >> mutex_lock(&dev_priv->dpio_lock); >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT, >> SB_CRWRDA_NP, addr, &val); >> mutex_unlock(&dev_priv->dpio_lock); >> } >> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, >> u32 reg) >> { >> u32 val = 0; >> >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT, >> SB_CRRDDA_NP, reg, &val); >> >> return val; >> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, >> u32 reg) >> >> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val) >> { >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT, >> SB_CRWRDA_NP, reg, &val); >> } >> >> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 >> addr) >> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >> >> mutex_lock(&dev_priv->dpio_lock); >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC, >> SB_CRRDDA_NP, addr, &val); >> mutex_unlock(&dev_priv->dpio_lock); >> >> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 >> addr) >> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg) >> { >> u32 val = 0; >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC, >> SB_CRRDDA_NP, reg, &val); >> return val; >> } >> >> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val) >> { >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC, >> SB_CRWRDA_NP, reg, &val); >> } >> >> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg) >> { >> u32 val = 0; >> -vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK, >> +vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK, >> SB_CRRDDA_NP, reg, &val); >> return val; >> } >> >> void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val) >> { >> -vlv_sideband_rw(dev_priv, PCI_D
Re: [Intel-gfx] [PATCH 4/7] drm/i915/skl: Updated the gen6_rps_limits function
On Mon, 2015-02-09 at 11:03 +, Chris Wilson wrote: > On Mon, Feb 09, 2015 at 10:26:33AM +0530, Akash Goel wrote: > > On Fri, 2015-02-06 at 15:43 +, Chris Wilson wrote: > > > On Fri, Feb 06, 2015 at 08:26:35PM +0530, akash.g...@intel.com wrote: > > > > From: Akash Goel > > > > > > > > RP Interrupt Up/Down Frequency Limits register (A014) definition > > > > has changed for SKL. Updated the gen6_rps_limits function as per that > > > > > > > > Signed-off-by: Akash Goel > > > > --- > > > > drivers/gpu/drm/i915/intel_pm.c | 16 +++- > > > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > index 215b200..db24b48 100644 > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > @@ -3623,7 +3623,7 @@ static void ironlake_disable_drps(struct > > > > drm_device *dev) > > > > * ourselves, instead of doing a rmw cycle (which might result in us > > > > clearing > > > > * all limits and the gpu stuck at whatever frequency it is at atm). > > > > */ > > > > -static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val) > > > > +static u32 get_rps_limits(struct drm_i915_private *dev_priv, u8 val) > > > > > > Spurious name change, it doesn't seem to add anything or clear up any > > > confusion with vlv. > > Fine will keep the original name, thought would be better to give a > > generic name to the function and abstract the platform specific > > differences inside its definition. > > Generic would be intel_rps_limits(). I am wary of using get(), the > common idiom is for a getter to return ownership as well, e.g. > kref_get(), intel_uncore_forcewake_get(). Also outside of trivial getters > and setters, get is such a generic verb that I don't think it adds much > self-documentating value, especially when breaking established patterns. Understood, using get() is a misnomer & would be inconsistent with the existing naming patterns. Will rename it to 'intel_rps_limits'. Thanks for the clarification. > -Chris > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
On Mon, 09 Feb 2015, Shobhit Kumar wrote: > On 02/05/2015 10:44 PM, Ville Syrjälä wrote: >> On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote: >>> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target >>> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the >>> port ID should be enough to identify devices unless they are MFD. The >>> SB_DevFn was intended to remove ambiguity in case of these MFD devices. >>> >>> For non MFD devices the recommendation for the target device IP was to >>> ignore these fields, but not all of them followed the recommendation. >>> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so >>> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because >>> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0). >>> It turned out that this did not follow the recommendation and expected 0 >>> in this field. >>> >>> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for >>> all devices except target PCI devices. >>> >>> Signed-off-by: Shobhit Kumar >> >> Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW >> GPIONC seems to have become more relaxed and accepts devfn 2.0 as well. >> >> I also tried to poke at all the other sideband units we care about and >> they appear to work just as well with 2.0 and 0.0. But being consistent >> seems like a good idea, so 0.0 it is for everyone. >> >> Reviewed-by: Ville Syrjälä > > Thanks Ville. Daniel this needs more testing/reviews to merge ? Pushed to drm-intel-next-fixes with cc: stable, thanks for the patch and review. BR, Jani. > > Regards > Shobhit > >> >>> --- >>> drivers/gpu/drm/i915/intel_sideband.c | 26 +- >>> 1 file changed, 13 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_sideband.c >>> b/drivers/gpu/drm/i915/intel_sideband.c >>> index 01d841e..731b10a 100644 >>> --- a/drivers/gpu/drm/i915/intel_sideband.c >>> +++ b/drivers/gpu/drm/i915/intel_sideband.c >>> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 >>> addr) >>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >>> >>> mutex_lock(&dev_priv->dpio_lock); >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT, >>> SB_CRRDDA_NP, addr, &val); >>> mutex_unlock(&dev_priv->dpio_lock); >>> >>> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, >>> u8 addr, u32 val) >>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >>> >>> mutex_lock(&dev_priv->dpio_lock); >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT, >>> SB_CRWRDA_NP, addr, &val); >>> mutex_unlock(&dev_priv->dpio_lock); >>> } >>> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, >>> u32 reg) >>> { >>> u32 val = 0; >>> >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT, >>> SB_CRRDDA_NP, reg, &val); >>> >>> return val; >>> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, >>> u32 reg) >>> >>> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val) >>> { >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT, >>> SB_CRWRDA_NP, reg, &val); >>> } >>> >>> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 >>> addr) >>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >>> >>> mutex_lock(&dev_priv->dpio_lock); >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC, >>> SB_CRRDDA_NP, addr, &val); >>> mutex_unlock(&dev_priv->dpio_lock); >>> >>> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 >>> addr) >>> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg) >>> { >>> u32 val = 0; >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC, >>> SB_CRRDDA_NP, reg, &val); >>> return val; >>> } >>> >>> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val) >>> { >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC, >>> SB_CRWRDA_NP, reg, &val); >>> } >>> >>> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg) >>> { >>> u32 val = 0; >>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK, >>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0,
Re: [Intel-gfx] [PATCH] drm/i915: Take runtime pm reference on hangcheck_info
On Thu, 05 Feb 2015, Chris Wilson wrote: > On Thu, Feb 05, 2015 at 06:41:48PM +0200, Mika Kuoppala wrote: >> We read the coherent current seqno and actual head from ring. >> For hardware access we need to take runtime_pm reference. >> >> Get hardware specific values with runtime reference held >> and print them first to emphasize hw state vs bookkeepping. >> >> v2: Reorder output according to hw access (Chris) >> remove superfluous locking (Daniel) >> >> Testcase: igt/pm_rpm/debugfs-read >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88910 >> Tested-by: Ding Heng (v1) >> Signed-off-by: Mika Kuoppala > > I'm happy with that: > Reviewed-by: Chris Wilson Pushed to drm-intel-next-fixes, thanks for the patch and review. BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Squelch overzealous uncore reset WARN_ON
On Thu, 05 Feb 2015, Chris Wilson wrote: > On Thu, Feb 05, 2015 at 05:45:42PM +0200, Mika Kuoppala wrote: >> We added this WARN_ON to guard against using uninitialized >> forcewake domains. But forgot blissfully that not all >> gens have forcewake domains in the first place. >> >> v2: Move WARN_ON to fw_domains_init (Chris) >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88911 >> Tested-by: Ding Heng (v1) >> Signed-off-by: Mika Kuoppala >> --- >> drivers/gpu/drm/i915/intel_uncore.c | 8 +++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c >> b/drivers/gpu/drm/i915/intel_uncore.c >> index 76b60a3..e10bcfc 100644 >> --- a/drivers/gpu/drm/i915/intel_uncore.c >> +++ b/drivers/gpu/drm/i915/intel_uncore.c >> @@ -166,7 +166,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum >> forcewake_domains fw_do >> struct intel_uncore_forcewake_domain *d; >> enum forcewake_domain_id id; >> >> -WARN_ON(dev_priv->uncore.fw_domains == 0); >> +if (dev_priv->uncore.fw_domains == 0) >> +return; >> >> for_each_fw_domain_mask(d, fw_domains, dev_priv, id) >> fw_domain_reset(d); >> @@ -997,6 +998,9 @@ static void intel_uncore_fw_domains_init(struct >> drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> +if (INTEL_INFO(dev_priv->dev)->gen <= 5) >> +return; >> + >> if (IS_GEN9(dev)) { >> dev_priv->uncore.funcs.force_wake_get = fw_domains_get; >> dev_priv->uncore.funcs.force_wake_put = fw_domains_put; >> @@ -1069,6 +1073,8 @@ static void intel_uncore_fw_domains_init(struct >> drm_device *dev) >> fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER, >> FORCEWAKE, FORCEWAKE_ACK); >> } >> + > Maybe: /* All future platforms are expected to require complex power gating */ >> +WARN_ON(dev_priv->uncore.fw_domains == 0); >> } > > Reviewed-by: Chris Wilson Pushed to drm-intel-next-fixes, thanks for the patch and review. BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop vblank wait from intel_dp_link_down
On Wed, 26 Nov 2014, Paulo Zanoni wrote: > 2014-11-24 13:54 GMT-02:00 Daniel Vetter : >> Nothing in Bspec seems to indicate that we actually needs this, and it >> looks like can't work since by this point the pipe is off and so >> vblanks won't really happen any more. >> >> Note that Bspec mentions that it takes a vblank for this bit to >> change, but _only_ when enabling. >> >> Dropping this code quenches an annoying backtrace introduced by the >> more anal checking since >> >> commit 51e31d49c89055299e34b8f44d13f70e19d1 >> Author: Daniel Vetter >> Date: Mon Sep 15 12:36:02 2014 +0200 >> >> drm/i915: Use generic vblank wait >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86095 >> Signed-off-by: Daniel Vetter >> --- >> drivers/gpu/drm/i915/intel_dp.c | 17 + >> 1 file changed, 1 insertion(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c >> b/drivers/gpu/drm/i915/intel_dp.c >> index 46731da407c0..63fcdbf90652 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -3514,8 +3514,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) >> enum port port = intel_dig_port->port; >> struct drm_device *dev = intel_dig_port->base.base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_crtc *intel_crtc = >> - to_intel_crtc(intel_dig_port->base.base.crtc); >> uint32_t DP = intel_dp->DP; >> >> if (WARN_ON(HAS_DDI(dev))) >> @@ -3540,8 +3538,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) >> >> if (HAS_PCH_IBX(dev) && >> I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { >> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc; >> - >> /* Hardware workaround: leaving our transcoder select >> * set to transcoder B while it's off will prevent the >> * corresponding HDMI output on transcoder A. >> @@ -3552,18 +3548,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) >> */ >> DP &= ~DP_PIPEB_SELECT; >> I915_WRITE(intel_dp->output_reg, DP); >> - >> - /* Changes to enable or select take place the vblank >> -* after being written. >> -*/ >> - if (WARN_ON(crtc == NULL)) { >> - /* We should never try to disable a port without a >> crtc >> -* attached. For paranoia keep the code around for a >> -* bit. */ >> - POSTING_READ(intel_dp->output_reg); >> - msleep(50); >> - } else >> - intel_wait_for_vblank(dev, intel_crtc->pipe); > > What I can guess is that we have the vblank wait here because the > DP_PORT_EN bit is still enabled at this point. It would make some > sense to have it if the pipe were not off... So removing the waits > looks sane: Reviewed-by: Paulo Zanoni > > But when I read the spec, it makes me think that maybe doing the > I915_WRITE above is also wrong, since the port is still enabled. Maybe > we should only clear bit 30 in the same write as the one that clears > bit 31? Ugh. So the spec says, "When disabling the port, software must temporarily enable the port with transcoder select (bit #30) cleared to ‘0’ after disabling the port." IIUC we should disable like we normally do, and do the w/a by enabling and disabling the port with DP_PIPEB_SELECT cleared *after* that. I think the current code is wrong, the patch is wrong, what Paulo suggests is wrong, and also intel_disable_hdmi() is wrong. BR, Jani. > > >> + POSTING_READ(intel_dp->output_reg); >> } >> >> DP &= ~DP_AUDIO_OUTPUT_ENABLE; >> -- >> 2.1.1 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Add check for minimum allocable Display Data Blocks
On Mon, Feb 09, 2015 at 03:06:09PM +0530, Kumar, Mahesh wrote: > Fifo Underrun is observed when allocating < minimum allocable blocks > for any plane, This patch calculate & checks for upper & lower DDB > bound for each plane according to total allocated DDB for that Pipe. > > Signed-off-by: Kumar, Mahesh Hi, It seems that my fix for that got lost (sigh): http://lists.freedesktop.org/archives/intel-gfx/2014-October/053713.html Would you mind reviewing that one instead? -- Damien > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_pm.c | 48 > + > 2 files changed, 50 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 26ffe8b..fe51a5a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1536,6 +1536,8 @@ struct skl_ddb_allocation { > struct skl_ddb_entry pipe[I915_MAX_PIPES]; > struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; > struct skl_ddb_entry cursor[I915_MAX_PIPES]; > + uint16_t min_alloc[I915_MAX_PIPES][I915_MAX_PLANES]; > + uint16_t max_alloc[I915_MAX_PIPES][I915_MAX_PLANES]; > }; > > struct skl_wm_values { > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 3c64810..d4d8994 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2482,6 +2482,43 @@ skl_get_total_relative_data_rate(struct intel_crtc > *intel_crtc, > } > > static void > +skl_calculate_allocable_blocks(struct intel_crtc *intel_crtc, > + const struct skl_pipe_wm_parameters *params, > + uint16_t alloc_size, struct skl_ddb_allocation *ddb) > +{ > + uint16_t min; > + uint16_t total_min_alloc = 0; > + enum pipe pipe = intel_crtc->pipe; > + int plane; > + > + for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) { > + const struct intel_plane_wm_parameters *p; > + > + p = ¶ms->plane[plane]; > + ddb->min_alloc[pipe][plane] = 0; > + > + if (!p->enabled) > + continue; > + > + /* > + * TODO: Calculate PlaneMinAlloc according to X/Y-Tiling > + * calculation, for now use X-Tiling PlaneMinAlloc > + */ > + > + min = 8; > + > + ddb->min_alloc[pipe][plane] = min; > + total_min_alloc += min; > + > + } > + > + for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) { > + ddb->max_alloc[pipe][plane] = alloc_size - total_min_alloc + > + ddb->min_alloc[pipe][plane]; > + } > +} > + > +static void > skl_allocate_pipe_ddb(struct drm_crtc *crtc, > const struct intel_wm_config *config, > const struct skl_pipe_wm_parameters *params, > @@ -2519,6 +2556,8 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, > total_data_rate = skl_get_total_relative_data_rate(intel_crtc, params); > > start = alloc->start; > + > + skl_calculate_allocable_blocks(intel_crtc, params, alloc_size, ddb); > for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) { > const struct intel_plane_wm_parameters *p; > unsigned int data_rate; > @@ -2537,6 +2576,15 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, > plane_blocks = div_u64((uint64_t)alloc_size * data_rate, > total_data_rate); > > + /* > + * Limit plane_blocks if out of limit > + */ > + > + if (plane_blocks > ddb->max_alloc[pipe][plane]) > + plane_blocks = ddb->max_alloc[pipe][plane]; > + if (plane_blocks < ddb->min_alloc[pipe][plane]) > + plane_blocks = ddb->min_alloc[pipe][plane]; > + > ddb->plane[pipe][plane].start = start; > ddb->plane[pipe][plane].end = start + plane_blocks; > > -- > 2.3.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/skl: Make sure to allocate mininum sizes in the DDB
I overlooked the fact that we need to allocate a minimum 8 blocks and that just allocating the planes depending on how much they need to fetch from the DDB in proportion of how much memory bw is necessary for the whole display can lead to cases where we don't respect those minima (and thus overrun). So, instead, start by allocating 8 blocks to each active display plane and then allocate the remaining blocks like before. v2: Rebase on top of -nightly Cc: Mahesh Kumar Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index bebefe7..f6c7e53 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2502,6 +2502,7 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, enum pipe pipe = intel_crtc->pipe; struct skl_ddb_entry *alloc = &ddb->pipe[pipe]; uint16_t alloc_size, start, cursor_blocks; + uint16_t minimum[I915_MAX_PLANES]; unsigned int total_data_rate; int plane; @@ -2520,9 +2521,21 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, alloc_size -= cursor_blocks; alloc->end -= cursor_blocks; + /* 1. Allocate the mininum required blocks for each active plane */ + for_each_plane(pipe, plane) { + const struct intel_plane_wm_parameters *p; + + p = ¶ms->plane[plane]; + if (!p->enabled) + continue; + + minimum[plane] = 8; + alloc_size -= minimum[plane]; + } + /* -* Each active plane get a portion of the remaining space, in -* proportion to the amount of data they need to fetch from memory. +* 2. Distribute the remaining space in proportion to the amount of +* data each plane needs to fetch from memory. * * FIXME: we may not allocate every single block here. */ @@ -2544,8 +2557,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, * promote the expression to 64 bits to avoid overflowing, the * result is < available as data_rate / total_data_rate < 1 */ - plane_blocks = div_u64((uint64_t)alloc_size * data_rate, - total_data_rate); + plane_blocks = minimum[plane]; + plane_blocks += div_u64((uint64_t)alloc_size * data_rate, + total_data_rate); ddb->plane[pipe][plane].start = start; ddb->plane[pipe][plane].end = start + plane_blocks; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop vblank wait from intel_dp_link_down
On Mon, Feb 09, 2015 at 03:15:56PM +0200, Jani Nikula wrote: > On Wed, 26 Nov 2014, Paulo Zanoni wrote: > > 2014-11-24 13:54 GMT-02:00 Daniel Vetter : > >> Nothing in Bspec seems to indicate that we actually needs this, and it > >> looks like can't work since by this point the pipe is off and so > >> vblanks won't really happen any more. > >> > >> Note that Bspec mentions that it takes a vblank for this bit to > >> change, but _only_ when enabling. > >> > >> Dropping this code quenches an annoying backtrace introduced by the > >> more anal checking since > >> > >> commit 51e31d49c89055299e34b8f44d13f70e19d1 > >> Author: Daniel Vetter > >> Date: Mon Sep 15 12:36:02 2014 +0200 > >> > >> drm/i915: Use generic vblank wait > >> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86095 > >> Signed-off-by: Daniel Vetter > >> --- > >> drivers/gpu/drm/i915/intel_dp.c | 17 + > >> 1 file changed, 1 insertion(+), 16 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dp.c > >> b/drivers/gpu/drm/i915/intel_dp.c > >> index 46731da407c0..63fcdbf90652 100644 > >> --- a/drivers/gpu/drm/i915/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/intel_dp.c > >> @@ -3514,8 +3514,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) > >> enum port port = intel_dig_port->port; > >> struct drm_device *dev = intel_dig_port->base.base.dev; > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> - struct intel_crtc *intel_crtc = > >> - to_intel_crtc(intel_dig_port->base.base.crtc); > >> uint32_t DP = intel_dp->DP; > >> > >> if (WARN_ON(HAS_DDI(dev))) > >> @@ -3540,8 +3538,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) > >> > >> if (HAS_PCH_IBX(dev) && > >> I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { > >> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc; > >> - > >> /* Hardware workaround: leaving our transcoder select > >> * set to transcoder B while it's off will prevent the > >> * corresponding HDMI output on transcoder A. > >> @@ -3552,18 +3548,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) > >> */ > >> DP &= ~DP_PIPEB_SELECT; > >> I915_WRITE(intel_dp->output_reg, DP); > >> - > >> - /* Changes to enable or select take place the vblank > >> -* after being written. > >> -*/ > >> - if (WARN_ON(crtc == NULL)) { > >> - /* We should never try to disable a port without a > >> crtc > >> -* attached. For paranoia keep the code around for > >> a > >> -* bit. */ > >> - POSTING_READ(intel_dp->output_reg); > >> - msleep(50); > >> - } else > >> - intel_wait_for_vblank(dev, intel_crtc->pipe); > > > > What I can guess is that we have the vblank wait here because the > > DP_PORT_EN bit is still enabled at this point. It would make some > > sense to have it if the pipe were not off... So removing the waits > > looks sane: Reviewed-by: Paulo Zanoni > > > > But when I read the spec, it makes me think that maybe doing the > > I915_WRITE above is also wrong, since the port is still enabled. Maybe > > we should only clear bit 30 in the same write as the one that clears > > bit 31? > > Ugh. So the spec says, "When disabling the port, software must > temporarily enable the port with transcoder select (bit #30) cleared to > ‘0’ after disabling the port." > > IIUC we should disable like we normally do, and do the w/a by enabling > and disabling the port with DP_PIPEB_SELECT cleared *after* that. I > think the current code is wrong, the patch is wrong, what Paulo suggests > is wrong, and also intel_disable_hdmi() is wrong. This code has been bugging me for a long time as well. IIRC I even had cooked up some patches to do the re-enable as you suggest since I read the spec the same way. But I never had enough time to test it. And in order to really test it I would first like to actually reproduce the problem that the workaround is supposed to fix. How else would you know if the workaround is correct after all. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop vblank wait from intel_dp_link_down
On Mon, 09 Feb 2015, Ville Syrjälä wrote: > On Mon, Feb 09, 2015 at 03:15:56PM +0200, Jani Nikula wrote: >> On Wed, 26 Nov 2014, Paulo Zanoni wrote: >> > 2014-11-24 13:54 GMT-02:00 Daniel Vetter : >> >> Nothing in Bspec seems to indicate that we actually needs this, and it >> >> looks like can't work since by this point the pipe is off and so >> >> vblanks won't really happen any more. >> >> >> >> Note that Bspec mentions that it takes a vblank for this bit to >> >> change, but _only_ when enabling. >> >> >> >> Dropping this code quenches an annoying backtrace introduced by the >> >> more anal checking since >> >> >> >> commit 51e31d49c89055299e34b8f44d13f70e19d1 >> >> Author: Daniel Vetter >> >> Date: Mon Sep 15 12:36:02 2014 +0200 >> >> >> >> drm/i915: Use generic vblank wait >> >> >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86095 >> >> Signed-off-by: Daniel Vetter >> >> --- >> >> drivers/gpu/drm/i915/intel_dp.c | 17 + >> >> 1 file changed, 1 insertion(+), 16 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c >> >> b/drivers/gpu/drm/i915/intel_dp.c >> >> index 46731da407c0..63fcdbf90652 100644 >> >> --- a/drivers/gpu/drm/i915/intel_dp.c >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> >> @@ -3514,8 +3514,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) >> >> enum port port = intel_dig_port->port; >> >> struct drm_device *dev = intel_dig_port->base.base.dev; >> >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> - struct intel_crtc *intel_crtc = >> >> - to_intel_crtc(intel_dig_port->base.base.crtc); >> >> uint32_t DP = intel_dp->DP; >> >> >> >> if (WARN_ON(HAS_DDI(dev))) >> >> @@ -3540,8 +3538,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) >> >> >> >> if (HAS_PCH_IBX(dev) && >> >> I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { >> >> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc; >> >> - >> >> /* Hardware workaround: leaving our transcoder select >> >> * set to transcoder B while it's off will prevent the >> >> * corresponding HDMI output on transcoder A. >> >> @@ -3552,18 +3548,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) >> >> */ >> >> DP &= ~DP_PIPEB_SELECT; >> >> I915_WRITE(intel_dp->output_reg, DP); >> >> - >> >> - /* Changes to enable or select take place the vblank >> >> -* after being written. >> >> -*/ >> >> - if (WARN_ON(crtc == NULL)) { >> >> - /* We should never try to disable a port without >> >> a crtc >> >> -* attached. For paranoia keep the code around >> >> for a >> >> -* bit. */ >> >> - POSTING_READ(intel_dp->output_reg); >> >> - msleep(50); >> >> - } else >> >> - intel_wait_for_vblank(dev, intel_crtc->pipe); >> > >> > What I can guess is that we have the vblank wait here because the >> > DP_PORT_EN bit is still enabled at this point. It would make some >> > sense to have it if the pipe were not off... So removing the waits >> > looks sane: Reviewed-by: Paulo Zanoni >> > >> > But when I read the spec, it makes me think that maybe doing the >> > I915_WRITE above is also wrong, since the port is still enabled. Maybe >> > we should only clear bit 30 in the same write as the one that clears >> > bit 31? >> >> Ugh. So the spec says, "When disabling the port, software must >> temporarily enable the port with transcoder select (bit #30) cleared to >> ‘0’ after disabling the port." >> >> IIUC we should disable like we normally do, and do the w/a by enabling >> and disabling the port with DP_PIPEB_SELECT cleared *after* that. I >> think the current code is wrong, the patch is wrong, what Paulo suggests >> is wrong, and also intel_disable_hdmi() is wrong. > > This code has been bugging me for a long time as well. IIRC I even had > cooked up some patches to do the re-enable as you suggest since I > read the spec the same way. But I never had enough time to test it. And > in order to really test it I would first like to actually reproduce the > problem that the workaround is supposed to fix. How else would you know > if the workaround is correct after all. *sigh* an alternative is to apply Daniel's patch and add a comment there's something fishy. Jani. > > -- > Ville Syrjälä > Intel OTC -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Make sure to allocate mininum sizes in the DDB
On Mon, Feb 09, 2015 at 01:35:10PM +, Damien Lespiau wrote: > I overlooked the fact that we need to allocate a minimum 8 blocks and > that just allocating the planes depending on how much they need to fetch > from the DDB in proportion of how much memory bw is necessary for the > whole display can lead to cases where we don't respect those minima (and > thus overrun). > > So, instead, start by allocating 8 blocks to each active display plane > and then allocate the remaining blocks like before. > > v2: Rebase on top of -nightly > > Cc: Mahesh Kumar > Signed-off-by: Damien Lespiau > --- > drivers/gpu/drm/i915/intel_pm.c | 22 ++ > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index bebefe7..f6c7e53 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2502,6 +2502,7 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, > enum pipe pipe = intel_crtc->pipe; > struct skl_ddb_entry *alloc = &ddb->pipe[pipe]; > uint16_t alloc_size, start, cursor_blocks; > + uint16_t minimum[I915_MAX_PLANES]; > unsigned int total_data_rate; > int plane; > > @@ -2520,9 +2521,21 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, > alloc_size -= cursor_blocks; > alloc->end -= cursor_blocks; > > + /* 1. Allocate the mininum required blocks for each active plane */ > + for_each_plane(pipe, plane) { > + const struct intel_plane_wm_parameters *p; > + > + p = ¶ms->plane[plane]; > + if (!p->enabled) > + continue; > + > + minimum[plane] = 8; > + alloc_size -= minimum[plane]; > + } > + > /* > - * Each active plane get a portion of the remaining space, in > - * proportion to the amount of data they need to fetch from memory. > + * 2. Distribute the remaining space in proportion to the amount of > + * data each plane needs to fetch from memory. >* >* FIXME: we may not allocate every single block here. >*/ > @@ -2544,8 +2557,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, >* promote the expression to 64 bits to avoid overflowing, the >* result is < available as data_rate / total_data_rate < 1 >*/ > - plane_blocks = div_u64((uint64_t)alloc_size * data_rate, > -total_data_rate); > + plane_blocks = minimum[plane]; > + plane_blocks += div_u64((uint64_t)alloc_size * data_rate, > + total_data_rate); The minimum[] array seems pointless. The value here is always going to be 8. > > ddb->plane[pipe][plane].start = start; > ddb->plane[pipe][plane].end = start + plane_blocks; > -- > 1.8.3.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Make sure to allocate mininum sizes in the DDB
On Mon, Feb 09, 2015 at 03:56:20PM +0200, Ville Syrjälä wrote: > > @@ -2544,8 +2557,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc, > > * promote the expression to 64 bits to avoid overflowing, the > > * result is < available as data_rate / total_data_rate < 1 > > */ > > - plane_blocks = div_u64((uint64_t)alloc_size * data_rate, > > - total_data_rate); > > + plane_blocks = minimum[plane]; > > + plane_blocks += div_u64((uint64_t)alloc_size * data_rate, > > + total_data_rate); > > The minimum[] array seems pointless. The value here is always going to > be 8. Not in the future with new features appearing. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Correct the base value while updating LP_OUTPUT_HOLD in MIPI_PORT_CTRL
On Mon, 09 Feb 2015, Shobhit Kumar wrote: > On 02/06/2015 01:11 AM, shuang...@intel.com wrote: >> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: >> shuang...@intel.com) >> Task id: 5718 >> -Summary- >> Platform Delta drm-intel-nightly Series Applied >> PNV -8 282/283 274/283 >> ILK +1 316/319 317/319 >> SNB 322/346 322/346 >> IVB -1 382/384 381/384 >> BYT 296/296 296/296 >> HSW 425/428 425/428 >> BDW 318/333 318/333 >> -Detailed- >> Platform Testdrm-intel-nightly >> Series Applied >> *PNV igt_gem_fence_thrash_bo-write-verify-y PASS(2, M23M7) >> FAIL(1, M7) >> PNV igt_gem_userptr_blits_coherency-sync CRASH(2, M7)PASS(1, M23) >> CRASH(1, M7) >> PNV igt_gem_userptr_blits_coherency-unsync CRASH(2, M7)PASS(1, M23) >>CRASH(1, M7) >> *PNV igt_gem_userptr_blits_forked-sync-interruptible PASS(2, M23M7) >> DMESG_WARN(1, M7) >> *PNV igt_gem_userptr_blits_forked-sync-multifd-interruptible PASS(2, >> M23M7) NO_RESULT(1, M7) >> PNV igt_gen3_render_linear_blits FAIL(3, M7)CRASH(1, M23)PASS(4, >> M25M23) FAIL(1, M7) >> PNV igt_gen3_render_mixed_blits FAIL(3, M7)PASS(1, M23) FAIL(1, >> M7) >> PNV igt_gen3_render_tiledx_blits FAIL(2, M7)PASS(1, M23) FAIL(1, >> M7) >> PNV igt_gem_tiled_pread_pwrite FAIL(2, M7)PASS(1, M23) FAIL(1, >> M7) >> *ILK igt_drv_suspend_forcewake DMESG_WARN(5, M26M37) PASS(1, M26) >> IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance >> DMESG_WARN(2, M34)PASS(5, M21M34) DMESG_WARN(1, M21) >> Note: You need to pay more attention to line start with '*' > > Not sure why this is giving issues for PNV and ILK. Patch is strictly > for DSI. Are you expecting me to do anything about this ? Yes - please ignore them! ;) BR, Jani. > > Regards > Shobhit > >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
Hi Gurus: Forgive my junior HW knowledge, I just found that in execlist context initialization function populate_lr_context(), this line: reg_state[CTX_CONTEXT_CONTROL+1] = _MASKED_BIT_ENABLE((1<<3) | MI_RESTORE_INHIBIT); It seems the "Inhibit Synchronous Context Switch" bit is not set here, so when HW is trying to wait some events, e.g. semaphore, according to Bspec, per my basic understanding, it will switch out current context with some reason bit set. Here comes my question, I think if this situation happen, should i915 remember this context and try to re-schedule it in a proper time, e.g. before submitting a new context until the context_completed bit in CSB is set? I don't find a register that will remember the context switched out by waiting event, so I think it should be i915 to handle this situation or just set "Inhibit Synchronous Context Switch" bit here?... Thanks, Zhi. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 03/24] drm/i915: Setup less PPGTT on failed page_directory
Michel Thierry writes: > From: Ben Widawsky > > The current code will both potentially print a WARN, and setup part of > the PPGTT structure. Neither of these harm the current code, it is > simply for clarity, and to perhaps prevent later bugs, or weird > debug messages. > > Signed-off-by: Ben Widawsky > Signed-off-by: Michel Thierry > --- Reviewed-by: Mika Kuoppala > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 58d54bd..b48b586 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1032,11 +1032,14 @@ alloc: > goto alloc; > } > > + if (ret) > + return ret; > + > if (ppgtt->node.start < dev_priv->gtt.mappable_end) > DRM_DEBUG("Forced to use aperture for PDEs\n"); > > ppgtt->num_pd_entries = GEN6_PPGTT_PD_ENTRIES; > - return ret; > + return 0; > } > > static int gen6_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) > -- > 2.1.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 04/24] drm/i915/gen8: Un-hardcode number of page directories
Michel Thierry writes: > From: Ben Widawsky > > Signed-off-by: Ben Widawsky > Signed-off-by: Michel Thierry Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 9d998ec..8f76990 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -282,7 +282,7 @@ struct i915_hw_ppgtt { > }; > union { > dma_addr_t *pt_dma_addr; > - dma_addr_t *gen8_pt_dma_addr[4]; > + dma_addr_t *gen8_pt_dma_addr[GEN8_LEGACY_PDPES]; > }; > > struct drm_i915_file_private *file_priv; > -- > 2.1.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Insert a command barrier on BLT/BSD cache flushes
On Thu, 22 Jan 2015, Chris Wilson wrote: > This looked like an odd regression from > > commit ec5cc0f9b019af95e4571a9fa162d94294c8d90b > Author: Chris Wilson > Date: Thu Jun 12 10:28:55 2014 +0100 > > drm/i915: Restrict GPU boost to the RCS engine > > but in reality it undercovered a much older coherency bug. The issue that > boosting the GPU frequency on the BCS ring was masking was that we could > wake the CPU up after completion of a BCS batch and inspect memory prior > to the write cache being fully evicted. In order to serialise the > breadcrumb interrupt (and so ensure that the CPU's view of memory is > coherent) we need to perform a post-sync operation in the MI_FLUSH_DW. > > v2: Fix all the MI_FLUSH_DW (bsd plus the duplication in execlists). > > Testcase: gpuX-rcs-gpu-read-after-write > Signed-off-by: Chris Wilson > Cc: sta...@vger.kernel.org > Acked-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_lrc.c| 20 +++- > drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++ > 2 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index e405b61cdac5..8e71d8851c9a 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1237,15 +1237,17 @@ static int gen8_emit_flush(struct intel_ringbuffer > *ringbuf, > > cmd = MI_FLUSH_DW + 1; > > - if (ring == &dev_priv->ring[VCS]) { > - if (invalidate_domains & I915_GEM_GPU_DOMAINS) > - cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD | > - MI_FLUSH_DW_STORE_INDEX | > - MI_FLUSH_DW_OP_STOREDW; > - } else { > - if (invalidate_domains & I915_GEM_DOMAIN_RENDER) > - cmd |= MI_INVALIDATE_TLB | MI_FLUSH_DW_STORE_INDEX | > - MI_FLUSH_DW_OP_STOREDW; > + /* We always require a command barrier so that subsequent > + * commands, such as breadcrumb interrupts, are strictly ordered > + * wrt the contents of the write cache being flushed to memory > + * (and thus being coherent from the CPU). > + */ > + cmd |= MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; > + > + if (invalidate_domains & I915_GEM_GPU_DOMAINS) { Why do you change the mask from I915_GEM_DOMAIN_RENDER to I915_GEM_GPU_DOMAINS for ring != VCS? BR, Jani. > + cmd |= MI_INVALIDATE_TLB; > + if (ring == &dev_priv->ring[VCS]) > + cmd |= MI_INVALIDATE_BSD; > } > > intel_logical_ring_emit(ringbuf, cmd); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 23020d67329b..718530fd6c6b 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2224,6 +2224,14 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs > *ring, > cmd = MI_FLUSH_DW; > if (INTEL_INFO(ring->dev)->gen >= 8) > cmd += 1; > + > + /* We always require a command barrier so that subsequent > + * commands, such as breadcrumb interrupts, are strictly ordered > + * wrt the contents of the write cache being flushed to memory > + * (and thus being coherent from the CPU). > + */ > + cmd |= MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; > + > /* >* Bspec vol 1c.5 - video engine command streamer: >* "If ENABLED, all TLBs will be invalidated once the flush > @@ -2231,8 +2239,8 @@ static int gen6_bsd_ring_flush(struct intel_engine_cs > *ring, >* Post-Sync Operation field is a value of 1h or 3h." >*/ > if (invalidate & I915_GEM_GPU_DOMAINS) > - cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD | > - MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; > + cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD; > + > intel_ring_emit(ring, cmd); > intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT); > if (INTEL_INFO(ring->dev)->gen >= 8) { > @@ -2328,6 +2336,14 @@ static int gen6_ring_flush(struct intel_engine_cs > *ring, > cmd = MI_FLUSH_DW; > if (INTEL_INFO(ring->dev)->gen >= 8) > cmd += 1; > + > + /* We always require a command barrier so that subsequent > + * commands, such as breadcrumb interrupts, are strictly ordered > + * wrt the contents of the write cache being flushed to memory > + * (and thus being coherent from the CPU). > + */ > + cmd |= MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; > + > /* >* Bspec vol 1c.3 - blitter engine command streamer: >* "If ENABLED, all TLBs will be invalidated once the flush > @@ -2335,8 +2351,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring, >* Post-Sync Operation field is a value of 1h or 3h." >*/ > if (invalidate & I915_G
Re: [Intel-gfx] [PATCH v2] drm/i915: Insert a command barrier on BLT/BSD cache flushes
On Mon, Feb 09, 2015 at 06:00:25PM +0200, Jani Nikula wrote: > On Thu, 22 Jan 2015, Chris Wilson wrote: > > This looked like an odd regression from > > > > commit ec5cc0f9b019af95e4571a9fa162d94294c8d90b > > Author: Chris Wilson > > Date: Thu Jun 12 10:28:55 2014 +0100 > > > > drm/i915: Restrict GPU boost to the RCS engine > > > > but in reality it undercovered a much older coherency bug. The issue that > > boosting the GPU frequency on the BCS ring was masking was that we could > > wake the CPU up after completion of a BCS batch and inspect memory prior > > to the write cache being fully evicted. In order to serialise the > > breadcrumb interrupt (and so ensure that the CPU's view of memory is > > coherent) we need to perform a post-sync operation in the MI_FLUSH_DW. > > > > v2: Fix all the MI_FLUSH_DW (bsd plus the duplication in execlists). > > > > Testcase: gpuX-rcs-gpu-read-after-write > > Signed-off-by: Chris Wilson > > Cc: sta...@vger.kernel.org > > Acked-by: Daniel Vetter > > --- > > drivers/gpu/drm/i915/intel_lrc.c| 20 +++- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++ > > 2 files changed, 30 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index e405b61cdac5..8e71d8851c9a 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -1237,15 +1237,17 @@ static int gen8_emit_flush(struct intel_ringbuffer > > *ringbuf, > > > > cmd = MI_FLUSH_DW + 1; > > > > - if (ring == &dev_priv->ring[VCS]) { > > - if (invalidate_domains & I915_GEM_GPU_DOMAINS) > > - cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD | > > - MI_FLUSH_DW_STORE_INDEX | > > - MI_FLUSH_DW_OP_STOREDW; > > - } else { > > - if (invalidate_domains & I915_GEM_DOMAIN_RENDER) > > - cmd |= MI_INVALIDATE_TLB | MI_FLUSH_DW_STORE_INDEX | > > - MI_FLUSH_DW_OP_STOREDW; > > + /* We always require a command barrier so that subsequent > > +* commands, such as breadcrumb interrupts, are strictly ordered > > +* wrt the contents of the write cache being flushed to memory > > +* (and thus being coherent from the CPU). > > +*/ > > + cmd |= MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; > > + > > + if (invalidate_domains & I915_GEM_GPU_DOMAINS) { > > Why do you change the mask from I915_GEM_DOMAIN_RENDER to > I915_GEM_GPU_DOMAINS for ring != VCS? My bad, I didn't notice that execlists was originally broken. The patch is correct. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 04/24] drm/i915/gen8: Un-hardcode number of page directories
On Mon, Feb 09, 2015 at 05:30:45PM +0200, Mika Kuoppala wrote: > Michel Thierry writes: > > > From: Ben Widawsky > > > > Signed-off-by: Ben Widawsky > > Signed-off-by: Michel Thierry > > Reviewed-by: Mika Kuoppala Merged up to this one, thanks for patches&review. -Daniel > > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > > b/drivers/gpu/drm/i915/i915_gem_gtt.h > > index 9d998ec..8f76990 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > > @@ -282,7 +282,7 @@ struct i915_hw_ppgtt { > > }; > > union { > > dma_addr_t *pt_dma_addr; > > - dma_addr_t *gen8_pt_dma_addr[4]; > > + dma_addr_t *gen8_pt_dma_addr[GEN8_LEGACY_PDPES]; > > }; > > > > struct drm_i915_file_private *file_priv; > > -- > > 2.1.1 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Do not invalidate obj->pages under mempressure
From: Chris Wilson This (partially) reverts commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd Author: Chris Wilson Date: Tue Mar 25 13:23:06 2014 + drm/i915: Invalidate our pages under memory pressure It appears given the right workload, that pages which are swapped out more than once are incorrectly invalidated and discarded. I had presumed that the swapin would mark the pages dirty again and so preserve them against the next cycle of invalidation - that appears to be false, and leads to memory corruption (even leak of stale pages to userspace). v2: Do a more throughrought revert and als get rid of the hunk in gem_free_objects which we've tried to patch up already in commit 340fbd8ca1c7d6006a6b6afe716c10007bbfde85 Author: Chris Wilson Date: Thu May 22 09:16:52 2014 +0100 drm/i915: Only discard backing storage on releasing the last ref This means this patch also fully reverts this fixup. Apparently this is just too tricky. Reported-by: Sean V Kelley Signed-off-by: Chris Wilson Cc: Sean V Kelley Cc: sta...@vger.kernel.org Cc: Chris Wilson Cc: Jani Nikula Signed-off-by: Daniel Vetter (v2) --- drivers/gpu/drm/i915/i915_gem.c | 49 ++--- 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 36f1093e3c63..39e2af9b5fef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1946,26 +1946,6 @@ i915_gem_object_truncate(struct drm_i915_gem_object *obj) obj->madv = __I915_MADV_PURGED; } -/* Try to discard unwanted pages */ -static void -i915_gem_object_invalidate(struct drm_i915_gem_object *obj) -{ - struct address_space *mapping; - - switch (obj->madv) { - case I915_MADV_DONTNEED: - i915_gem_object_truncate(obj); - case __I915_MADV_PURGED: - return; - } - - if (obj->base.filp == NULL) - return; - - mapping = file_inode(obj->base.filp)->i_mapping, - invalidate_mapping_pages(mapping, 0, (loff_t)-1); -} - static void i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj) { @@ -2028,7 +2008,8 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj) ops->put_pages(obj); obj->pages = NULL; - i915_gem_object_invalidate(obj); + if (i915_gem_object_is_purgeable(obj)) + i915_gem_object_truncate(obj); return 0; } @@ -4458,30 +4439,6 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev, return obj; } -static bool discard_backing_storage(struct drm_i915_gem_object *obj) -{ - /* If we are the last user of the backing storage (be it shmemfs -* pages or stolen etc), we know that the pages are going to be -* immediately released. In this case, we can then skip copying -* back the contents from the GPU. -*/ - - if (obj->madv != I915_MADV_WILLNEED) - return false; - - if (obj->base.filp == NULL) - return true; - - /* At first glance, this looks racy, but then again so would be -* userspace racing mmap against close. However, the first external -* reference to the filp can only be obtained through the -* i915_gem_mmap_ioctl() which safeguards us against the user -* acquiring such a reference whilst we are in the middle of -* freeing the object. -*/ - return atomic_long_read(&obj->base.filp->f_count) == 1; -} - void i915_gem_free_object(struct drm_gem_object *gem_obj) { struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); @@ -4524,8 +4481,6 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (WARN_ON(obj->pages_pin_count)) obj->pages_pin_count = 0; - if (discard_backing_storage(obj)) - obj->madv = I915_MADV_DONTNEED; i915_gem_object_put_pages(obj); i915_gem_object_free_mmap_offset(obj); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not invalidate obj->pages under mempressure
On Sun, Feb 08, 2015 at 03:27:13PM -0800, Sean V Kelley wrote: > > > On 01/16/2015 08:05 PM, Daniel Vetter wrote: > > On Thu, Jan 15, 2015 at 08:44:00PM +, Chris Wilson wrote: > >> On Thu, Jan 15, 2015 at 08:36:15PM +0100, Daniel Vetter wrote: > >>> On Wed, Jan 14, 2015 at 9:34 PM, Chris Wilson > >>> wrote: > This (partially) reverts > > commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd Author: Chris > Wilson Date: Tue Mar 25 13:23:06 > 2014 + > > drm/i915: Invalidate our pages under memory pressure > >>> > >>> Shouldn't we also revert the hunk in i915_gem_free_objects? > >>> Without the truncate vs. invalidate disdinction it seems to > >>> have lost it's reason for existence ... > >> > >> No, setting MADV_DONTNEED has other nice properties during > >> put_pages() - I think it is useful in its own right, for example > >> that is where my page stealing code goes... > > > > Well right now I can't make sense of this bit any more (tbh I > > didn't with the other code either, but overlooked that while > > reviewing). When it's just there for future work but atm dead code > > I prefer for it to get removed. -Daniel > > > So can we also revert the hunk in i915_gem_free_objects? I would like > to get this patch merged, it looks like that is the primary concern. A problem I have is that the test written to hit the exact condition considered in the changelog does not ellict the bug. Can you test whether diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 39e032615b31..6269204ba16f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1030,6 +1030,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, /* update for the implicit flush after a batch */ obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; } + obj->dirty = 1; if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) { i915_gem_request_assign(&obj->last_fenced_req, req); if (entry->flags & __EXEC_OBJECT_HAS_FENCE) { makes the bug go away. If so, I think the bug is in the caller not setting reloc domains correctly. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use symbolic irqreturn for ->hpd_pulse
On Fri, Feb 06, 2015 at 08:49:12PM +0200, Ville Syrjälä wrote: > On Fri, Jan 23, 2015 at 06:00:31AM +0100, Daniel Vetter wrote: > > Self-explanatory code is better code. > > This causes the VDD off -> HPD -> VDD on -> VDD off -> HPD ... cycle to > make another appearance on my BSW. Looks like you forgot to convert one return > in intel_dp_hpd_pulse(): > > @@ -4499,7 +4499,7 @@ intel_dp_hpd_pulse(struct intel_digital_port > *intel_dig_port, bool long_hpd) > */ > DRM_DEBUG_KMS("ignoring long hpd on eDP port %c\n", > port_name(intel_dig_port->port)); > - return false; > + return IRQ_HANDLED; > } > > DRM_DEBUG_KMS("got hpd irq on port %c - %s\n", Oh dear that's rather bad. Can you please supply a full patch so that Jani can take it up for dinf? Free Reviewed-by: Daniel Vetter fwiw (which isn't much). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RESEND 0/9] Reviewed FBC patches
From: Paulo Zanoni As requested by Daniel, here is the series with the FBC patches that were already reviewed. Some of them were rebased after the review, but the changes are minor. Paulo Zanoni (9): drm/i915: don't try to find crtcs for FBC if it's disabled drm/i915: don't keep reassigning FBC_UNSUPPORTED drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc drm/i915: pass which operation triggered the frontbuffer tracking drm/i915: also do frontbuffer tracking on pwrites drm/i915: add frontbuffer tracking to FBC drm/i915: extract intel_fbc_find_crtc() drm/i915: HSW+ FBC is tied to pipe A drm/i915: gen5+ can have FBC with multiple pipes drivers/gpu/drm/i915/i915_drv.h| 18 +-- drivers/gpu/drm/i915/i915_gem.c| 14 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 5 +- drivers/gpu/drm/i915/intel_drv.h | 17 ++- drivers/gpu/drm/i915/intel_fbc.c | 196 ++--- drivers/gpu/drm/i915/intel_frontbuffer.c | 29 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c| 41 +- drivers/gpu/drm/i915/intel_ringbuffer.h| 1 - drivers/gpu/drm/i915/intel_sprite.c| 2 +- 10 files changed, 173 insertions(+), 152 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/9] drm/i915: don't try to find crtcs for FBC if it's disabled
From: Paulo Zanoni .. because it would be a waste of time, so move the place where the check is done. Also, with this we won't risk printing "more than one pipe active, disabling compression" or "no output, disabling" when FBC is actually disabled. This patch also represents a small behavior difference when using i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on this part of the code. V2: Rebase. Reviewed-by: Rodrigo Vivi (v1) Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index b572bb6e..c9a470f 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -507,10 +507,16 @@ void intel_fbc_update(struct drm_device *dev) return; } - if (!i915.powersave) { + if (i915.enable_fbc < 0) { + if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) + DRM_DEBUG_KMS("disabled per chip default\n"); + goto out_disable; + } + + if (!i915.enable_fbc || !i915.powersave) { if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) DRM_DEBUG_KMS("fbc disabled per module param\n"); - return; + goto out_disable; } /* @@ -545,16 +551,6 @@ void intel_fbc_update(struct drm_device *dev) obj = intel_fb_obj(fb); adjusted_mode = &intel_crtc->config->base.adjusted_mode; - if (i915.enable_fbc < 0) { - if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) - DRM_DEBUG_KMS("disabled per chip default\n"); - goto out_disable; - } - if (!i915.enable_fbc) { - if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) - DRM_DEBUG_KMS("fbc disabled per module param\n"); - goto out_disable; - } if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) || (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) { if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915: extract intel_fbc_find_crtc()
From: Paulo Zanoni I want to make this code a little more complicated, so let's extract the function first. Reviewed-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 46 +--- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 26c4f9c..7829c21 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -442,6 +442,32 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, return true; } +static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = dev_priv->dev; + struct drm_crtc *crtc = NULL, *tmp_crtc; + + for_each_crtc(dev, tmp_crtc) { + if (intel_crtc_active(tmp_crtc) && + to_intel_crtc(tmp_crtc)->primary_enabled) { + if (crtc) { + if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES)) + DRM_DEBUG_KMS("more than one pipe active, disabling compression\n"); + return NULL; + } + crtc = tmp_crtc; + } + } + + if (!crtc || crtc->primary->fb == NULL) { + if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT)) + DRM_DEBUG_KMS("no output, disabling\n"); + return NULL; + } + + return crtc; +} + /** * intel_fbc_update - enable/disable FBC as needed * @dev: the drm_device @@ -464,7 +490,7 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, void intel_fbc_update(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_crtc *crtc = NULL, *tmp_crtc; + struct drm_crtc *crtc = NULL; struct intel_crtc *intel_crtc; struct drm_framebuffer *fb; struct drm_i915_gem_object *obj; @@ -495,23 +521,9 @@ void intel_fbc_update(struct drm_device *dev) * - new fb is too large to fit in compressed buffer * - going to an unsupported config (interlace, pixel multiply, etc.) */ - for_each_crtc(dev, tmp_crtc) { - if (intel_crtc_active(tmp_crtc) && - to_intel_crtc(tmp_crtc)->primary_enabled) { - if (crtc) { - if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES)) - DRM_DEBUG_KMS("more than one pipe active, disabling compression\n"); - goto out_disable; - } - crtc = tmp_crtc; - } - } - - if (!crtc || crtc->primary->fb == NULL) { - if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT)) - DRM_DEBUG_KMS("no output, disabling\n"); + crtc = intel_fbc_find_crtc(dev_priv); + if (!crtc) goto out_disable; - } intel_crtc = to_intel_crtc(crtc); fb = crtc->primary->fb; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/9] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc
From: Paulo Zanoni Since the mapping from CRTCs to planes is fixed, looking at the CRTC is essentially the same as looking at the plane. Also, the next patches wil start using the frontbuffer_bits macros, and they take the pipe as the parameter instead of the plane, and this could differ on gens 2 and 3. Another nice thing is that we don't risk accidentally initializing things to PLANE_A if we don't set the value before it is used for the first time. But this shouldn't be a problem with the current code. V2: Rebase. Reviewed-by: Rodrigo Vivi (v1) Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 5 ++--- drivers/gpu/drm/i915/intel_fbc.c | 6 +++--- drivers/gpu/drm/i915/intel_sprite.c | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 26ffe8b..c0b8644 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -775,7 +775,7 @@ struct i915_fbc { unsigned long uncompressed_size; unsigned threshold; unsigned int fb_id; - enum plane plane; + struct intel_crtc *crtc; int y; struct drm_mm_node compressed_fb; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3fe9598..2655b63 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4277,11 +4277,10 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - int plane = intel_crtc->plane; intel_crtc_wait_for_pending_flips(crtc); - if (dev_priv->fbc.plane == plane) + if (dev_priv->fbc.crtc == intel_crtc) intel_fbc_disable(dev); hsw_disable_ips(intel_crtc); @@ -11932,7 +11931,7 @@ intel_check_primary_plane(struct drm_plane *plane, */ if (intel_crtc->primary_enabled && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && - dev_priv->fbc.plane == intel_crtc->plane && + dev_priv->fbc.crtc == intel_crtc && state->base.rotation != BIT(DRM_ROTATE_0)) { intel_crtc->atomic.disable_fbc = true; } diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index ce7774d..7341e87 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -369,7 +369,7 @@ static void intel_fbc_work_fn(struct work_struct *__work) if (work->crtc->primary->fb == work->fb) { dev_priv->display.enable_fbc(work->crtc); - dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane; + dev_priv->fbc.crtc = to_intel_crtc(work->crtc); dev_priv->fbc.fb_id = work->crtc->primary->fb->base.id; dev_priv->fbc.y = work->crtc->y; } @@ -460,7 +460,7 @@ void intel_fbc_disable(struct drm_device *dev) return; dev_priv->display.disable_fbc(dev); - dev_priv->fbc.plane = -1; + dev_priv->fbc.crtc = NULL; } static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, @@ -612,7 +612,7 @@ void intel_fbc_update(struct drm_device *dev) * cannot be unpinned (and have its GTT offset and fence revoked) * without first being decoupled from the scanout and FBC disabled. */ - if (dev_priv->fbc.plane == intel_crtc->plane && + if (dev_priv->fbc.crtc == intel_crtc && dev_priv->fbc.fb_id == fb->base.id && dev_priv->fbc.y == crtc->y) return; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0a52c44..5afc89e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -993,7 +993,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); mutex_lock(&dev->struct_mutex); - if (dev_priv->fbc.plane == intel_crtc->plane) + if (dev_priv->fbc.crtc == intel_crtc) intel_fbc_disable(dev); mutex_unlock(&dev->struct_mutex); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 9/9] drm/i915: gen5+ can have FBC with multiple pipes
From: Paulo Zanoni So allow it. Reviewed-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0d1c75d..6c84fb2 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -446,17 +446,19 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) { struct drm_crtc *crtc = NULL, *tmp_crtc; enum pipe pipe; - bool pipe_a_only = false; + bool pipe_a_only = false, one_pipe_only = false; if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8) pipe_a_only = true; + else if (INTEL_INFO(dev_priv)->gen <= 4) + one_pipe_only = true; for_each_pipe(dev_priv, pipe) { tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe]; if (intel_crtc_active(tmp_crtc) && to_intel_crtc(tmp_crtc)->primary_enabled) { - if (crtc) { + if (one_pipe_only && crtc) { if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES)) DRM_DEBUG_KMS("more than one pipe active, disabling compression\n"); return NULL; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/9] drm/i915: also do frontbuffer tracking on pwrites
From: Paulo Zanoni We need this for FBC, and possibly for PSR too. Reviewed-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_gem.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3d198f8..15910fa 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -,6 +,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, ret = i915_gem_phys_pwrite(obj, args, file); else ret = i915_gem_shmem_pwrite(dev, obj, args, file); + + intel_fb_obj_flush(obj, false, ORIGIN_CPU); + } else { + intel_fb_obj_flush(obj, false, ORIGIN_GTT); } out: -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: add frontbuffer tracking to FBC
From: Paulo Zanoni Kill the blt/render tracking we currently have and use the frontbuffer tracking infrastructure. Don't enable things by default yet. v2: (Rodrigo) Fix small conflict on rebase and typo at subject. v3: (Paulo) Rebase on RENDER_CS change. v4: (Paulo) Rebase. Reviewed-by: Rodrigo Vivi (v2) Signed-off-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 9 +-- drivers/gpu/drm/i915/intel_drv.h | 6 +- drivers/gpu/drm/i915/intel_fbc.c | 107 --- drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +--- drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +--- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 6 files changed, 80 insertions(+), 98 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d578211..dc002df 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -782,6 +782,7 @@ struct i915_fbc { unsigned long uncompressed_size; unsigned threshold; unsigned int fb_id; + unsigned int possible_framebuffer_bits; struct intel_crtc *crtc; int y; @@ -794,14 +795,6 @@ struct i915_fbc { * possible. */ bool enabled; - /* On gen8 some rings cannont perform fbc clean operation so for now -* we are doing this on SW with mmio. -* This variable works in the opposite information direction -* of ring->fbc_dirty telling software on frontbuffer tracking -* to perform the cache clean on sw side. -*/ - bool need_sw_cache_clean; - struct intel_fbc_work { struct delayed_work work; struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a73d091..ed85df8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1101,7 +1101,11 @@ bool intel_fbc_enabled(struct drm_device *dev); void intel_fbc_update(struct drm_device *dev); void intel_fbc_init(struct drm_i915_private *dev_priv); void intel_fbc_disable(struct drm_device *dev); -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value); +void intel_fbc_invalidate(struct drm_i915_private *dev_priv, + unsigned int frontbuffer_bits, + enum fb_op_origin origin); +void intel_fbc_flush(struct drm_i915_private *dev_priv, +unsigned int frontbuffer_bits, enum fb_op_origin origin); /* intel_hdmi.c */ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 7341e87..26c4f9c 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -174,29 +174,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev) return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN; } -static void snb_fbc_blit_update(struct drm_device *dev) +static void intel_fbc_nuke(struct drm_i915_private *dev_priv) { - struct drm_i915_private *dev_priv = dev->dev_private; - u32 blt_ecoskpd; - - /* Make sure blitter notifies FBC of writes */ - - /* Blitter is part of Media powerwell on VLV. No impact of -* his param in other platforms for now */ - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA); - - blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD); - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY << - GEN6_BLITTER_LOCK_SHIFT; - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY; - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY << -GEN6_BLITTER_LOCK_SHIFT); - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - POSTING_READ(GEN6_BLITTER_ECOSKPD); - - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA); + I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE); + POSTING_READ(MSG_FBC_REND_STATE); } static void ilk_fbc_enable(struct drm_crtc *crtc) @@ -239,9 +220,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc) I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj->fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); - snb_fbc_blit_update(dev); } + intel_fbc_nuke(dev_priv); + DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane)); } @@ -320,7 +302,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc) SNB_CPU_FENCE_ENABLE | obj->fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); - snb_fbc_blit_update(dev); + intel_fbc_nuke(dev_priv); DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane)); } @@ -340,19 +322,6 @@ bool intel_fbc_enabled(struct drm_device *dev) return dev_priv->fbc.enabled;
[Intel-gfx] [PATCH 4/9] drm/i915: pass which operation triggered the frontbuffer tracking
From: Paulo Zanoni We want to port FBC to the frontbuffer tracking infrastructure, but for that we need to know what caused the object invalidation/flush so we can react accordingly: CPU mmaps need manual, GTT mmaps and flips don't need handling and ring rendering needs nukes. v2: - s/ORIGIN_RENDER/ORIGIN_CS/ (Daniel, Rodrigo) - Fix copy/pasted wrong documentation - Rebase v3: - Rebase Reviewed-by: Rodrigo Vivi (v2) Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h| 7 +++ drivers/gpu/drm/i915/i915_gem.c| 10 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 11 +++ drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c0b8644..d578211 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -771,6 +771,13 @@ struct intel_context { struct list_head link; }; +enum fb_op_origin { + ORIGIN_GTT, + ORIGIN_CPU, + ORIGIN_CS, + ORIGIN_FLIP, +}; + struct i915_fbc { unsigned long uncompressed_size; unsigned threshold; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c26d36c..3d198f8 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2310,7 +2310,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) list_move_tail(&vma->mm_list, &vma->vm->inactive_list); } - intel_fb_obj_flush(obj, true); + intel_fb_obj_flush(obj, true, ORIGIN_CS); list_del_init(&obj->ring_list); @@ -3677,7 +3677,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj) old_write_domain = obj->base.write_domain; obj->base.write_domain = 0; - intel_fb_obj_flush(obj, false); + intel_fb_obj_flush(obj, false, ORIGIN_GTT); trace_i915_gem_object_change_domain(obj, obj->base.read_domains, @@ -3699,7 +3699,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj) old_write_domain = obj->base.write_domain; obj->base.write_domain = 0; - intel_fb_obj_flush(obj, false); + intel_fb_obj_flush(obj, false, ORIGIN_CPU); trace_i915_gem_object_change_domain(obj, obj->base.read_domains, @@ -3764,7 +3764,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write) } if (write) - intel_fb_obj_invalidate(obj, NULL); + intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT); trace_i915_gem_object_change_domain(obj, old_read_domains, @@ -4079,7 +4079,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write) } if (write) - intel_fb_obj_invalidate(obj, NULL); + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU); trace_i915_gem_object_change_domain(obj, old_read_domains, diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index b773368..f4342f0 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -971,7 +971,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, obj->dirty = 1; i915_gem_request_assign(&obj->last_write_req, req); - intel_fb_obj_invalidate(obj, ring); + intel_fb_obj_invalidate(obj, ring, ORIGIN_CS); /* update for the implicit flush after a batch */ obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 76b3c20..a73d091 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -853,13 +853,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state); /* intel_frontbuffer.c */ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj, -struct intel_engine_cs *ring); +struct intel_engine_cs *ring, +enum fb_op_origin origin); void intel_frontbuffer_flip_prepare(struct drm_device *dev, unsigned frontbuffer_bits); void intel_frontbuffer_flip_complete(struct drm_device *dev, unsigned frontbuffer_bits); void intel_frontbuffer_flush(struct drm_device *dev, -unsigned frontbuffer_bits); +unsigned frontbuffer_bits, +enum
[Intel-gfx] [PATCH 8/9] drm/i915: HSW+ FBC is tied to pipe A
From: Paulo Zanoni So add code to consider this case. Reviewed-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 7829c21..0d1c75d 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -444,10 +444,16 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) { - struct drm_device *dev = dev_priv->dev; struct drm_crtc *crtc = NULL, *tmp_crtc; + enum pipe pipe; + bool pipe_a_only = false; + + if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8) + pipe_a_only = true; + + for_each_pipe(dev_priv, pipe) { + tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe]; - for_each_crtc(dev, tmp_crtc) { if (intel_crtc_active(tmp_crtc) && to_intel_crtc(tmp_crtc)->primary_enabled) { if (crtc) { @@ -457,6 +463,9 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) } crtc = tmp_crtc; } + + if (pipe_a_only) + break; } if (!crtc || crtc->primary->fb == NULL) { @@ -712,11 +721,14 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) return; } - /* TODO: some platforms have FBC tied to a specific plane! */ - for_each_pipe(dev_priv, pipe) + for_each_pipe(dev_priv, pipe) { dev_priv->fbc.possible_framebuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(pipe); + if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8) + break; + } + if (INTEL_INFO(dev_priv)->gen >= 7) { dev_priv->display.fbc_enabled = ilk_fbc_enabled; dev_priv->display.enable_fbc = gen7_fbc_enable; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915: don't keep reassigning FBC_UNSUPPORTED
From: Paulo Zanoni This may save a few picoseconds on !HAS_FBC platforms. And it also satisfies my OCD. Reviewed-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index c9a470f..ce7774d 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -502,10 +502,8 @@ void intel_fbc_update(struct drm_device *dev) const struct drm_display_mode *adjusted_mode; unsigned int max_width, max_height; - if (!HAS_FBC(dev)) { - set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED); + if (!HAS_FBC(dev)) return; - } if (i915.enable_fbc < 0) { if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) @@ -670,6 +668,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) { if (!HAS_FBC(dev_priv)) { dev_priv->fbc.enabled = false; + dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED; return; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Add tiled framebuffer modifiers
On Thu, Feb 05, 2015 at 02:41:53PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > To be used from the new addfb2 extension. > > Signed-off-by: Tvrtko Ursulin > --- > include/uapi/drm/i915_drm.h | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 6eed16b..e4c09e2 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -28,6 +28,7 @@ > #define _UAPI_I915_DRM_H_ > > #include > +#include > > /* Please note that modifications to all structs defined here are > * subject to backwards-compatibility constraints. > @@ -1101,4 +1102,18 @@ struct drm_i915_gem_context_param { > __u64 value; > }; > > +/** @{ > + * Intel framebuffer modifiers > + * > + * Tiling modes supported by the display hardware > + * to be passed in via the DRM addfb2 ioctl. > + */ > +/** Bits reserved for tiling */ > +#define I915_FORMAT_MOD_TILING_MASK fourcc_mod_code(INTEL, 0xff) > +/** None */ > +#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L) > +/** X tiling */ > +#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL, > 0x01L) > +/** @} */ These must be in drm_fourcc.h, adn ** @{ isn't how kerneldoc works. I've fixed this up plus add a bit of wording to make it clearer what these are. Btw for the new tiling modes I think we should fully spec out the layout, as Rob suggested. -Daniel > + > #endif /* _UAPI_I915_DRM_H_ */ > -- > 2.2.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop vblank wait from intel_dp_link_down
On Mon, 09 Feb 2015, Jani Nikula wrote: > On Mon, 09 Feb 2015, Ville Syrjälä wrote: >> On Mon, Feb 09, 2015 at 03:15:56PM +0200, Jani Nikula wrote: >>> On Wed, 26 Nov 2014, Paulo Zanoni wrote: >>> > 2014-11-24 13:54 GMT-02:00 Daniel Vetter : >>> >> Nothing in Bspec seems to indicate that we actually needs this, and it >>> >> looks like can't work since by this point the pipe is off and so >>> >> vblanks won't really happen any more. >>> >> >>> >> Note that Bspec mentions that it takes a vblank for this bit to >>> >> change, but _only_ when enabling. >>> >> >>> >> Dropping this code quenches an annoying backtrace introduced by the >>> >> more anal checking since >>> >> >>> >> commit 51e31d49c89055299e34b8f44d13f70e19d1 >>> >> Author: Daniel Vetter >>> >> Date: Mon Sep 15 12:36:02 2014 +0200 >>> >> >>> >> drm/i915: Use generic vblank wait >>> >> >>> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86095 >>> >> Signed-off-by: Daniel Vetter >>> >> --- >>> >> drivers/gpu/drm/i915/intel_dp.c | 17 + >>> >> 1 file changed, 1 insertion(+), 16 deletions(-) >>> >> >>> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c >>> >> b/drivers/gpu/drm/i915/intel_dp.c >>> >> index 46731da407c0..63fcdbf90652 100644 >>> >> --- a/drivers/gpu/drm/i915/intel_dp.c >>> >> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> >> @@ -3514,8 +3514,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) >>> >> enum port port = intel_dig_port->port; >>> >> struct drm_device *dev = intel_dig_port->base.base.dev; >>> >> struct drm_i915_private *dev_priv = dev->dev_private; >>> >> - struct intel_crtc *intel_crtc = >>> >> - to_intel_crtc(intel_dig_port->base.base.crtc); >>> >> uint32_t DP = intel_dp->DP; >>> >> >>> >> if (WARN_ON(HAS_DDI(dev))) >>> >> @@ -3540,8 +3538,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) >>> >> >>> >> if (HAS_PCH_IBX(dev) && >>> >> I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { >>> >> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc; >>> >> - >>> >> /* Hardware workaround: leaving our transcoder select >>> >> * set to transcoder B while it's off will prevent the >>> >> * corresponding HDMI output on transcoder A. >>> >> @@ -3552,18 +3548,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) >>> >> */ >>> >> DP &= ~DP_PIPEB_SELECT; >>> >> I915_WRITE(intel_dp->output_reg, DP); >>> >> - >>> >> - /* Changes to enable or select take place the vblank >>> >> -* after being written. >>> >> -*/ >>> >> - if (WARN_ON(crtc == NULL)) { >>> >> - /* We should never try to disable a port without >>> >> a crtc >>> >> -* attached. For paranoia keep the code around >>> >> for a >>> >> -* bit. */ >>> >> - POSTING_READ(intel_dp->output_reg); >>> >> - msleep(50); >>> >> - } else >>> >> - intel_wait_for_vblank(dev, intel_crtc->pipe); >>> > >>> > What I can guess is that we have the vblank wait here because the >>> > DP_PORT_EN bit is still enabled at this point. It would make some >>> > sense to have it if the pipe were not off... So removing the waits >>> > looks sane: Reviewed-by: Paulo Zanoni >>> > >>> > But when I read the spec, it makes me think that maybe doing the >>> > I915_WRITE above is also wrong, since the port is still enabled. Maybe >>> > we should only clear bit 30 in the same write as the one that clears >>> > bit 31? >>> >>> Ugh. So the spec says, "When disabling the port, software must >>> temporarily enable the port with transcoder select (bit #30) cleared to >>> ‘0’ after disabling the port." >>> >>> IIUC we should disable like we normally do, and do the w/a by enabling >>> and disabling the port with DP_PIPEB_SELECT cleared *after* that. I >>> think the current code is wrong, the patch is wrong, what Paulo suggests >>> is wrong, and also intel_disable_hdmi() is wrong. >> >> This code has been bugging me for a long time as well. IIRC I even had >> cooked up some patches to do the re-enable as you suggest since I >> read the spec the same way. But I never had enough time to test it. And >> in order to really test it I would first like to actually reproduce the >> problem that the workaround is supposed to fix. How else would you know >> if the workaround is correct after all. > > *sigh* an alternative is to apply Daniel's patch and add a comment > there's something fishy. I've done just that, with cc: stable for v3.19. I referenced this discussion from the commit message. Thanks for the patch, review, and bikeshedding. BR, Jani. > > Jani. > >> >> -- >> Ville Syrjälä >> Intel OTC > > -- > Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Techn
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Add tiled framebuffer modifiers
On Thu, Feb 05, 2015 at 02:41:53PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > To be used from the new addfb2 extension. > > Signed-off-by: Tvrtko Ursulin > --- > include/uapi/drm/i915_drm.h | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 6eed16b..e4c09e2 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -28,6 +28,7 @@ > #define _UAPI_I915_DRM_H_ > > #include > +#include > > /* Please note that modifications to all structs defined here are > * subject to backwards-compatibility constraints. > @@ -1101,4 +1102,18 @@ struct drm_i915_gem_context_param { > __u64 value; > }; > > +/** @{ > + * Intel framebuffer modifiers > + * > + * Tiling modes supported by the display hardware > + * to be passed in via the DRM addfb2 ioctl. > + */ > +/** Bits reserved for tiling */ > +#define I915_FORMAT_MOD_TILING_MASK fourcc_mod_code(INTEL, 0xff) > +/** None */ > +#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L) And we don't need an intel specific version for untiled either. -Daniel > +/** X tiling */ > +#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL, > 0x01L) > +/** @} */ > + > #endif /* _UAPI_I915_DRM_H_ */ > -- > 2.2.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use symbolic irqreturn for ->hpd_pulse
On Mon, 09 Feb 2015, Daniel Vetter wrote: > On Fri, Feb 06, 2015 at 08:49:12PM +0200, Ville Syrjälä wrote: >> On Fri, Jan 23, 2015 at 06:00:31AM +0100, Daniel Vetter wrote: >> > Self-explanatory code is better code. >> >> This causes the VDD off -> HPD -> VDD on -> VDD off -> HPD ... cycle to >> make another appearance on my BSW. Looks like you forgot to convert one >> return >> in intel_dp_hpd_pulse(): >> >> @@ -4499,7 +4499,7 @@ intel_dp_hpd_pulse(struct intel_digital_port >> *intel_dig_port, bool long_hpd) >> */ >> DRM_DEBUG_KMS("ignoring long hpd on eDP port %c\n", >> port_name(intel_dig_port->port)); >> - return false; >> + return IRQ_HANDLED; >> } >> >> DRM_DEBUG_KMS("got hpd irq on port %c - %s\n", > > Oh dear that's rather bad. Can you please supply a full patch so that Jani > can take it up for dinf? Free Reviewed-by: Daniel Vetter > > fwiw (which isn't much). *facepalm* from the "reviewer" here too. Ville, please change that early return to a "goto out" maybe? BR, Jani. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Kernel 3.19rc6 flooding intel_check_page_flip warnings when using compton
On 05/02/15 11:01, Chris Wilson wrote: > On Thu, Feb 05, 2015 at 12:44:21PM +0200, Sakari Kapanen wrote: >> On 02/04/2015 11:26 AM, Jani Nikula wrote: >>> On Mon, 02 Feb 2015, Sakari Kapanen wrote: Dear maintainers, On an Asus Zenbook UX32VD laptop with an i5-3317U CPU and Intel HD4000 graphics, I'm experiencing the following with the latest 3.19rc6 mainline kernel (built from the Arch Linux AUR package: https://aur.archlinux.org/packages/linux-mainline/ ). The problem is related to the compton compositor: https://github.com/chjj/compton >>> Hi, a full dmesg from boot to the problem with drm.debug=14 module >>> parameter set might be useful. Also, you may get fastest results if you >>> do a git bisect from a good to bad kernel. >>> >>> BR, >>> Jani. >> >> Hi, I did a bisect between 3.18 to 3.19-rc1. I could only narrow it >> down to ~110 >> commits. They were based on 3.17-rc5 which wouldn't boot on my computer >> due to an unrelated kernel panic which I couldn't resolve, so I >> couldn't bisect any >> further. Sorry about that! >> >> I noticed one thing: the warnings I mentioned appear only when threader IRQs >> are enabled via the `threadirqs` kernel flag. Without that flag, I >> didn't get any >> of those warnings on any kernel. >> >> I attached the bisect log, in which the commits that were left are >> shown. Also, >> there's a dmesg log with drm.debug=14 set. The first warning appears at >> 4.895940 when X is started (no compton yet). Compton was started at ~14, >> and the first warning due to it appears at 15.009088. >> >> Please let me know if I any other information would be useful. > > The commit you were looking for is: > > commit f326038a29092534b59626f736a3c6e599bda017 > Author: Daniel Vetter > Date: Mon Sep 15 14:55:23 2014 +0200 > > drm/i915: Clarify event_lock locking, irq&mixed context > > Now we tackle the functions also called from interrupt handlers. > > - intel_check_page_flip is exclusively called from irq handlers, so a > plain spin_lock is all we need. In i915_irq.c we have the convention > to give all such functions an _irq_handler postfix, but that would > look strange and als be a bit a misleading name. I've opted for a > WARN_ON(!in_irq()) instead. > > - The other two places left are called both from interrupt handlers > and from our reset work, so need the full irqsave dance. Annotate > them with a short comment. > > Reviewed-by: Jesse Barnes > Signed-off-by: Daniel Vetter > > The WARN_ON(!in_irq() fires > >> [4.889038] [drm:intel_crtc_set_config] [CRTC:8] [FB:33] #connectors=1 (x >> y) (0 0) >> [4.889045] [drm:intel_set_config_compute_mode_changes] computed changes >> for [CRTC:8], mode_changed=0, fb_changed=1 >> [4.889050] [drm:intel_modeset_stage_output_state] [CONNECTOR:21:eDP-1] >> to [CRTC:8] >> [4.891027] [drm:ironlake_update_primary_plane] Writing base 0076C000 >> 0 0 5632 >> [4.895940] [ cut here ] >> [4.895980] WARNING: CPU: 1 PID: 248 at >> drivers/gpu/drm/i915/intel_display.c:9754 intel_check_page_flip+0x99/0xe0 >> [i915]() >> [4.895983] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek >> snd_hda_codec_generic nls_iso8859_1 nls_cp437 vfat fat joydev mousedev msr >> coretemp rtsx_usb_ms intel_rapl memstick rtsx_usb_sdmmc uvcvideo >> x86_pkg_temp_thermal mmc_core videobuf2_vmalloc intel_powerclamp >> videobuf2_memops videobuf2_core kvm_intel v4l2_common kvm rtsx_usb videodev >> iTCO_wdt media iTCO_vendor_support arc4 crct10dif_pclmul crc32_pclmul >> crc32c_intel ghash_clmulni_intel aesni_intel aes_x86_64 glue_helper lrw >> gf128mul ablk_helper cryptd nouveau >> [4.896007] [drm:drm_mode_setcrtc] [CRTC:12] >> [4.896009] [drm:intel_crtc_set_config] [CRTC:12] [NOFB] >> [4.896011] snd_hda_intel iwldvm >> [4.896013] [drm:intel_set_config_compute_mode_changes] computed changes >> for [CRTC:12], mode_changed=0, fb_changed=0 >> [4.896014] [drm:intel_modeset_stage_output_state] [CONNECTOR:21:eDP-1] >> to [CRTC:8] >> [4.896016] snd_hda_controller >> [4.896017] evdev mac80211 >> [4.896018] [drm:drm_mode_setcrtc] [CRTC:16] >> [4.896018] psmouse >> [4.896019] serio_raw mac_hid pcspkr snd_hda_codec i2c_i801 >> [4.896023] [drm:intel_crtc_set_config] [CRTC:16] [NOFB] >> [4.896025] [drm:intel_set_config_compute_mode_changes] computed changes >> for [CRTC:16], mode_changed=0, fb_changed=0 >> [4.896026] [drm:intel_modeset_stage_output_state] [CONNECTOR:21:eDP-1] >> to [CRTC:8] >> [4.896027] mxm_wmi i915 ttm snd_hwdep iwlwifi lpc_ich snd_pcm mfd_core >> snd_timer snd cfg80211 soundcore mei_me mei thermal int3403_thermal fan >> button i2c_algo_bit drm_kms_helper battery int3400_thermal drm >> acpi_thermal_rel int3402_thermal i2c_core ac tpm_tis intel_gtt tpm processor >> shpchp sch_fq_codel ext4 crc16 mbcache jbd2
[Intel-gfx] [PATCH 3/5] drm/i915: Use fb format modifiers in skylake_update_primary_plane
Just a little demo really. We probably need to introduce skl specific functions for a lot of the format validation stuff, or at least helpers. Specifically I think intel_framebuffer_init and intel_fb_align_height must be adjusted to have an i915_ and a skl_ variant. And only shared code should be converted to fb modifiers, platform code (like the plane config readout can keep on using old tiling_mode defines to avoid some churn). Cc: Tvrtko Ursulin Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2d69cce03ab5..41b3ddc4068d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2773,11 +2773,11 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, * The stride is either expressed as a multiple of 64 bytes chunks for * linear buffers or in number of tiles for tiled buffers. */ - switch (obj->tiling_mode) { - case I915_TILING_NONE: + switch (fb->modifier[0]) { + case DRM_FORMAT_MOD_NONE: stride = fb->pitches[0] >> 6; break; - case I915_TILING_X: + case I915_FORMAT_MOD_X_TILED: plane_ctl |= PLANE_CTL_TILED_X; stride = fb->pitches[0] >> 9; break; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] i915 fb modifier support, respun
Hi all, So this is the very quickly spun together version for my take on fb modifiers. Aim is to reduce the churn a bit with: - Only validate fb modifiers and old tiling_mode behaviour in framebuffer_init to ensure they are consistent. Don't convert over all the code for old platforms. - Guarantee that X-tiling always implies that the underlying bo is fenced with X-tiling mode, too. Otherwise the implications into existing code (e.g. adjusting fbc) are a bit too much. - One draft patch to show how I think we should us fb modifiers: Directly switch on them like we do with obj->tiling_mode, no need to have remap/masking functions around. At least for now. This isn't complete since some of the shared code, specifically the fb_align stuff used by framebuffer_init and the fbdev emulation code still uses obj->tiling_mode. That needs to be converted into a function which takes u64 fb modifers (for shared code) and the old code retained in an i9xx_ variant (for the plane_config readout code for old platforms). Then we can add the skl+ stuff in another version, together with a if/else. And then framebuffer_init obvsiouly needs to be extended. Commments highly welcome. Cheers, Daniel Daniel Vetter (3): drm/i915: Add fb format modifier support drm/i915: Use fb format modifiers in skylake_update_primary_plane drm: Also check unused fields for addfb2 Tvrtko Ursulin (2): drm/i915: Add tiled framebuffer modifiers drm/i915: Announce support for framebuffer modifiers drivers/gpu/drm/drm_crtc.c | 17 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 32 include/uapi/drm/drm_fourcc.h| 31 +++ 4 files changed, 73 insertions(+), 8 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915: Announce support for framebuffer modifiers
From: Tvrtko Ursulin Let the DRM core know we can handle it. v2: Change to boolean true. (Daniel Vetter) Signed-off-by: Tvrtko Ursulin Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 41b3ddc4068d..e96d0c75f89c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13214,6 +13214,8 @@ void intel_modeset_init(struct drm_device *dev) dev->mode_config.preferred_depth = 24; dev->mode_config.prefer_shadow = 1; + dev->mode_config.allow_fb_modifiers = true; + dev->mode_config.funcs = &intel_mode_funcs; intel_init_quirks(dev); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Add fb format modifier support
Currently we don't support anything but X tiled. And for an easier transition it makes a lot of sense to just keep requiring that X tiled is properly fenced. Which means we need to do absolutely nothing in old code to support fb modifiers, yay! Cc: Tvrtko Ursulin Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 24 +++- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3fe95982be93..2d69cce03ab5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12707,7 +12707,20 @@ static int intel_framebuffer_init(struct drm_device *dev, WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - if (obj->tiling_mode == I915_TILING_Y) { + if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) { + /* Enforce that fb modifier and tiling mode match, but only for +* X-tiled. */ + if (!!(obj->tiling_mode == I915_TILING_X) != + !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) { + DRM_DEBUG("tiling_mode doesn't match fb modifier\n"); + return -EINVAL; + } + } else { + if (obj->tiling_mode == I915_TILING_X) + mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; + } + + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED) { DRM_DEBUG("hardware does not support tiling Y\n"); return -EINVAL; } @@ -12721,12 +12734,12 @@ static int intel_framebuffer_init(struct drm_device *dev, if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) { pitch_limit = 32*1024; } else if (INTEL_INFO(dev)->gen >= 4) { - if (obj->tiling_mode) + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED) pitch_limit = 16*1024; else pitch_limit = 32*1024; } else if (INTEL_INFO(dev)->gen >= 3) { - if (obj->tiling_mode) + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED) pitch_limit = 8*1024; else pitch_limit = 16*1024; @@ -12736,12 +12749,13 @@ static int intel_framebuffer_init(struct drm_device *dev, if (mode_cmd->pitches[0] > pitch_limit) { DRM_DEBUG("%s pitch (%d) must be at less than %d\n", - obj->tiling_mode ? "tiled" : "linear", + mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED ? + "tiled" : "linear", mode_cmd->pitches[0], pitch_limit); return -EINVAL; } - if (obj->tiling_mode != I915_TILING_NONE && + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED && mode_cmd->pitches[0] != obj->stride) { DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n", mode_cmd->pitches[0], obj->stride); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Add tiled framebuffer modifiers
From: Tvrtko Ursulin To be used from the new addfb2 extension. v2: - Drop Intel-specific untiled modfier. - Move to drm_fourcc.h. - Document layouts a bit and denote them as platform-specific and not useable for cross-driver sharing. - Add Y-tiling for completeness. - Drop special docstring markers to avoid confusing kerneldoc. Signed-off-by: Tvrtko Ursulin Cc: Tvrtko Ursulin Signed-off-by: Daniel Vetter (v2) --- drivers/gpu/drm/i915/i915_drv.h | 1 + include/uapi/drm/drm_fourcc.h | 31 +++ 2 files changed, 32 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 217845951b7f..a027a983c82b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -31,6 +31,7 @@ #define _I915_DRV_H_ #include +#include #include "i915_reg.h" #include "intel_bios.h" diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 622109677747..886814c6f9d2 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -164,4 +164,35 @@ * authoritative source for all of these. */ +/* Intel framebuffer modifiers */ + +/* + * Intel X-tiling layout + * + * This is a tiled layout using 4Kb tiles (except on gen2 where the tiles 2Kb) + * in row-major layout. Within the tile bytes are laid out row-major, with + * a platform-dependent stride. On top of that the memory can apply + * platform-depending swizzling of some higher address bits into bit6. + * + * This format is highly platforms specific and not useful for cross-driver + * sharing. It exists since on a given platform it does uniquely identify the + * layout in a simple way for i915-specific userspace. + */ +#define I915_FORMAT_MOD_X_TILEDfourcc_mod_code(INTEL, 1) + +/* + * Intel Y-tiling layout + * + * This is a tiled layout using 4Kb tiles (except on gen2 where the tiles 2Kb) + * in row-major layout. Within the tile bytes are laid out in OWORD (16 bytes) + * chunks column-major, with a platform-dependent height. On top of that the + * memory can apply platform-depending swizzling of some higher address bits + * into bit6. + * + * This format is highly platforms specific and not useful for cross-driver + * sharing. It exists since on a given platform it does uniquely identify the + * layout in a simple way for i915-specific userspace. + */ +#define I915_FORMAT_MOD_Y_TILEDfourcc_mod_code(INTEL, 1) + #endif /* DRM_FOURCC_H */ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm: Also check unused fields for addfb2
Just the usual paranoia ... Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index b15d720eda4c..a12d7e8a0ca0 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3322,6 +3322,23 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } } + for (; i < 4; i++) { + if (r->handles[i]) { + DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i); + return -EINVAL; + } + + if (r->pitches[i] || r->offsets[i]) { + DRM_DEBUG_KMS("buffer pitch/offset for unused plane", i); + return -EINVAL; + } + + if (r->modifier[i]) { + DRM_DEBUG_KMS("fb modifer for unused plane", i); + return -EINVAL; + } + } + return 0; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/10] drm/i915: gen 9 h/w w/a (WaDisablePartialInstShootdown)
On Fri, Feb 06, 2015 at 09:36:08AM +, Nick Hoath wrote: > On 06/02/2015 08:52, Daniel Vetter wrote: > >On Thu, Feb 05, 2015 at 05:51:46PM +, Damien Lespiau wrote: > >>On Thu, Feb 05, 2015 at 10:47:18AM +, Nick Hoath wrote: > >>>From: "Hoath, Nicholas" > >>> > >>>Add: > >>>WaDisablePartialInstShootdown > >> > >>Just an editor note: that's not really additional information compared > >>to the subject of the patch. Also subject message could be a bit more > >>direct and mention SKL: > >> > >> drm/i915/skl: Implement WaDisablePartialInstShootdown > > > >Well it's gen9 but yeah. The commit message body should explain the > >commit (e.g. more details on impact), but for w/a that's only really > >required if there's been an outside report. > So an empty body is ok if the subject has sufficient information? Yeah for hw wa there's often not more to say. Except when some details about the impact are known which are relevant (e.g. existing bug report or not wa for a feature not yet used in upstream). Generally an empty commit message is a bit thin, but just repeating the summar doesn't add value. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: gen 9 h/w w/a Fix stepping check
On Fri, Feb 06, 2015 at 11:52:42AM +, Damien Lespiau wrote: > On Fri, Feb 06, 2015 at 11:30:03AM +, Nick Hoath wrote: > > Fixed the stepping check on WaDisableDgMirrorFixInHalfSliceChicken5 > > to be for the correct SOC (Skylake) > > > > Signed-off-by: Nick Hoath > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 573b80f..fb71e33 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -886,7 +886,8 @@ static int gen9_init_workarounds(struct intel_engine_cs > > *ring) > > WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, > > GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); > > > > - if (INTEL_REVID(dev) == SKL_REVID_A0) { > > + if (INTEL_REVID(dev) >= SKL_REVID_A0 && > > + INTEL_REVID(dev) <= SKL_REVID_B0) { > > x >= 0 && x <= 1 looks really better as x == 0 || x == 1 for pre-production w/a I don't really care all that much how they look, we'll remove them anyway in a few months or so. For production stuff I prefer we stick to how we do gen checks: - gen >= 5 (read as gen5+) - gen < 6 (read as pre-gen5) Consistency to avoid accidentaly off-by-one and it seems to match what Bspec uses, too. > > Otherwise: > > Reviewed-by: Damien Lespiau Both merged, thanks for patches&review. -Daniel > > > /* > > * WaDisableDgMirrorFixInHalfSliceChicken5:skl > > * This is a pre-production w/a. > > -- > > 2.1.1 > > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Insert a command barrier on BLT/BSD cache flushes
On Mon, 09 Feb 2015, Chris Wilson wrote: > On Mon, Feb 09, 2015 at 06:00:25PM +0200, Jani Nikula wrote: >> On Thu, 22 Jan 2015, Chris Wilson wrote: >> > This looked like an odd regression from >> > >> > commit ec5cc0f9b019af95e4571a9fa162d94294c8d90b >> > Author: Chris Wilson >> > Date: Thu Jun 12 10:28:55 2014 +0100 >> > >> > drm/i915: Restrict GPU boost to the RCS engine >> > >> > but in reality it undercovered a much older coherency bug. The issue that >> > boosting the GPU frequency on the BCS ring was masking was that we could >> > wake the CPU up after completion of a BCS batch and inspect memory prior >> > to the write cache being fully evicted. In order to serialise the >> > breadcrumb interrupt (and so ensure that the CPU's view of memory is >> > coherent) we need to perform a post-sync operation in the MI_FLUSH_DW. >> > >> > v2: Fix all the MI_FLUSH_DW (bsd plus the duplication in execlists). >> > >> > Testcase: gpuX-rcs-gpu-read-after-write >> > Signed-off-by: Chris Wilson >> > Cc: sta...@vger.kernel.org >> > Acked-by: Daniel Vetter >> > --- >> > drivers/gpu/drm/i915/intel_lrc.c| 20 +++- >> > drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++ >> > 2 files changed, 30 insertions(+), 13 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c >> > b/drivers/gpu/drm/i915/intel_lrc.c >> > index e405b61cdac5..8e71d8851c9a 100644 >> > --- a/drivers/gpu/drm/i915/intel_lrc.c >> > +++ b/drivers/gpu/drm/i915/intel_lrc.c >> > @@ -1237,15 +1237,17 @@ static int gen8_emit_flush(struct intel_ringbuffer >> > *ringbuf, >> > >> >cmd = MI_FLUSH_DW + 1; >> > >> > - if (ring == &dev_priv->ring[VCS]) { >> > - if (invalidate_domains & I915_GEM_GPU_DOMAINS) >> > - cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD | >> > - MI_FLUSH_DW_STORE_INDEX | >> > - MI_FLUSH_DW_OP_STOREDW; >> > - } else { >> > - if (invalidate_domains & I915_GEM_DOMAIN_RENDER) >> > - cmd |= MI_INVALIDATE_TLB | MI_FLUSH_DW_STORE_INDEX | >> > - MI_FLUSH_DW_OP_STOREDW; >> > + /* We always require a command barrier so that subsequent >> > + * commands, such as breadcrumb interrupts, are strictly ordered >> > + * wrt the contents of the write cache being flushed to memory >> > + * (and thus being coherent from the CPU). >> > + */ >> > + cmd |= MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; >> > + >> > + if (invalidate_domains & I915_GEM_GPU_DOMAINS) { >> >> Why do you change the mask from I915_GEM_DOMAIN_RENDER to >> I915_GEM_GPU_DOMAINS for ring != VCS? > > My bad, I didn't notice that execlists was originally broken. The patch > is correct. I'll take your and Daniel's word for it. I hope I won't have to regret not asking you to split this into two patches... Pushed to drm-intel-next-fixes, thanks for the patch and ack. BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Correct the base value while updating LP_OUTPUT_HOLD in MIPI_PORT_CTRL
On Thu, 05 Feb 2015, Shobhit Kumar wrote: > LP_OUTPUT_HOLD is only in MIPI_PORT_CTRL(PORT_A) even for PORT_C in case > of dual link. In the dual link implementation, the bit is correctly set > or unset for hardcoded PORT_A, but for bit update the register base value > is read by using MIPI_PORT_CTRL(port) in a loop. The second iteration will > read base value from PORT_C and program for PORT_A. Mostly in case of dual > link all other bit values should be same, but logically we should read from > PORT_A. So hardcode to read initial value from PORT_A as well. > > Signed-off-by: Shobhit Kumar Pushed to drm-intel-next-fixes, thanks for the patch. BR, Jani. > --- > drivers/gpu/drm/i915/intel_dsi.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > b/drivers/gpu/drm/i915/intel_dsi.c > index 6857d19..3fe8a1e 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -177,12 +177,11 @@ static void intel_dsi_device_ready(struct intel_encoder > *encoder) > I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER); > usleep_range(2500, 3000); > > - val = I915_READ(MIPI_PORT_CTRL(port)); > - > /* Enable MIPI PHY transparent latch >* Common bit for both MIPI Port A & MIPI Port C >* No similar bit in MIPI Port C reg >*/ > + val = I915_READ(MIPI_PORT_CTRL(PORT_A)); > I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD); > usleep_range(1000, 1500); > > @@ -360,10 +359,10 @@ static void intel_dsi_clear_device_ready(struct > intel_encoder *encoder) > == 0x0), 30)) > DRM_ERROR("DSI LP not going Low\n"); > > - val = I915_READ(MIPI_PORT_CTRL(port)); > /* Disable MIPI PHY transparent latch >* Common bit for both MIPI Port A & MIPI Port C >*/ > + val = I915_READ(MIPI_PORT_CTRL(PORT_A)); > I915_WRITE(MIPI_PORT_CTRL(PORT_A), val & ~LP_OUTPUT_HOLD); > usleep_range(1000, 1500); > > -- > 1.9.1 > -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: pass which operation triggered the frontbuffer tracking
On Mon, Feb 09, 2015 at 02:46:30PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > We want to port FBC to the frontbuffer tracking infrastructure, but > for that we need to know what caused the object invalidation/flush so > we can react accordingly: CPU mmaps need manual, GTT mmaps and > flips don't need handling and ring rendering needs nukes. > > v2: - s/ORIGIN_RENDER/ORIGIN_CS/ (Daniel, Rodrigo) > - Fix copy/pasted wrong documentation > - Rebase > v3: - Rebase > > Reviewed-by: Rodrigo Vivi (v2) > Signed-off-by: Paulo Zanoni Sorry for being really late with my review here. I fully agree that origin for invalidate makes a lot of sense, but for flush it kinda doesn't. There's no origin for a flush, it just means that any previous rendering (from _any_ origin) has now completed. I've looked ahead a bit and I'll comment on the patch that uses the flush origin what should imo be used instead. Merged the first 3 patches from this series meanwhile, thanks. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h| 7 +++ > drivers/gpu/drm/i915/i915_gem.c| 10 +- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 11 +++ > drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++- > 5 files changed, 30 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index c0b8644..d578211 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -771,6 +771,13 @@ struct intel_context { > struct list_head link; > }; > > +enum fb_op_origin { > + ORIGIN_GTT, > + ORIGIN_CPU, > + ORIGIN_CS, > + ORIGIN_FLIP, > +}; > + > struct i915_fbc { > unsigned long uncompressed_size; > unsigned threshold; > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c26d36c..3d198f8 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2310,7 +2310,7 @@ i915_gem_object_move_to_inactive(struct > drm_i915_gem_object *obj) > list_move_tail(&vma->mm_list, &vma->vm->inactive_list); > } > > - intel_fb_obj_flush(obj, true); > + intel_fb_obj_flush(obj, true, ORIGIN_CS); > > list_del_init(&obj->ring_list); > > @@ -3677,7 +3677,7 @@ i915_gem_object_flush_gtt_write_domain(struct > drm_i915_gem_object *obj) > old_write_domain = obj->base.write_domain; > obj->base.write_domain = 0; > > - intel_fb_obj_flush(obj, false); > + intel_fb_obj_flush(obj, false, ORIGIN_GTT); > > trace_i915_gem_object_change_domain(obj, > obj->base.read_domains, > @@ -3699,7 +3699,7 @@ i915_gem_object_flush_cpu_write_domain(struct > drm_i915_gem_object *obj) > old_write_domain = obj->base.write_domain; > obj->base.write_domain = 0; > > - intel_fb_obj_flush(obj, false); > + intel_fb_obj_flush(obj, false, ORIGIN_CPU); > > trace_i915_gem_object_change_domain(obj, > obj->base.read_domains, > @@ -3764,7 +3764,7 @@ i915_gem_object_set_to_gtt_domain(struct > drm_i915_gem_object *obj, bool write) > } > > if (write) > - intel_fb_obj_invalidate(obj, NULL); > + intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT); > > trace_i915_gem_object_change_domain(obj, > old_read_domains, > @@ -4079,7 +4079,7 @@ i915_gem_object_set_to_cpu_domain(struct > drm_i915_gem_object *obj, bool write) > } > > if (write) > - intel_fb_obj_invalidate(obj, NULL); > + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU); > > trace_i915_gem_object_change_domain(obj, > old_read_domains, > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index b773368..f4342f0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -971,7 +971,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, > obj->dirty = 1; > i915_gem_request_assign(&obj->last_write_req, req); > > - intel_fb_obj_invalidate(obj, ring); > + intel_fb_obj_invalidate(obj, ring, ORIGIN_CS); > > /* update for the implicit flush after a batch */ > obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 76b3c20..a73d091 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -853,13 +853,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc > *crtc, bool state); > > /* intel_frontbuffer.c */ > void intel_fb_obj_invalidate(s
Re: [Intel-gfx] [PATCH] drm/i915: Do not invalidate obj->pages under mempressure
On 02/09/2015 08:46 AM, Chris Wilson wrote: > On Sun, Feb 08, 2015 at 03:27:13PM -0800, Sean V Kelley wrote: >> >> >> On 01/16/2015 08:05 PM, Daniel Vetter wrote: >>> On Thu, Jan 15, 2015 at 08:44:00PM +, Chris Wilson wrote: On Thu, Jan 15, 2015 at 08:36:15PM +0100, Daniel Vetter wrote: > On Wed, Jan 14, 2015 at 9:34 PM, Chris Wilson > wrote: >> This (partially) reverts >> >> commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd Author: >> Chris Wilson Date: Tue Mar >> 25 13:23:06 2014 + >> >> drm/i915: Invalidate our pages under memory pressure > > Shouldn't we also revert the hunk in > i915_gem_free_objects? Without the truncate vs. invalidate > disdinction it seems to have lost it's reason for existence > ... No, setting MADV_DONTNEED has other nice properties during put_pages() - I think it is useful in its own right, for example that is where my page stealing code goes... >>> >>> Well right now I can't make sense of this bit any more (tbh I >>> didn't with the other code either, but overlooked that while >>> reviewing). When it's just there for future work but atm dead >>> code I prefer for it to get removed. -Daniel >> >> >> So can we also revert the hunk in i915_gem_free_objects? I would >> like to get this patch merged, it looks like that is the primary >> concern. > > A problem I have is that the test written to hit the exact > condition considered in the changelog does not ellict the bug. > > Can you test whether > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index > 39e032615b31..6269204ba16f 100644 --- > a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1030,6 +1030,7 @@ > i915_gem_execbuffer_move_to_active(struct list_head *vmas, /* > update for the implicit flush after a batch */ > obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; } + > obj->dirty = 1; if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) { > i915_gem_request_assign(&obj->last_fenced_req, req); if > (entry->flags & __EXEC_OBJECT_HAS_FENCE) { > > makes the bug go away. If so, I think the bug is in the caller not > setting reloc domains correctly. -Chris Sure, I will take a look. Thanks, Sean > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: also do frontbuffer tracking on pwrites
On Mon, Feb 09, 2015 at 02:46:31PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > We need this for FBC, and possibly for PSR too. > > Reviewed-by: Rodrigo Vivi > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_gem.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 3d198f8..15910fa 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -,6 +,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void > *data, > ret = i915_gem_phys_pwrite(obj, args, file); > else > ret = i915_gem_shmem_pwrite(dev, obj, args, file); > + > + intel_fb_obj_flush(obj, false, ORIGIN_CPU); > + } else { > + intel_fb_obj_flush(obj, false, ORIGIN_GTT); A flush alone does nothing. Well should, but you're kinda not quite using it correctly in the next patch to convert fbc over to frontbuffer tracking. I guess the docs aren't perfect, so let me try again. There are two kinds of events the frontbuffer tracking code supplies to tell its consumers that screen changes are happening: - invalidate/flush: Invalidate denotes the start of the frontbuffer rendering, from that point on psr/fbc/drrs must update the screen with the usual refresh rate and not cache anything anywhere. When the flush happens (which could easily be after a _very_ long time, e.g. fbcon) then only can caching recomence. Caching = enable fbc, allow psr or reduce refresh rate. - flip events: That's an instantenous event (well at least for consumer, internally we need to track it as prepare/complete for async flips), and mostly just interesting when the hw doesn't notice flips (some psr modes and drrs). So if you want to add frontbuffer tracking to pwrite then we need both an invalidate (before the actual pwrite) and a flush (after the pwrite, like you've added here). The other issue is that there's a bug with the origin assignemnt: phys_pwrite also goes through the gtt. I think it would be best if we push the fb_obj_invalidate/flush into the relevant pwrite functions. That should make it easier to review, since the invalidate/flush will be next to the write op. -Daniel > } > > out: > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: add frontbuffer tracking to FBC
On Mon, Feb 09, 2015 at 02:46:32PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > Kill the blt/render tracking we currently have and use the frontbuffer > tracking infrastructure. > > Don't enable things by default yet. > > v2: (Rodrigo) Fix small conflict on rebase and typo at subject. > v3: (Paulo) Rebase on RENDER_CS change. > v4: (Paulo) Rebase. > > Reviewed-by: Rodrigo Vivi (v2) > Signed-off-by: Rodrigo Vivi > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_drv.h | 9 +-- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > drivers/gpu/drm/i915/intel_fbc.c | 107 > --- > drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +--- > drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +--- > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > 6 files changed, 80 insertions(+), 98 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index d578211..dc002df 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -782,6 +782,7 @@ struct i915_fbc { > unsigned long uncompressed_size; > unsigned threshold; > unsigned int fb_id; > + unsigned int possible_framebuffer_bits; > struct intel_crtc *crtc; > int y; > > @@ -794,14 +795,6 @@ struct i915_fbc { >* possible. */ > bool enabled; > > - /* On gen8 some rings cannont perform fbc clean operation so for now > - * we are doing this on SW with mmio. > - * This variable works in the opposite information direction > - * of ring->fbc_dirty telling software on frontbuffer tracking > - * to perform the cache clean on sw side. > - */ > - bool need_sw_cache_clean; > - > struct intel_fbc_work { > struct delayed_work work; > struct drm_crtc *crtc; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index a73d091..ed85df8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1101,7 +1101,11 @@ bool intel_fbc_enabled(struct drm_device *dev); > void intel_fbc_update(struct drm_device *dev); > void intel_fbc_init(struct drm_i915_private *dev_priv); > void intel_fbc_disable(struct drm_device *dev); > -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value); > +void intel_fbc_invalidate(struct drm_i915_private *dev_priv, > + unsigned int frontbuffer_bits, > + enum fb_op_origin origin); > +void intel_fbc_flush(struct drm_i915_private *dev_priv, > + unsigned int frontbuffer_bits, enum fb_op_origin origin); > > /* intel_hdmi.c */ > void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port); > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > b/drivers/gpu/drm/i915/intel_fbc.c > index 7341e87..26c4f9c 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -174,29 +174,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev) > return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN; > } > > -static void snb_fbc_blit_update(struct drm_device *dev) > +static void intel_fbc_nuke(struct drm_i915_private *dev_priv) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > - u32 blt_ecoskpd; > - > - /* Make sure blitter notifies FBC of writes */ > - > - /* Blitter is part of Media powerwell on VLV. No impact of > - * his param in other platforms for now */ > - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA); > - > - blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD); > - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY << > - GEN6_BLITTER_LOCK_SHIFT; > - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); > - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY; > - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); > - blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY << > - GEN6_BLITTER_LOCK_SHIFT); > - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); > - POSTING_READ(GEN6_BLITTER_ECOSKPD); > - > - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA); > + I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE); > + POSTING_READ(MSG_FBC_REND_STATE); > } > > static void ilk_fbc_enable(struct drm_crtc *crtc) > @@ -239,9 +220,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc) > I915_WRITE(SNB_DPFC_CTL_SA, > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); > - snb_fbc_blit_update(dev); > } > > + intel_fbc_nuke(dev_priv); > + > DRM_DEBUG_KMS("enabled fbc on plane %c\n", > plane_name(intel_crtc->plane)); > } > > @@ -320,7 +302,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc) > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y); > > - snb_fbc_blit_update(dev); > + intel_fb
[Intel-gfx] [PATCH v2] drm/i915/bdw: Implement WaForceContextSaveRestoreNonCoherent
v2: Reorder defines (Ben) Reviewed-by: Ben Widawsky Reviewed-by: Ville Syrjälä Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b4abd50..4b8450d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5273,10 +5273,11 @@ enum skl_disp_power_wells { /* GEN8 chicken */ #define HDC_CHICKEN0 0x7300 -#define HDC_FORCE_NON_COHERENT(1<<4) -#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) #define HDC_FENCE_DEST_SLM_DISABLE(1<<14) +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) #define HDC_BARRIER_PERFORMANCE_DISABLE (1<<10) +#define HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT (1<<5) +#define HDC_FORCE_NON_COHERENT(1<<4) /* WaCatErrorRejectionIssue */ #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 46486a5..a9cde64 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -790,9 +790,11 @@ static int bdw_init_workarounds(struct intel_engine_cs *ring) */ /* WaForceEnableNonCoherent:bdw */ /* WaHdcDisableFetchWhenMasked:bdw */ + /* WaForceContextSaveRestoreNonCoherent:bdw */ /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */ WA_SET_BIT_MASKED(HDC_CHICKEN0, HDC_FORCE_NON_COHERENT | + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | HDC_DONOT_FETCH_MEM_WHEN_MASKED | (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0)); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/18] A few more workarounds
Following Nick's work, I found a few more. -- Damien Damien Lespiau (18): drm/i915: Support not having an init clock gating function defined drm/i915/skl: Implement WaDisableHBR2 drm/i915/skl: Document the WM read latency W/A with its name drm/i915/skl: Provide a gen9 specific init_render_ring() drm/i915/skl: Make the init clock gating function skylake specific drm/i915/skl: Implement WaSetGAPSunitClckGateDisable drm/i915/skl: Implement WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken drm/i915/skl: Document that we implement WaRsClearFWBitsAtReset drm/i915/skl: Implement WaEnableLbsSlaRetryTimerDecrement drm/i915/skl: Implement WaDisableVFUnitClockGating drm/i915/skl: Introduce a SKL specific init_workarounds() drm/i915/skl: Implement WaDisablePowerCompilerClockGating drm/i915/skl: Implement WaDisablePartialResolveInVc drm/i915/skl: Implement WaDisableLSQCROPERFforOCL drm/i915/skl: Implement WaDisableHDCInvalidation drm/i915/skl: Implement WaDisableChickenBitTSGBarrierAckForFFSliceCS drm/i915/skl: Implement WaCcsTlbPrefetchDisable:skl drm/i915/skl: Implement WaBarrierPerformanceFixDisable drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 24 +-- drivers/gpu/drm/i915/intel_dp.c | 5 +++- drivers/gpu/drm/i915/intel_lrc.c| 16 - drivers/gpu/drm/i915/intel_pm.c | 41 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 41 - drivers/gpu/drm/i915/intel_uncore.c | 1 + 7 files changed, 121 insertions(+), 8 deletions(-) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/18] drm/i915/skl: Make the init clock gating function skylake specific
We'll gather cross-gen9 W/A in a separate function later. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6fd6f26..03e27c2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -52,7 +52,7 @@ #define INTEL_RC6p_ENABLE (1<<1) #define INTEL_RC6pp_ENABLE (1<<2) -static void gen9_init_clock_gating(struct drm_device *dev) +static void skl_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -6415,7 +6415,7 @@ void intel_init_pm(struct drm_device *dev) if (INTEL_INFO(dev)->gen >= 9) { skl_setup_wm_latency(dev); - dev_priv->display.init_clock_gating = gen9_init_clock_gating; + dev_priv->display.init_clock_gating = skl_init_clock_gating; dev_priv->display.update_wm = skl_update_wm; dev_priv->display.update_sprite_wm = skl_update_sprite_wm; } else if (HAS_PCH_SPLIT(dev)) { -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/18] drm/i915/skl: Document that we implement WaRsClearFWBitsAtReset
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_uncore.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index c47a3ba..ad71575 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -972,6 +972,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv, d->val_set = FORCEWAKE_KERNEL; d->val_clear = 0; } else { + /* WaRsClearFWBitsAtReset:bdw,skl */ d->val_reset = _MASKED_BIT_DISABLE(0x); d->val_set = _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL); d->val_clear = _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/18] drm/i915/skl: Implement WaSetGAPSunitClckGateDisable
Let's also take the opportunity the remove the comment telling it's a pre-prod W/A, it should be obvious from the stepping test. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ee1964..578fd90 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6016,6 +6016,7 @@ enum skl_disp_power_wells { #define GEN6_RSTCTL0x9420 #define GEN8_UCGCTL6 0x9430 +#define GEN8_GAPSUNIT_CLOCK_GATE_DISABLE (1<<24) #define GEN8_SDEUNIT_CLOCK_GATE_DISABLE (1<<14) #define GEN6_GFXPAUSE 0xA000 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 03e27c2..2c66423 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -59,9 +59,10 @@ static void skl_init_clock_gating(struct drm_device *dev) if (INTEL_REVID(dev) == SKL_REVID_A0) { /* * WaDisableSDEUnitClockGating:skl -* This seems to be a pre-production w/a. +* WaSetGAPSunitClckGateDisable:skl */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | + GEN8_GAPSUNIT_CLOCK_GATE_DISABLE | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); } } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/18] drm/i915/skl: Document the WM read latency W/A with its name
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a3b979d..6fd6f26 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1701,6 +1701,8 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8]) GEN9_MEM_LATENCY_LEVEL_MASK; /* +* WaWmMemoryReadLatency:skl +* * punit doesn't take into account the read latency so we need * to add 2us to the various latency levels we retrieve from * the punit. -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/18] drm/i915/skl: Implement WaDisablePowerCompilerClockGating
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 8 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a457c28..fdfbdb3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5241,8 +5241,9 @@ enum skl_disp_power_wells { #define COMMON_SLICE_CHICKEN2 0x7014 # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (1<<0) -#define HIZ_CHICKEN0x7018 -# define CHV_HZ_8X8_MODE_IN_1X (1<<15) +#define HIZ_CHICKEN0x7018 +# define CHV_HZ_8X8_MODE_IN_1X (1<<15) +# define BDW_HIZ_POWER_COMPILER_CLOCK_GATING_DISABLE (1<<3) #define GEN9_SLICE_COMMON_ECO_CHICKEN0 0x7308 #define DISABLE_PIXEL_MASK_CAMMING(1<<14) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 27d101c..3135192 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -930,8 +930,16 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) static int skl_init_workarounds(struct intel_engine_cs *ring) { + struct drm_device *dev = ring->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + gen9_init_workarounds(ring); + /* WaDisablePowerCompilerClockGating:skl */ + if (INTEL_REVID(dev) == SKL_REVID_B0) + WA_SET_BIT_MASKED(HIZ_CHICKEN, + BDW_HIZ_POWER_COMPILER_CLOCK_GATING_DISABLE); + return 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/18] drm/i915/skl: Provide a gen9 specific init_render_ring()
WaDisableAsyncFlipPerfMode isn't listed for SKL and INSTPM_FORCE_ORDERING is MBZ so let's make a gen9 specific render init function. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_lrc.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d05f3bc..fe25ced 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1149,6 +1149,17 @@ static int gen8_init_render_ring(struct intel_engine_cs *ring) return init_workarounds_ring(ring); } +static int gen9_init_render_ring(struct intel_engine_cs *ring) +{ + int ret; + + ret = gen8_init_common_ring(ring); + if (ret) + return ret; + + return init_workarounds_ring(ring); +} + static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf, struct intel_context *ctx, u64 offset, unsigned flags) @@ -1408,7 +1419,10 @@ static int logical_render_ring_init(struct drm_device *dev) if (HAS_L3_DPF(dev)) ring->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT; - ring->init_hw = gen8_init_render_ring; + if (INTEL_INFO(dev)->gen >= 9) + ring->init_hw = gen9_init_render_ring; + else + ring->init_hw = gen8_init_render_ring; ring->init_context = gen8_init_rcs_context; ring->cleanup = intel_fini_pipe_control; ring->get_seqno = gen8_get_seqno; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/18] drm/i915/skl: Introduce a SKL specific init_workarounds()
This function will host SKL-only W/As. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index b15d596..27d101c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -928,6 +928,13 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) return 0; } +static int skl_init_workarounds(struct intel_engine_cs *ring) +{ + gen9_init_workarounds(ring); + + return 0; +} + int init_workarounds_ring(struct intel_engine_cs *ring) { struct drm_device *dev = ring->dev; @@ -943,7 +950,9 @@ int init_workarounds_ring(struct intel_engine_cs *ring) if (IS_CHERRYVIEW(dev)) return chv_init_workarounds(ring); - if (IS_GEN9(dev)) + if (IS_SKYLAKE(dev)) + return skl_init_workarounds(ring); + else if (IS_GEN9(dev)) return gen9_init_workarounds(ring); return 0; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 17/18] drm/i915/skl: Implement WaCcsTlbPrefetchDisable:skl
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 4 2 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 610fcd4..090ddd7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6216,6 +6216,7 @@ enum skl_disp_power_wells { #define GEN9_HALF_SLICE_CHICKEN5 0xe188 #define GEN9_DG_MIRROR_FIX_ENABLE(1<<5) +#define GEN9_CCS_TLB_PREFETCH_ENABLE (1<<3) #define GEN8_ROW_CHICKEN 0xe4f0 #define PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE(1<<8) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index db83baf..57432ca 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -928,6 +928,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) /* WaDisablePartialResolveInVc:skl */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE); + /* WaCcsTlbPrefetchDisable:skl */ + WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, + GEN9_CCS_TLB_PREFETCH_ENABLE); + return 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/18] drm/i915/skl: Implement WaDisableLSQCROPERFforOCL
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 5 + 3 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c0b8644..0765bd1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2391,6 +2391,7 @@ struct drm_i915_cmd_table { #define SKL_REVID_B0 (0x1) #define SKL_REVID_C0 (0x2) #define SKL_REVID_D0 (0x3) +#define SKL_REVID_E0 (0x4) /* * The genX designation typically refers to the render engine, so render diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d519ed9..3ae7a09 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5264,6 +5264,9 @@ enum skl_disp_power_wells { #define GEN7_L3SQCREG4 0xb034 #define L3SQ_URB_READ_CAM_MATCH_DISABLE (1<<27) +#define GEN8_L3SQCREG4 0xb118 +#define GEN8_LQSC_RO_PERF_DIS (1<<27) + /* GEN8 chicken */ #define HDC_CHICKEN0 0x7300 #define HDC_FORCE_NON_COHERENT(1<<4) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ff4e289..f53ef11 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -80,6 +80,11 @@ static void skl_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN6_UCGCTL2, I915_READ(GEN6_UCGCTL2) | GEN6_VFUNIT_CLOCK_GATE_DISABLE); } + + if (INTEL_REVID(dev) <= SKL_REVID_E0) + /* WaDisableLSQCROPERFforOCL:skl */ + I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | + GEN8_LQSC_RO_PERF_DIS); } static void i915_pineview_get_mem_freq(struct drm_device *dev) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/18] drm/i915/skl: Implement WaDisablePartialResolveInVc
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index fdfbdb3..d519ed9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1483,6 +1483,7 @@ enum skl_disp_power_wells { #define CACHE_MODE_1 0x7004 /* IVB+ */ #define PIXEL_SUBSPAN_COLLECT_OPT_DISABLE(1<<6) #define GEN8_4x4_STC_OPTIMIZATION_DISABLE(1<<6) +#define GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE (1<<1) #define GEN6_BLITTER_ECOSKPD 0x221d0 #define GEN6_BLITTER_LOCK_SHIFT 16 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 3135192..db83baf 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -925,6 +925,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) /* Wa4x4STCOptimizationDisable:skl */ WA_SET_BIT_MASKED(CACHE_MODE_1, GEN8_4x4_STC_OPTIMIZATION_DISABLE); + /* WaDisablePartialResolveInVc:skl */ + WA_SET_BIT_MASKED(CACHE_MODE_1, GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE); + return 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/18] drm/i915/skl: Implement WaDisableHDCInvalidation
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 6 ++ 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3ae7a09..b363c5e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -140,6 +140,7 @@ #define GEN8_RING_PDP_LDW(ring, n) ((ring)->mmio_base+0x270 + (n) * 8) #define GAM_ECOCHK 0x4090 +#define BDW_DISABLE_HDC_INVALIDATION (1<<25) #define ECOCHK_SNB_BIT (1<<10) #define HSW_ECOCHK_ARB_PRIO_SOL (1<<6) #define ECOCHK_PPGTT_CACHE64B(0x3<<3) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f53ef11..8f9149b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -81,6 +81,12 @@ static void skl_init_clock_gating(struct drm_device *dev) GEN6_VFUNIT_CLOCK_GATE_DISABLE); } + if (INTEL_REVID(dev) <= SKL_REVID_D0) + /* WaDisableHDCInvalidation:skl */ + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + BDW_DISABLE_HDC_INVALIDATION); + + if (INTEL_REVID(dev) <= SKL_REVID_E0) /* WaDisableLSQCROPERFforOCL:skl */ I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/18] drm/i915/skl: Implement WaDisableHBR2
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_dp.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d4c82d7..4a60c6a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -129,7 +129,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp) case DP_LINK_BW_2_7: break; case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */ - if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || + if (IS_SKYLAKE(dev) && dev->pdev->revision <= 0x1) + /* WaDisableHBR2:skl */ + max_link_bw = DP_LINK_BW_2_7; + else if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || INTEL_INFO(dev)->gen >= 8) && intel_dp->dpcd[DP_DPCD_REV] >= 0x12) max_link_bw = DP_LINK_BW_5_4; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/18] drm/i915/skl: Implement WaDisableVFUnitClockGating
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 4 2 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2043e82..a457c28 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6007,6 +6007,7 @@ enum skl_disp_power_wells { # define GEN6_CSUNIT_CLOCK_GATE_DISABLE(1 << 7) #define GEN6_UCGCTL2 0x9404 +# define GEN6_VFUNIT_CLOCK_GATE_DISABLE(1 << 31) # define GEN7_VDSUNIT_CLOCK_GATE_DISABLE (1 << 30) # define GEN7_TDLUNIT_CLOCK_GATE_DISABLE (1 << 22) # define GEN6_RCZUNIT_CLOCK_GATE_DISABLE (1 << 13) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ed029e7..ff4e289 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -75,6 +75,10 @@ static void skl_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_GAPSUNIT_CLOCK_GATE_DISABLE | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); + + /* WaDisableVFUnitClockGating:skl */ + I915_WRITE(GEN6_UCGCTL2, I915_READ(GEN6_UCGCTL2) | + GEN6_VFUNIT_CLOCK_GATE_DISABLE); } } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/18] drm/i915/skl: Implement WaEnableLbsSlaRetryTimerDecrement
This W/A is put in a gen9 specific function because it may well be needed on other gen9 platforms. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 11 +++ 2 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index cb66c8f..2043e82 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5275,6 +5275,9 @@ enum skl_disp_power_wells { #define HSW_SCRATCH1 0xb038 #define HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE (1<<27) +#define BDW_SCRATCH1 0xb11c +#define GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE (1<<2) + /* PCH */ /* south display engine interrupt: IBX */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2c66423..ed029e7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -52,10 +52,21 @@ #define INTEL_RC6p_ENABLE (1<<1) #define INTEL_RC6pp_ENABLE (1<<2) +static void gen9_init_clock_gating(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + /* WaEnableLbsSlaRetryTimerDecrement:skl */ + I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | + GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); +} + static void skl_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + gen9_init_clock_gating(dev); + if (INTEL_REVID(dev) == SKL_REVID_A0) { /* * WaDisableSDEUnitClockGating:skl -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/18] drm/i915/skl: Implement WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 578fd90..cb66c8f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5237,12 +5237,16 @@ enum skl_disp_power_wells { /* GEN7 chicken */ #define GEN7_COMMON_SLICE_CHICKEN1 0x7010 # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((1<<10) | (1<<26)) +# define GEN9_RHWO_OPTIMIZATION_DISABLE(1<<14) #define COMMON_SLICE_CHICKEN2 0x7014 # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (1<<0) #define HIZ_CHICKEN0x7018 # define CHV_HZ_8X8_MODE_IN_1X (1<<15) +#define GEN9_SLICE_COMMON_ECO_CHICKEN0 0x7308 +#define DISABLE_PIXEL_MASK_CAMMING(1<<14) + #define GEN7_L3SQCREG1 0xB010 #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D3 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 293d1b6..b15d596 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -897,6 +897,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) ~GEN9_DG_MIRROR_FIX_ENABLE); } + if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) { + /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl */ + WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1, + GEN9_RHWO_OPTIMIZATION_DISABLE); + WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0, + DISABLE_PIXEL_MASK_CAMMING); + } + if (INTEL_REVID(dev) >= SKL_REVID_C0) { /* WaEnableYV12BugFixInHalfSliceChicken7:skl */ WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/18] drm/i915: Support not having an init clock gating function defined
When enabling new platforms, we may not have any W/A to apply, especially that, now, a bunch of them have to be done from the ring. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3c64810..a3b979d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6386,7 +6386,8 @@ void intel_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - dev_priv->display.init_clock_gating(dev); + if (dev_priv->display.init_clock_gating) + dev_priv->display.init_clock_gating(dev); } void intel_suspend_hw(struct drm_device *dev) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 16/18] drm/i915/skl: Implement WaDisableChickenBitTSGBarrierAckForFFSliceCS
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 7 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b363c5e..610fcd4 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5236,6 +5236,9 @@ enum skl_disp_power_wells { #define HSW_NDE_RSTWRN_OPT 0x46408 #define RESET_PCH_HANDSHAKE_ENABLE(1<<4) +#define FF_SLICE_CS_CHICKEN2 0x02e4 +#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8) + /* GEN7 chicken */ #define GEN7_COMMON_SLICE_CHICKEN1 0x7010 # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((1<<10) | (1<<26)) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8f9149b..6254102 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -81,11 +81,16 @@ static void skl_init_clock_gating(struct drm_device *dev) GEN6_VFUNIT_CLOCK_GATE_DISABLE); } - if (INTEL_REVID(dev) <= SKL_REVID_D0) + if (INTEL_REVID(dev) <= SKL_REVID_D0) { /* WaDisableHDCInvalidation:skl */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | BDW_DISABLE_HDC_INVALIDATION); + /* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */ + I915_WRITE(FF_SLICE_CS_CHICKEN2, + I915_READ(FF_SLICE_CS_CHICKEN2) | + GEN9_TSG_BARRIER_ACK_DISABLE); + } if (INTEL_REVID(dev) <= SKL_REVID_E0) /* WaDisableLSQCROPERFforOCL:skl */ -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/18] drm/i915/skl: Implement WaBarrierPerformanceFixDisable
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++ 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 090ddd7..b4abd50 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5276,6 +5276,7 @@ enum skl_disp_power_wells { #define HDC_FORCE_NON_COHERENT(1<<4) #define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) #define HDC_FENCE_DEST_SLM_DISABLE(1<<14) +#define HDC_BARRIER_PERFORMANCE_DISABLE (1<<10) /* WaCatErrorRejectionIssue */ #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 57432ca..93365fe 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -947,6 +947,13 @@ static int skl_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(HIZ_CHICKEN, BDW_HIZ_POWER_COMPILER_CLOCK_GATING_DISABLE); + if (INTEL_REVID(dev) == SKL_REVID_C0 || + INTEL_REVID(dev) == SKL_REVID_D0) + /* WaBarrierPerformanceFixDisable:skl */ + WA_SET_BIT_MASKED(HDC_CHICKEN0, + HDC_FENCE_DEST_SLM_DISABLE | + HDC_BARRIER_PERFORMANCE_DISABLE); + return 0; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/bdw: Implement WaForceContextSaveRestoreNonCoherent
On Mon, Feb 09, 2015 at 07:25:56PM +, Damien Lespiau wrote: > v2: Reorder defines (Ben) Bikeshed time? > /* WaCatErrorRejectionIssue */ > #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 46486a5..a9cde64 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -790,9 +790,11 @@ static int bdw_init_workarounds(struct intel_engine_cs > *ring) >*/ > /* WaForceEnableNonCoherent:bdw */ > /* WaHdcDisableFetchWhenMasked:bdw */ > + /* WaForceContextSaveRestoreNonCoherent:bdw */ > /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */ > WA_SET_BIT_MASKED(HDC_CHICKEN0, > HDC_FORCE_NON_COHERENT | > + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | > HDC_DONOT_FETCH_MEM_WHEN_MASKED | > (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0)); This is nicer as: WA_SET_BIT_MASKED(HDC_CHICKEN0, /* WaForceEnableNonCoherent:bdw */ HDC_FORCE_NON_COHERENT | + /* WaForceContextSaveRestoreNonCoherent:bdw */ + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | /* WaHdcDisableFetchWhenMasked:bdw */ HDC_DONOT_FETCH_MEM_WHEN_MASKED | /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */ (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0)); Indeed, that shows that you misorded your comment! ;-) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: obey wbinvd threshold in more places
Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.h | 4 drivers/gpu/drm/i915/i915_gem.c | 32 drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ++--- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5d2f62d..dfecdfd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2818,6 +2818,10 @@ static inline bool cpu_cache_is_coherent(struct drm_device *dev, { return HAS_LLC(dev) || level != I915_CACHE_NONE; } +static inline bool i915_gem_obj_should_clflush(struct drm_i915_gem_object *obj) +{ + return obj->base.size >= to_i915(obj->base.dev)->wbinvd_threshold; +} int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj); int __must_check i915_gem_init(struct drm_device *dev); int i915_gem_init_rings(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 4d5a69d..59be709 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -204,6 +204,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) char *vaddr = obj->phys_handle->vaddr; struct sg_table *st; struct scatterlist *sg; + const bool do_wbinvd = i915_gem_obj_should_clflush(obj); int i; if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj))) @@ -219,12 +220,15 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) src = kmap_atomic(page); memcpy(vaddr, src, PAGE_SIZE); - drm_clflush_virt_range(vaddr, PAGE_SIZE); + if (!do_wbinvd) + drm_clflush_virt_range(vaddr, PAGE_SIZE); kunmap_atomic(src); page_cache_release(page); vaddr += PAGE_SIZE; } + if (do_wbinvd) + wbinvd(); i915_gem_chipset_flush(obj->base.dev); @@ -252,6 +256,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) static void i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj) { + const bool do_wbinvd = i915_gem_obj_should_clflush(obj); int ret; BUG_ON(obj->madv == __I915_MADV_PURGED); @@ -282,7 +287,8 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj) continue; dst = kmap_atomic(page); - drm_clflush_virt_range(vaddr, PAGE_SIZE); + if (!do_wbinvd) + drm_clflush_virt_range(vaddr, PAGE_SIZE); memcpy(dst, vaddr, PAGE_SIZE); kunmap_atomic(dst); @@ -295,6 +301,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj) obj->dirty = 0; } + if (do_wbinvd && !ret) + wbinvd(); + sg_free_table(obj->pages); kfree(obj->pages); @@ -396,7 +405,10 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, return -EFAULT; } - drm_clflush_virt_range(vaddr, args->size); + if (args->size >= to_i915(obj->base.dev)->wbinvd_threshold) + wbinvd(); + else + drm_clflush_virt_range(vaddr, args->size); i915_gem_chipset_flush(dev); return 0; } @@ -647,6 +659,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int obj_do_bit17_swizzling, page_do_bit17_swizzling; int prefaulted = 0; int needs_clflush = 0; + bool do_wbinvd = false; struct sg_page_iter sg_iter; user_data = to_user_ptr(args->data_ptr); @@ -658,6 +671,9 @@ i915_gem_shmem_pread(struct drm_device *dev, if (ret) return ret; + if (needs_clflush && i915_gem_obj_should_clflush(obj)) + do_wbinvd = true; + offset = args->offset; for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, @@ -714,6 +730,9 @@ next_page: } out: + if (do_wbinvd && !ret) + wbinvd(); + i915_gem_object_unpin_pages(obj); return ret; @@ -4061,7 +4080,12 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write) /* Flush the CPU cache if it's still invalid. */ if ((obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0) { - i915_gem_clflush_object(obj, false); + struct drm_i915_private *dev_priv = obj->base.dev->dev_private; + if (is_cpu_flush_required(obj) && + obj->base.size >= dev_priv->wbinvd_threshold) + wbinvd(); + else + i915_gem_clflush_object(obj, false); obj->base.read_domains |= I915_GEM_DOMAIN_CPU; } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 74
[Intel-gfx] [PATCH 1/6] drm/i915: Remove the useless flush_chipset
flush_chipset makes no sense with execlists because the former is for strictly prior to gen6, while the latter is for gen >= 8 Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/intel_lrc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 70e449b..e727217 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -576,7 +576,6 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf, struct intel_engine_cs *ring = ringbuf->ring; struct i915_vma *vma; uint32_t flush_domains = 0; - bool flush_chipset = false; int ret; list_for_each_entry(vma, vmas, exec_list) { @@ -587,7 +586,7 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf, return ret; if (obj->base.write_domain & I915_GEM_DOMAIN_CPU) - flush_chipset |= i915_gem_clflush_object(obj, false); + i915_gem_clflush_object(obj, false); flush_domains |= obj->base.write_domain; } -- 2.3.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915: Extract checking the necessity of flush
Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_gem.c | 53 +++-- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5bfb332..4d5a69d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -69,6 +69,35 @@ static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj) return obj->pin_display; } +static bool +is_cpu_flush_required(struct drm_i915_gem_object *obj) +{ + /* If we don't have a page list set up, then we're not pinned +* to GPU, and we can ignore the cache flush because it'll happen +* again at bind time. +*/ + if (obj->pages == NULL) + return false; + + /* +* Stolen memory is always coherent with the GPU as it is explicitly +* marked as wc by the system, or the system is cache-coherent. +*/ + if (obj->stolen || obj->phys_handle) + return false; + + /* If the GPU is snooping the contents of the CPU cache, +* we do not need to manually clear the CPU cache lines. However, +* the caches are only snooped when the render cache is +* flushed/invalidated. As we always have to emit invalidations +* and flushes when moving into and out of the RENDER domain, correct +* snooping behaviour occurs naturally as the result of our domain +* tracking. +*/ + return !cpu_cache_is_coherent(obj->base.dev, obj->cache_level); +} + + static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj) { if (obj->tiling_mode) @@ -3615,29 +3644,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force) { - /* If we don't have a page list set up, then we're not pinned -* to GPU, and we can ignore the cache flush because it'll happen -* again at bind time. -*/ - if (obj->pages == NULL) - return false; - - /* -* Stolen memory is always coherent with the GPU as it is explicitly -* marked as wc by the system, or the system is cache-coherent. -*/ - if (obj->stolen || obj->phys_handle) - return false; - - /* If the GPU is snooping the contents of the CPU cache, -* we do not need to manually clear the CPU cache lines. However, -* the caches are only snooped when the render cache is -* flushed/invalidated. As we always have to emit invalidations -* and flushes when moving into and out of the RENDER domain, correct -* snooping behaviour occurs naturally as the result of our domain -* tracking. -*/ - if (!force && cpu_cache_is_coherent(obj->base.dev, obj->cache_level)) { + if (!force && !is_cpu_flush_required(obj)) { obj->cache_dirty = true; return false; } -- 2.3.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Pass eb_vmas to execbuffer implementations
The role of eb_vmas continues to grow here as it becomes the proper encapsulation for the data passed to the various execution function. Next patch makes use of it... This patch was initially part of the next patch, but got split out after I had found a bug that convinced me the two should be separate. Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.h| 13 +++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 26 ++ drivers/gpu/drm/i915/intel_lrc.c | 8 +--- drivers/gpu/drm/i915/intel_lrc.h | 3 ++- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4179b90..90ff6aa 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1641,6 +1641,15 @@ struct i915_workarounds { u32 count; }; +struct eb_vmas { + struct list_head vmas; + int and; + union { + struct i915_vma *lut[0]; + struct hlist_head buckets[0]; + }; +}; + struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; @@ -1896,7 +1905,7 @@ struct drm_i915_private { struct intel_engine_cs *ring, struct intel_context *ctx, struct drm_i915_gem_execbuffer2 *args, - struct list_head *vmas, + struct eb_vmas *eb, struct drm_i915_gem_object *batch_obj, u64 exec_start, u32 flags); int (*init_rings)(struct drm_device *dev); @@ -2626,7 +2635,7 @@ int i915_gem_ringbuffer_submission(struct drm_device *dev, struct intel_engine_cs *ring, struct intel_context *ctx, struct drm_i915_gem_execbuffer2 *args, - struct list_head *vmas, + struct eb_vmas *eb, struct drm_i915_gem_object *batch_obj, u64 exec_start, u32 flags); int i915_gem_execbuffer(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index b773368..13ed13e 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -41,15 +41,6 @@ #define BATCH_OFFSET_BIAS (256*1024) -struct eb_vmas { - struct list_head vmas; - int and; - union { - struct i915_vma *lut[0]; - struct hlist_head buckets[0]; - }; -}; - static struct eb_vmas * eb_create(struct drm_i915_gem_execbuffer2 *args) { @@ -617,10 +608,11 @@ eb_vma_misplaced(struct i915_vma *vma) static int i915_gem_execbuffer_reserve(struct intel_engine_cs *ring, - struct list_head *vmas, + struct eb_vmas *eb, bool *need_relocs) { struct drm_i915_gem_object *obj; + struct list_head *vmas = &eb->vmas; struct i915_vma *vma; struct i915_address_space *vm; struct list_head ordered_vmas; @@ -803,7 +795,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, goto err; need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0; - ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, &need_relocs); + ret = i915_gem_execbuffer_reserve(ring, eb, &need_relocs); if (ret) goto err; @@ -829,8 +821,9 @@ err: static int i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring, - struct list_head *vmas) + struct eb_vmas *eb) { + struct list_head *vmas = &eb->vmas; struct i915_vma *vma; uint32_t flush_domains = 0; bool flush_chipset = false; @@ -1136,12 +1129,13 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file, struct intel_engine_cs *ring, struct intel_context *ctx, struct drm_i915_gem_execbuffer2 *args, - struct list_head *vmas, + struct eb_vmas *eb, struct drm_i915_gem_object *batch_obj, u64 exec_start, u32 flags) { struct drm_clip_rect *cliprects = NULL; struct drm_i915_private *dev_priv = dev->dev_private; + struct list_head *vmas = &eb->vmas; u64 exec_len; int instp_mode; u32 instp_mask; @@ -1190,7 +1184,7 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file, } } - ret = i915_gem_execbuffer_move_to_gp
[Intel-gfx] [PATCH 3/6] drm/i915: Opportunistically reduce flushing at execbuf
If we're moving a bunch of buffers from the CPU domain to the GPU domain, and we've already blown out the entire cache via a wbinvd, there is nothing more to do. With this and the previous patches, I am seeing a 3x FPS increase on a certain benchmark which uses a giant 2d array texture. Unless I missed something in the code, it should only effect non-LLC i915 platforms. I haven't yet run any numbers for other benchmarks, nor have I attempted to check if various conformance tests still pass. v2: Rewrite the patch to be i915 only Obtain whether or not we wbinvd up front. Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.h| 8 drivers/gpu/drm/i915/i915_gem.c| 11 +-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 drivers/gpu/drm/i915/intel_lrc.c | 10 -- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 90ff6aa..5d2f62d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1643,6 +1643,7 @@ struct i915_workarounds { struct eb_vmas { struct list_head vmas; + bool do_wbinvd; int and; union { struct i915_vma *lut[0]; @@ -1913,6 +1914,8 @@ struct drm_i915_private { void (*stop_ring)(struct intel_engine_cs *ring); } gt; + size_t wbinvd_threshold; + uint32_t request_uniq; /* @@ -2810,6 +2813,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); +static inline bool cpu_cache_is_coherent(struct drm_device *dev, +enum i915_cache_level level) +{ + return HAS_LLC(dev) || level != I915_CACHE_NONE; +} int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj); int __must_check i915_gem_init(struct drm_device *dev); int i915_gem_init_rings(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fc81889..5bfb332 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -61,12 +61,6 @@ static int i915_gem_shrinker_oom(struct notifier_block *nb, void *ptr); static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv); -static bool cpu_cache_is_coherent(struct drm_device *dev, - enum i915_cache_level level) -{ - return HAS_LLC(dev) || level != I915_CACHE_NONE; -} - static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj) { if (!cpu_cache_is_coherent(obj->base.dev, obj->cache_level)) @@ -4878,6 +4872,11 @@ int i915_gem_init(struct drm_device *dev) dev_priv->gt.stop_ring = intel_logical_ring_stop; } + dev_priv->wbinvd_threshold = boot_cpu_data.x86_cache_size << 10; + /* Pick a high default in the unlikely case we got nothing */ + if (!dev_priv->wbinvd_threshold) + dev_priv->wbinvd_threshold = (8 << 20); + ret = i915_gem_init_userptr(dev); if (ret) goto out_unlock; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 13ed13e..56f9268 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -50,7 +50,7 @@ eb_create(struct drm_i915_gem_execbuffer2 *args) unsigned size = args->buffer_count; size *= sizeof(struct i915_vma *); size += sizeof(struct eb_vmas); - eb = kmalloc(size, GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + eb = kzalloc(size, GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); } if (eb == NULL) { @@ -78,6 +78,7 @@ eb_reset(struct eb_vmas *eb) { if (eb->and >= 0) memset(eb->buckets, 0, (eb->and+1)*sizeof(struct hlist_head)); + eb->do_wbinvd = false; } static int @@ -154,6 +155,11 @@ eb_lookup_vmas(struct eb_vmas *eb, hlist_add_head(&vma->exec_node, &eb->buckets[handle & eb->and]); } + + if (vma->node.size >= to_i915(obj->base.dev)->wbinvd_threshold && + obj->base.write_domain & I915_GEM_DOMAIN_CPU && + !cpu_cache_is_coherent(obj->base.dev, obj->cache_level)) + eb->do_wbinvd = true; ++i; } @@ -826,7 +832,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring, struct list_head *vmas = &eb->vmas; struct i915_vma *vma; uint32_t flush_domains = 0; - bool flush_chipset = false; + bool flush_chipset = eb->do_wbinvd; int ret; list_for_each_entry(
[Intel-gfx] [PATCH 4/6] drm/i915: Add debugfs knobs for wbinvd threshold
Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_debugfs.c | 34 ++ 1 file changed, 34 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b315f01..e0fd3ba 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4316,6 +4316,39 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops, i915_cache_sharing_get, i915_cache_sharing_set, "%llu\n"); +static int i915_wbinvd_threshold_get(void *data, u64 *val) +{ + struct drm_device *dev = data; + struct drm_i915_private *dev_priv = dev->dev_private; + int ret; + + ret = mutex_lock_interruptible(&dev->struct_mutex); + if (ret) + return ret; + *val = dev_priv->wbinvd_threshold; + mutex_unlock(&dev_priv->dev->struct_mutex); + + return 0; +} + +static int i915_wbinvd_threshold_set(void *data, u64 val) +{ + struct drm_device *dev = data; + struct drm_i915_private *dev_priv = dev->dev_private; + int ret; + + ret = mutex_lock_interruptible(&dev->struct_mutex); + if (ret) + return ret; + dev_priv->wbinvd_threshold = val; + mutex_unlock(&dev_priv->dev->struct_mutex); + + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(i915_wbinvd_threshold_fops, + i915_wbinvd_threshold_get, i915_wbinvd_threshold_set, + "%llu\n"); static int i915_forcewake_open(struct inode *inode, struct file *file) { struct drm_device *dev = inode->i_private; @@ -4450,6 +4483,7 @@ static const struct i915_debugfs_files { {"i915_spr_wm_latency", &i915_spr_wm_latency_fops}, {"i915_cur_wm_latency", &i915_cur_wm_latency_fops}, {"i915_fbc_false_color", &i915_fbc_fc_fops}, + {"i915_wbinvd_threshold", &i915_wbinvd_threshold_fops}, }; void intel_display_crc_init(struct drm_device *dev) -- 2.3.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] Prefer wbinvd() where appropriate
A while back we had a workload which severely suffered from excessive clflushing. I sent out a patch series which solved this generically in DRM core, but that had the unfortunate side effect of possibly regressing non-intel platforms. I've re-spun the series to only take this shortcut when on an Intel platform. This series is untested by me since we've now worked around the problem in a different way within mesa. Jesse asked me to send this out, though I don't have time to see if it's still useful (until my mesa patches are merged, it would be useful, but afterwards, I do not know). I also didn't check that I rebased things properly. Do what you want with them... Ben Widawsky (6): drm/i915: Remove the useless flush_chipset drm/i915: Pass eb_vmas to execbuffer implementations drm/i915: Opportunistically reduce flushing at execbuf drm/i915: Add debugfs knobs for wbinvd threshold drm/i915: Extract checking the necessity of flush drm/i915: obey wbinvd threshold in more places drivers/gpu/drm/i915/i915_debugfs.c| 34 +++ drivers/gpu/drm/i915/i915_drv.h| 25 +++- drivers/gpu/drm/i915/i915_gem.c| 96 -- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 46 +++--- drivers/gpu/drm/i915/i915_gem_gtt.c| 13 +++- drivers/gpu/drm/i915/intel_lrc.c | 21 --- drivers/gpu/drm/i915/intel_lrc.h | 3 +- 7 files changed, 172 insertions(+), 66 deletions(-) -- 2.3.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Make sure to allocate mininum sizes in the DDB
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5732 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 282/283 282/283 ILK 308/319 308/319 SNB +1-3 340/346 338/346 IVB +1-2 378/384 377/384 BYT 296/296 296/296 HSW +1 421/428 422/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied *SNB igt_kms_flip_dpms-vs-vblank-race PASS(3, M22) DMESG_WARN(1, M22) SNB igt_kms_flip_dpms-vs-vblank-race-interruptible DMESG_WARN(3, M22)PASS(1, M22) DMESG_WARN(1, M22) SNB igt_kms_flip_modeset-vs-vblank-race-interruptible DMESG_WARN(1, M22)PASS(2, M22) DMESG_WARN(1, M22) SNB igt_kms_pipe_crc_basic_read-crc-pipe-A DMESG_WARN(1, M22)PASS(4, M22) PASS(1, M22) *IVB igt_gem_pwrite_pread_snooped-copy-performance PASS(2, M34) DMESG_WARN(1, M34) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(1, M34)PASS(1, M34) PASS(1, M34) IVB igt_gem_storedw_batches_loop_secure-dispatch DMESG_WARN(1, M34)PASS(1, M34) DMESG_WARN(1, M34) HSW igt_gem_storedw_loop_blt DMESG_WARN(2, M20)PASS(1, M20) PASS(1, M20) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx