Re: [Intel-gfx] [PATCH 1/3] drm/i915: remove bogus mutex_unlock from error-path
On Tue, Feb 12, 2013 at 03:36:03PM +0100, Daniel Vetter wrote: > This has been lost in the locking rework for intel_alloc_context_page: > > commit 2c34b850ee1e9f86b41706149d0954eee58757a3 > Author: Ben Widawsky > Date: Sat Mar 19 18:14:26 2011 -0700 > > drm/i915: fix ilk rc6 teardown locking > > Cc: Ben Widawsky > Signed-off-by: Daniel Vetter I'm confused about how it got lost. Reviewed-by: Ben Widawsky > --- > drivers/gpu/drm/i915/intel_pm.c |1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 3280cff..7658c39 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2286,7 +2286,6 @@ err_unpin: > i915_gem_object_unpin(ctx); > err_unref: > drm_gem_object_unreference(&ctx->base); > - mutex_unlock(&dev->struct_mutex); > return NULL; > } > > -- > 1.7.10.4 > -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix PIPE_CONTROL DW/QW write through global GTT on IVB+
On Tue, Feb 12, 2013 at 10:01:39PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > The bit controlling whether PIPE_CONTROL DW/QW write targets > the global GTT or PPGTT moved moved from DW 2 bit 2 to > DW 1 bit 24 on IVB. > > TODO: need to test on IVB actually and make sure things don't > explode. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d7542cd..69a95c6 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -308,6 +308,7 @@ > #define DISPLAY_PLANE_A (0<<20) > #define DISPLAY_PLANE_B (1<<20) > #define GFX_OP_PIPE_CONTROL(len) ((0x3<<29)|(0x3<<27)|(0x2<<24)|(len-2)) > +#define PIPE_CONTROL_GLOBAL_GTT_IVB(1<<24) /* > gen7+ */ > #define PIPE_CONTROL_CS_STALL (1<<20) > #define PIPE_CONTROL_TLB_INVALIDATE(1<<18) > #define PIPE_CONTROL_QW_WRITE (1<<14) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 9b8b058..f397bd7 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -330,8 +330,8 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, > return ret; > > intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4)); > - intel_ring_emit(ring, flags); > - intel_ring_emit(ring, scratch_addr | PIPE_CONTROL_GLOBAL_GTT); > + intel_ring_emit(ring, flags | PIPE_CONTROL_GLOBAL_GTT_IVB); > + intel_ring_emit(ring, scratch_addr); > intel_ring_emit(ring, 0); > intel_ring_advance(ring); > In the fashion of my favorite maintainer... bikeshed! Can you move the PIPE_CONTROL_GLOBAL_GTT_IVB up to where we set CS_STALL (or really anywhere else). That way we have a consistent way of setting flags, ie. dword2 -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Print the pipe control page GTT address
On Tue, Feb 12, 2013 at 10:01:38PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > We already print the HWS addresses during init, so do the same for the > pipe control page. Reduces guesswork when looking at hex addresses > later. > > Signed-off-by: Ville Syrjälä Queued for -next, thanks for the patch. I'll hold off on patch 2 until the report from the bomb squad is in ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Fix PIPE_CONTROL DW/QW write through global GTT on IVB+
From: Ville Syrjälä The bit controlling whether PIPE_CONTROL DW/QW write targets the global GTT or PPGTT moved moved from DW 2 bit 2 to DW 1 bit 24 on IVB. TODO: need to test on IVB actually and make sure things don't explode. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d7542cd..69a95c6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -308,6 +308,7 @@ #define DISPLAY_PLANE_A (0<<20) #define DISPLAY_PLANE_B (1<<20) #define GFX_OP_PIPE_CONTROL(len) ((0x3<<29)|(0x3<<27)|(0x2<<24)|(len-2)) +#define PIPE_CONTROL_GLOBAL_GTT_IVB (1<<24) /* gen7+ */ #define PIPE_CONTROL_CS_STALL(1<<20) #define PIPE_CONTROL_TLB_INVALIDATE (1<<18) #define PIPE_CONTROL_QW_WRITE(1<<14) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 9b8b058..f397bd7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -330,8 +330,8 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, return ret; intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4)); - intel_ring_emit(ring, flags); - intel_ring_emit(ring, scratch_addr | PIPE_CONTROL_GLOBAL_GTT); + intel_ring_emit(ring, flags | PIPE_CONTROL_GLOBAL_GTT_IVB); + intel_ring_emit(ring, scratch_addr); intel_ring_emit(ring, 0); intel_ring_advance(ring); -- 1.7.12.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Print the pipe control page GTT address
From: Ville Syrjälä We already print the HWS addresses during init, so do the same for the pipe control page. Reduces guesswork when looking at hex addresses later. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 00525ff..9b8b058 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -467,6 +467,9 @@ init_pipe_control(struct intel_ring_buffer *ring) if (pc->cpu_page == NULL) goto err_unpin; + DRM_DEBUG_DRIVER("%s pipe control offset: 0x%08x\n", +ring->name, pc->gtt_offset); + pc->obj = obj; ring->private = pc; return 0; -- 1.7.12.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 PCH backlight vs. Dell XPS13
On Fri, Feb 8, 2013 at 8:36 PM, Kamal Mostafa wrote: > > Daniel's backlight fixes which landed in 3.6 did enable working > > backlight controls on the XPS13, but it appears that this commit from > > Paolo broke it again: > > a4f32fc drm/i915: don't forget the PCH backlight registers On Fri, 2013-02-08 at 20:55 +0100, Daniel Vetter wrote: > You might want to try the latest drm-intel-nightly git branch from > http://cgit.freedesktop.org/~danvet/drm-intel This has a few more > tricks which helped on some similar platforsm to yours (and similar > resume issues). Yes indeed, this commit from drm-intel-nightly does fix the problem: "drm/i915: write backlight harder". Thanks as always, for your help Daniel! -Kamal signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: move LVDS support check to output setup
On Tue, Feb 12, 2013 at 07:37:08PM +0200, Jani Nikula wrote: > On Tue, 12 Feb 2013, Chris Wilson wrote: > > On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote: > >> Keep all the platform output selection in intel_output_setup(), and don't > >> scatter it around. > > > > I see this as doing the opposite. You are littering an already over > > complicated routine with LVDS specific information. > > I think it's fairly straightforward to follow what's happening there, > and with all the platform dependent stuff in the same place you don't > have to dig down from that complicated routine to look for *other* > places where there are platform dependent ifs and checks. Especially if > we need to add *more* of those deep down. I consider the rest of those to be a mess of repetious code that would have been simpler had the platform specific presence checks been pushed down. The only critical aspect of that function is the ordering and more importantly when to and when not to initialise certain outputs. That I feel is hidden in the forest of ifs. Just my 2 cents. -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: move LVDS support check to output setup
On Tue, 12 Feb 2013, Chris Wilson wrote: > On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote: >> Keep all the platform output selection in intel_output_setup(), and don't >> scatter it around. > > I see this as doing the opposite. You are littering an already over > complicated routine with LVDS specific information. I think it's fairly straightforward to follow what's happening there, and with all the platform dependent stuff in the same place you don't have to dig down from that complicated routine to look for *other* places where there are platform dependent ifs and checks. Especially if we need to add *more* of those deep down. >> As a useful side effect, do not try to enable LVDS on >> HSW or VLV. > > But you wouldn't with the old arrangement either. On HSW it would depend on I915_READ(PCH_LVDS) & LVDS_DETECTED, which I believe it shouldn't, and VBT. On mobile VLV it depends on VBT only. And I think we know better than the VBT. Unless I'm missing something and you have to spell it out for me... :/ >> Some checks are done in a slightly different order than before, and on some >> platforms VGA is now initialized before LVDS. > > Is that significant? I don't think so, but I thought I'd write it down as a note just in case it turns out to be significant. > You have not sold me on the benefits of this change. The main motivation was: http://mid.gmane.org/cakmk7ugxfyuzcksbdkqqf15lgcp9vqgkwcu1-hhtq0vmrzv...@mail.gmail.com BR, Jani. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: move LVDS support check to output setup
On Tue, Feb 12, 2013 at 07:06:59PM +0200, Jani Nikula wrote: > Keep all the platform output selection in intel_output_setup(), and don't > scatter it around. I see this as doing the opposite. You are littering an already over complicated routine with LVDS specific information. > As a useful side effect, do not try to enable LVDS on > HSW or VLV. But you wouldn't with the old arrangement either. > Some checks are done in a slightly different order than before, and on some > platforms VGA is now initialized before LVDS. Is that significant? You have not sold me on the benefits of this change. -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] drm/i915: move LVDS support check to output setup
Keep all the platform output selection in intel_output_setup(), and don't scatter it around. As a useful side effect, do not try to enable LVDS on HSW or VLV. Some checks are done in a slightly different order than before, and on some platforms VGA is now initialized before LVDS. Signed-off-by: Jani Nikula --- I took this out of the VLV series to give it the attention it needs. --- drivers/gpu/drm/i915/intel_display.c | 25 - drivers/gpu/drm/i915/intel_lvds.c| 24 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d75c6a0..a1fb320 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8198,12 +8198,19 @@ static void intel_setup_outputs(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *encoder; bool dpd_is_edp = false; - bool has_lvds; - has_lvds = intel_lvds_init(dev); - if (!has_lvds && !HAS_PCH_SPLIT(dev)) { - /* disable the panel fitter on everything but LVDS */ - I915_WRITE(PFIT_CONTROL, 0); + if (!HAS_PCH_SPLIT(dev)) { + bool has_lvds = false; + + /* Prior to PCH split LVDS was only attached to mobile products, +* except for the inglorious 830gm */ + if (IS_MOBILE(dev) && !IS_I830(dev)) + has_lvds = intel_lvds_init(dev); + + if (!has_lvds) { + /* disable the panel fitter on everything but LVDS */ + I915_WRITE(PFIT_CONTROL, 0); + } } if (!(HAS_DDI(dev) && (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES))) @@ -8230,6 +8237,14 @@ static void intel_setup_outputs(struct drm_device *dev) intel_ddi_init(dev, PORT_D); } else if (HAS_PCH_SPLIT(dev)) { int found; + + if (I915_READ(PCH_LVDS) & LVDS_DETECTED) { + if (dev_priv->edp.support) + DRM_DEBUG_KMS("disable LVDS for eDP support\n"); + else + intel_lvds_init(dev); + } + dpd_is_edp = intel_dpd_is_edp(dev); if (has_edp_a(dev)) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 7e4ec63..3c76e91 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1024,18 +1024,6 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP; } -static bool intel_lvds_supported(struct drm_device *dev) -{ - /* With the introduction of the PCH we gained a dedicated -* LVDS presence pin, use it. */ - if (HAS_PCH_SPLIT(dev)) - return true; - - /* Otherwise LVDS was only attached to mobile products, -* except for the inglorious 830gm */ - return IS_MOBILE(dev) && !IS_I830(dev); -} - /** * intel_lvds_init - setup LVDS connectors on this device * @dev: drm device @@ -1060,9 +1048,6 @@ bool intel_lvds_init(struct drm_device *dev) int pipe; u8 pin; - if (!intel_lvds_supported(dev)) - return false; - /* Skip init on machines we know falsely report LVDS */ if (dmi_check_system(intel_no_lvds)) return false; @@ -1073,15 +1058,6 @@ bool intel_lvds_init(struct drm_device *dev) return false; } - if (HAS_PCH_SPLIT(dev)) { - if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0) - return false; - if (dev_priv->edp.support) { - DRM_DEBUG_KMS("disable LVDS for eDP support\n"); - return false; - } - } - lvds_encoder = kzalloc(sizeof(struct intel_lvds_encoder), GFP_KERNEL); if (!lvds_encoder) return false; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: push struct_mutex locking down to ironlake_enable_rc6
Originally dev->struct_mutex protected all kinds of things for rps/ips (debugfs access, driver state, ...). Nowadays we have dev_priv->rps.lock for this, so only the ilk ips code needs it really. Push the locking down. I haven't yet changed the cleanup side since there we lock large parts of the code with struct_mutex. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c |2 -- drivers/gpu/drm/i915/intel_pm.c | 13 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index da1ad9c..91e99a6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8945,9 +8945,7 @@ void intel_modeset_init_hw(struct drm_device *dev) intel_init_clock_gating(dev); - mutex_lock(&dev->struct_mutex); intel_enable_gt_powersave(dev); - mutex_unlock(&dev->struct_mutex); } void intel_modeset_init(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7658c39..3ed7114 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2715,6 +2715,8 @@ void ironlake_teardown_rc6(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + WARN_ON(!mutex_is_locked(&dev->struct_mutex)); + if (dev_priv->ips.renderctx) { i915_gem_object_unpin(dev_priv->ips.renderctx); drm_gem_object_unreference(&dev_priv->ips.renderctx->base); @@ -2778,11 +2780,11 @@ static void ironlake_enable_rc6(struct drm_device *dev) if (!intel_enable_rc6(dev)) return; - WARN_ON(!mutex_is_locked(&dev->struct_mutex)); + mutex_lock(&dev->struct_mutex); ret = ironlake_setup_rc6(dev); if (ret) - return; + goto unlock; was_interruptible = dev_priv->mm.interruptible; dev_priv->mm.interruptible = false; @@ -2795,7 +2797,7 @@ static void ironlake_enable_rc6(struct drm_device *dev) if (ret) { ironlake_teardown_rc6(dev); dev_priv->mm.interruptible = was_interruptible; - return; + goto unlock; } intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN); @@ -2820,11 +2822,14 @@ static void ironlake_enable_rc6(struct drm_device *dev) if (ret) { DRM_ERROR("failed to enable ironlake power power savings\n"); ironlake_teardown_rc6(dev); - return; + goto unlock; } I915_WRITE(PWRCTXA, dev_priv->ips.pwrctx->gtt_offset | PWRCTX_EN); I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT); + +unlock: + mutex_lock(&dev->struct_mutex); } static unsigned long intel_pxfreq(u32 vidfreq) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: cancel console_resume_work before destroying the fbdev
Could lead to tears if the work indeed runs after we've destroyed the fbdev ... Totally unlikely to happen though in reality. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_dma.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 99daa89..363d2d0 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1684,9 +1684,9 @@ int i915_driver_unload(struct drm_device *dev) acpi_video_unregister(); if (drm_core_check_feature(dev, DRIVER_MODESET)) { + cancel_work_sync(&dev_priv->console_resume_work); intel_fbdev_fini(dev); intel_modeset_cleanup(dev); - cancel_work_sync(&dev_priv->console_resume_work); /* * free the memory space allocated for the child device -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: remove bogus mutex_unlock from error-path
This has been lost in the locking rework for intel_alloc_context_page: commit 2c34b850ee1e9f86b41706149d0954eee58757a3 Author: Ben Widawsky Date: Sat Mar 19 18:14:26 2011 -0700 drm/i915: fix ilk rc6 teardown locking Cc: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_pm.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3280cff..7658c39 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2286,7 +2286,6 @@ err_unpin: i915_gem_object_unpin(ctx); err_unref: drm_gem_object_unreference(&ctx->base); - mutex_unlock(&dev->struct_mutex); return NULL; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] some locking clarifications
Hi all, Three patches for little things I've recently noticed. The long-term aim behind the last one is to slowly reduce the scope of struct_mutex and clarify what's actually protected by it. Comments highly welcome. Cheers, Daniel Daniel Vetter (3): drm/i915: remove bogus mutex_unlock from error-path drm/i915: cancel console_resume_work before destroying the fbdev drm/i915: push struct_mutex locking down to ironlake_enable_rc6 drivers/gpu/drm/i915/i915_dma.c |2 +- drivers/gpu/drm/i915/intel_display.c |2 -- drivers/gpu/drm/i915/intel_pm.c | 14 +- 3 files changed, 10 insertions(+), 8 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests: Forbid to run the blit tests with count of 1
From: Damien Lespiau Invoking say, sudo ./tests/gem_render_linear_blits 1 does not make a lot of sense as we're creating a single bo. The test does not yell at you and passes, even if the rendercopy function does not do anything. This makes it quite harmful when trying to debug rendercopy without realizing that count is the number of allocated bos and must be >= 2. Signed-off-by: Damien Lespiau --- tests/gem_linear_blits.c| 4 tests/gem_render_linear_blits.c | 5 + tests/gem_render_tiled_blits.c | 5 + tests/gem_tiled_blits.c | 3 +++ 4 files changed, 17 insertions(+) diff --git a/tests/gem_linear_blits.c b/tests/gem_linear_blits.c index fe15f1d..ec2dc56 100644 --- a/tests/gem_linear_blits.c +++ b/tests/gem_linear_blits.c @@ -189,6 +189,10 @@ int main(int argc, char **argv) count = atoi(argv[1]); if (count == 0) count = 3 * gem_aperture_size(fd) / (1024*1024) / 2; + else if (count < 2) { + fprintf(stderr, "count must be >= 2\n"); + return 1; + } if (count > intel_get_total_ram_mb() * 9 / 10) { count = intel_get_total_ram_mb() * 9 / 10; diff --git a/tests/gem_render_linear_blits.c b/tests/gem_render_linear_blits.c index 8ba24a3..a7e0189 100644 --- a/tests/gem_render_linear_blits.c +++ b/tests/gem_render_linear_blits.c @@ -85,6 +85,11 @@ int main(int argc, char **argv) count = atoi(argv[1]); if (count == 0) count = 3 * gem_aperture_size(fd) / SIZE / 2; + else if (count < 2) { + fprintf(stderr, "count must be >= 2\n"); + return 1; + } + printf("Using %d 1MiB buffers\n", count); bo = malloc(sizeof(*bo)*count); diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c index 31b2ee1..626e652 100644 --- a/tests/gem_render_tiled_blits.c +++ b/tests/gem_render_tiled_blits.c @@ -88,6 +88,11 @@ int main(int argc, char **argv) count = atoi(argv[1]); if (count == 0) count = 3 * gem_aperture_size(fd) / SIZE / 2; + else if (count < 2) { + fprintf(stderr, "count must be >= 2\n"); + return 1; + } + printf("Using %d 1MiB buffers\n", count); buf = malloc(sizeof(*buf)*count); diff --git a/tests/gem_tiled_blits.c b/tests/gem_tiled_blits.c index a6d..bb43976 100644 --- a/tests/gem_tiled_blits.c +++ b/tests/gem_tiled_blits.c @@ -132,6 +132,9 @@ int main(int argc, char **argv) if (count == 0) { count = 3 * gem_aperture_size(fd) / (1024*1024) / 2; count += (count & 1) == 0; + } else if (count < 2) { + fprintf(stderr, "count must be >= 2\n"); + return 1; } if (count > intel_get_total_ram_mb() * 9 / 10) { -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl
By exporting the ability to map user address and inserting PTEs representing their backing pages into the GTT, we can exploit UMA in order to utilize normal application data as a texture source or even as a render target (depending upon the capabilities of the chipset). This has a number of uses, with zero-copy downloads to the GPU and efficient readback making the intermixed streaming of CPU and GPU operations fairly efficient. This ability has many widespread implications from faster rendering of client-side software rasterisers (chromium), mitigation of stalls due to read back (firefox) and to faster pipelining of texture data (such as pixel buffer objects in GL or data blobs in CL). v2: Compile with CONFIG_MMU_NOTIFIER v3: We can sleep while performing invalidate-range, which we can utilise to drop our page references prior to the kernel manipulating the vma (for either discard or cloning) and so protect normal users. v4: Only run the invalidate notifier if the range intercepts the bo. v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_dma.c|1 + drivers/gpu/drm/i915/i915_drv.h| 22 ++ drivers/gpu/drm/i915/i915_gem.c| 31 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c |7 +- drivers/gpu/drm/i915/i915_gem_userptr.c| 329 include/uapi/drm/i915_drm.h| 16 ++ 7 files changed, 393 insertions(+), 14 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91f3ac6..42858f6 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_gem_userptr.o \ i915_sysfs.o \ i915_trace_points.o \ i915_ums.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 4fa6beb..9b1984c 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1883,6 +1883,7 @@ struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 923dc0a..90070f4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -42,6 +42,7 @@ #include #include #include +#include #include /* General customization: @@ -1076,6 +1077,7 @@ struct drm_i915_gem_object_ops { */ int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *); + void (*release)(struct drm_i915_gem_object *); }; struct drm_i915_gem_object { @@ -1222,6 +1224,23 @@ struct drm_i915_gem_object { }; #define to_gem_object(obj) (&((struct drm_i915_gem_object *)(obj))->base) +struct i915_gem_userptr_object { + struct drm_i915_gem_object gem; + uintptr_t user_ptr; + size_t user_size; + int read_only; + + struct mm_struct *mm; +#if defined(CONFIG_MMU_NOTIFIER) + struct mmu_notifier mn; +#endif +}; + +union drm_i915_gem_objects { + struct drm_i915_gem_object base; + struct i915_gem_userptr_object userptr; +}; + #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) /** @@ -1501,6 +1520,8 @@ int i915_gem_entervt_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); int i915_gem_set_tiling(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling(struct drm_device *dev, void *data, @@ -1554,6 +1575,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) BUG_ON(obj->pages_pin_count == 0); obj->pages_pin_count--; } +int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); int i915_gem_object_sync(struct drm_i915_gem_object *obj, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 73b1e9e..65a36bf 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/
Re: [Intel-gfx] [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds
On Fri, Feb 08, 2013 at 04:35:38PM +0200, Mika Kuoppala wrote: > commit c1d1f5aeda2033d96e872f416388653f05d4c16d > Author: Mika Kuoppala > Date: Tue Feb 5 17:26:52 2013 +0200 > > drm/i915: disable shared panel fitter for pipe > > moved panel fit disabling to be crtc property. Thus > the need to explicitly turn pfit off in encoder side > became obsolete. Take advantage of that and get rid of > pfit_dirty state handling in lvds. > > Signed-off-by: Mika Kuoppala Merged both patches, with the bit of bikeshed applied as discussed on irc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: unify HDMI/DP hpd definitions
On Thu, Feb 07, 2013 at 11:15:26AM -0200, Paulo Zanoni wrote: > 2013/2/7 Daniel Vetter : > > They're physically the same pins and also the same bits, duplicating > > only confuses the reader. This also makes it a bit obvious that we > > have quite some code duplication going on here. Squashing that is for > > a larger rework in our hpd handling though. > > > > Signed-off-by: Daniel Vetter > > Reviewed-by: Paulo Zanoni Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: detect wrong MCH watermark values
On Sat, Feb 09, 2013 at 12:24:02PM -0800, Ben Widawsky wrote: > On Sat, Feb 09, 2013 at 09:03:42PM +0100, Daniel Vetter wrote: > > Some early bios versions seem to ship with the wrong tuning values for > > the MCH, possible resulting in pipe underruns under load. Especially > > on DP outputs this can lead to black screen, since DP really doesn't > > like an occasional whack from an underrun. > > > > Unfortunately the registers seem to be locked after boot, so the only > > thing we can do is politely point out issues and suggest a BIOS > > upgrade. > > > > Arthur Runyan pointed us at this issue while discussion DP bugs - thus > > far no confirmation from a bug report yet that it helps. But at least > > some of my machines here have wrong values, so this might be useful in > > understanding bug reports. > > > > v2: After a bit more discussion with Art and Ben we've decided to only > > the check the watermark values, since the OREF ones could be be a > > notch more aggressive on certain machines. > > > > Cc: Ben Widawsky > > Cc: Runyan, Arthur J > > Signed-off-by: Daniel Vetter > > I wouldn't mind if you printed the OREF values, even if you didn't use > them to detect. They're different already on my machine than the most conservative recommended values, so I've thought it'll be of little value to us. If the hw guys need them, we can always ask reporters for it. > But, > Reviewed-by: Ben Widawsky Thanks for the review, patch merged. -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 v2] drm/i915/ctx: Remove bad invariant
On Mon, Feb 11, 2013 at 01:31:27PM -0800, Ben Widawsky wrote: > It's not that the assertion is incorrect, but rather that we can call > do_destroy early in loading, and we will falsely BUG(). > > Since contexts have been in for a while now, and in the internal APIs > are pretty stable, it should be fairly safe to remove this. > > v2: Remove unused dev_priv, and dev > > Signed-off-by: Ben Widawsky Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx