Re: [Intel-gfx] [PATCH] drm/i915/dp: make link rate printing prettier
On Tue, 19 May 2015, David Weinehall wrote: > On Mon, May 18, 2015 at 04:01:45PM +0300, Jani Nikula wrote: >> Turn >> >> [drm:intel_dp_print_rates] source rates: 162000,27,54, >> [drm:intel_dp_print_rates] sink rates: 162000,27, >> [drm:intel_dp_print_rates] common rates: 162000,27, >> >> into >> >> [drm:intel_dp_print_rates] source rates: 162000, 27, 54 >> [drm:intel_dp_print_rates] sink rates: 162000, 27 >> [drm:intel_dp_print_rates] common rates: 162000, 27 > > Wouldn't aligning the sink rates with the other two rates look nicer > (either by right-aligning the entire text, or by just right-aligning > the colon? > > Admittedly this is just bikeshedding, but... And clearly a separate patch, which is most welcome! ;) BR, Jani. > > > Kind regards, David -- 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] Documentation/drm: Update rotation property with 90/270 and description
On 5/13/2015 9:57 AM, Jindal, Sonika wrote:
On 5/12/2015 6:20 PM, Ville Syrjälä wrote:
On Wed, Apr 15, 2015 at 04:05:19PM +0530, Sonika Jindal wrote:
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Sonika Jindal
---
Documentation/DocBook/drm.tmpl |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl
b/Documentation/DocBook/drm.tmpl
index f4976cd..266d50a 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev)
Plane
“rotation”
BITMASK
-{ 0, "rotate-0" }, { 2, "rotate-180" }
+{ 0, "rotate-0" }, { 1, "rotate-90" },
+{ 2, "rotate-180" }, { 3, "rotate-270" }
Plane
-TBD
+To set plane HW rotation. This rotation
property does
+the plane rotation in counter clockwise direction which is
+inline with the way XRandr works.
I would suggest moving the thing to the generci props section since we
have omap and i915 both supporting it.
You mean in DRM properties section?
Right now, OMAP section also has rotation property. I will remove it
from OMAP section as well if you think drm is the better place.
As for the description, we should also document the reflect flags.
Also, i915 doesn't support reflect flags. Only rotation is supported there.
For the "generic" part, you meant just moving to the generic group in
i915 property section?
I might write it as something like this:
"rotate-0,rotate-90,rotate-180,rotate-270 rotate the image by the
specified amount in degrees in a counter clockwise direction.
reflect-x,reflect-y reflect the image along the specified axis,
prior to rotation."
SDVO-TV
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Update comment in clear_intel_crtc_state()
Explain why a few fields of the new pipe_config have their values preserved, while the others are zeroed. Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_display.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a7732b4..b0cd649 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11460,7 +11460,11 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state) enum intel_dpll_id shared_dpll; uint32_t ddi_pll_sel; - /* Clear only the intel specific part of the crtc state excluding scalers */ + /* FIXME: before the switch to atomic started, a new pipe_config was +* kzalloc'd. Code that depends on any field being zero should be +* fixed, so that the crtc_state can be safely duplicated. For now, +* only fields that are know to not cause problems are preserved. */ + tmp_state = crtc_state->base; scaler_state = crtc_state->scaler_state; shared_dpll = crtc_state->shared_dpll; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: don't fail colorkey + scaler request
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6431 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 234/234 234/234 ILK 262/262 262/262 SNB -1 282/282 281/282 IVB 300/300 300/300 BYT 254/254 254/254 BDW -1 275/275 274/275 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(7)PASS(1) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x *BDW igt@gem_gtt_hog PASS(2) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane assertion_failure@assertion failure WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/11] Skylake display NV12 feature addition
> -Original Message- > From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] > Sent: Monday, May 18, 2015 3:43 AM > To: Konduru, Chandra; Vetter, Daniel; Lespiau, Damien; Syrjala, Ville > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 00/11] Skylake display NV12 feature addition > > > Hi, > > On 05/18/2015 06:24 AM, Konduru, Chandra wrote: > >> -Original Message- > >> From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] > >> Sent: Thursday, May 14, 2015 5:47 AM > >> To: Konduru, Chandra; Vetter, Daniel; Lespiau, Damien; Syrjala, Ville > >> Cc: intel-gfx@lists.freedesktop.org > >> Subject: Re: [Intel-gfx] [PATCH 00/11] Skylake display NV12 feature > >> addition > >> > >> > >> On 05/14/2015 06:13 AM, Konduru, Chandra wrote: > >>> Hi, > >>> I have seen review comments from you and addressed/responded to them. > >>> Can you please give R-b tag? > >> > >> What about that WARN_ON(fb->pixel_format == DRM_FORMAT_NV12) I am > >> hitting (and subsequent hard hang)? Were you able to repro that? > >> > >> I did rebase your series on latest nightly but it was very trivial if > >> anything so I don't think I did something wrong there. > > > > I was able to reproduce the issue and found the root-cause. > > The required steps to reproduce this issue requires NV12 as primary > > plane format via drmModeCrtcSetConfig() and the way you modified > > kms_rotation_crc is just doing that. In crtc set config flow the way > > atomic commit planes called without atomic commit call. So, it is missing > setupscalers required for NV12. > > And the WARN_ON I added is catching this scenario. > > > > Though this may be not common, it requires proper handling in i915. > > I just send combined NV12 patch series for 0/180 and 90/270 including > > addressing this issue. > > Can you please check at your end with updated series? > > > > By the way, you need latest drm-intel-nightly, then apply your GEM > > remapping for NV12 patches (2 to 7 of 8, 8th not required. And 1st one > > is already merged), then apply my series (12 of 12). > > Warn is gone but my box still locks up hard. Good. > > I can even reproduce the lockup by running nv12-plane-linear subtest from > kms_nv12, where it happens perhaps on the second run. May be more easily > triggerable with drm.debug=0 - but I am not so confident about that. I have been testing on two boards, and on one board (A step), issue never happens i.e., no lockups. On the other board (C step), issue happens. I tried chicken bit workarounds for C, but still seeing issue. Debugging what is causing lockup. On another note, when I tested earlier with 400rc3 kernel, I didn't see these lockups but after moving to 410rc2 started seeing them. So combination of NV12 and whatever happened in kernel upgrade is causing lockup (at least that' how it appears now). And this only happens on board (C) not on (A). > > And also with nv12-plane-linear I see FIFO underruns so perhaps wm > programming is not fully OK for NV12? Looking at wm programming to rule out that is the case. > > Regards, > > Tvrtko ___ 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: Add 90/270 rotation for NV12 format.
> -Original Message-
> From: Runyan, Arthur J
> Sent: Monday, May 18, 2015 12:19 PM
> To: Konduru, Chandra; Ville Syrjälä
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville; Jindal,
> Sonika
> Subject: RE: [Intel-gfx] [PATCH 2/2] drm/i915: Add 90/270 rotation for NV12
> format.
>
> The statement is correct - " the X offset must always be even for YUV422+NV12,
> and the Y offset must be even when rotated 90/270 degrees."
Thanks Art.
Then below code to take care evenness for both X and Y offsets when YUV 90/270
is ok.
Ville, with this, can you give R-b tag on the ones you reviewed?
>
> >From: Konduru, Chandra
> >> From: Runyan, Arthur J
> >>
> >> I'll take a look.
> >
> >Art, Any update to close on this?
> >
> >[snip]
> >
> >> > > > @@ -13144,6 +13149,10 @@ intel_check_primary_plane(struct
> >> > > > drm_plane
> >> > > *plane,
> >> > > > if (fb && format_is_yuv(fb->pixel_format)) {
> >> > > > src->x1 &= ~0x1;
> >> > > > src->x2 &= ~0x1;
> >> > > > +if (intel_rotation_90_or_270(state->base.rotation)) {
> >> > > > +src->y1 &= ~0x1;
> >> > > > +src->y2 &= ~0x1;
> >> > > > +}
> >> > >
> >> > > This feels fishy. Why do we need to make the Y coordinates even? The
> >> > > reson for making the X coordinates even is to make them macropixel
> >> > > aligned, but there are no macropixels in the Y direction so this
> >> > > doesn't make much sense to me.
> >> >
> >> > Hi Ville,
> >> > Per skl spec, it is expecting even lines aligned with 90/270 rotation
> >> > not only for NV12 but also for 422 formats. Perhaps we might have
> >> > missed when 90/270 enabled for packed YUV formats.
> >>
> >> The src coordinates are always in the fb orientation, so macropixels
> >> appear in
> >> the src.x direction only. And when we do 90/270 rotation the hardware Y
> offset
> >> comes from src.x coordinates.
> >>
> >> The spec does seem a bit confused though; It claims the X offset must
> >> always
> be
> >> even for YUV422+NV12, and the Y offset must be even when rotated 90/270
> >> degrees. I suspect the X offset text just didn't get updated when 90/270
> rotation
> >> was added.
> >>
> >> Art, can you confirm?
> >>
> >> --
> >> 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 8/8] drm/i915/skl: Deinit/init the display at suspend/resume
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6430 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 234/234 234/234 ILK 262/262 262/262 SNB -1 282/282 281/282 IVB 300/300 300/300 BYT 256/256 256/256 BDW 275/275 275/275 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(7)PASS(1) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: fix RC6 residency time calculation
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6429 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 234/234 234/234 ILK 262/262 262/262 SNB -1 282/282 281/282 IVB 300/300 300/300 BYT 254/254 254/254 BDW 275/275 275/275 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(7)PASS(1) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add HAS_DP_MST feature test macro
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6428 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 234/234 234/234 ILK 262/262 262/262 SNB -1 282/282 281/282 IVB 300/300 300/300 BYT 254/254 254/254 BDW 275/275 275/275 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(6)PASS(1) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x 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
[Intel-gfx] [PATCH 3/3] drm/i915: Enable GTT caching on gen8
From: Ville Syrjälä
GTT caching was disabled by default on gen8 due to not working with
big pages. Some information suggests that it got fixed, but still
GTT caching has been left disabled by default. Or could be it just
meant that the default was changed to off, and hence the problem
got solved.
Enable GTT caching in the hopes of some performance increase.
Whether or not the big pages issue has been fixed is irrelevant
at this stage since we don't use big pages.
This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
BDW, but supposedly it has larger TLBs so might not benefit as much.
On HSW GTT caching is enabled by default.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 13 +
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 84af255..90640d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
#define RING_HWS_PGA(base) ((base)+0x80)
#define RING_HWS_PGA_GEN6(base)((base)+0x2080)
+#define HSW_GTT_CACHE_EN 0x4024
+#define GTT_CACHE_EN_ALL 0xF0007FFF
#define GEN7_WR_WATERMARK 0x4028
#define GEN7_GFX_PRIO_CTRL 0x402C
#define ARB_MODE 0x4030
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5ec56b6..58517a50 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct
drm_device *dev)
I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+ /*
+* WaGttCachingOffByDefault:bdw
+* GTT cache may not work with big pages, so if those
+* are ever enabled GTT cache may need to be disabled.
+*/
+ I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
+
lpt_init_clock_gating(dev);
}
@@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct
drm_device *dev)
/* WaDisableSDEUnitClockGating:chv */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
+ /*
+* GTT cache may not work with big pages, so if those
+* are ever enabled GTT cache may need to be disabled.
+*/
+ I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
}
static void g4x_init_clock_gating(struct drm_device *dev)
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Use ilk_init_lp_watermarks() on BDW
From: Ville Syrjälä We're not using ilk_init_lp_watermarks() on BDW for some reason. Probably due to the BDW patches and the relevant WM patches landing roughlly at the same time. Fix it up. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ce1d079..206bd41 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6166,9 +6166,7 @@ static void broadwell_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; enum pipe pipe; - I915_WRITE(WM3_LP_ILK, 0); - I915_WRITE(WM2_LP_ILK, 0); - I915_WRITE(WM1_LP_ILK, 0); + ilk_init_lp_watermarks(dev); /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Move WaProgramL3SqcReg1Default:bdw to init_clock_gating()
From: Ville Syrjälä
GEN8_L3SQCREG1 isn't saved in the context (verified by going through
a context dump), and so we shouldn't be using the ring w/a code to
initialize it. Also Bspec explicitly talks about MMIO and writing it
with the CPU.
Additionally there's another w/a WaTempDisableDOPClkGating:bdw which
tells us to disable DOP clock gating around the GEN8_L3SQCREG1 write
to make sure everyone notices the change. So let's do that as well.
Cc: Rodrigo Vivi
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 10 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 206bd41..5ec56b6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6165,6 +6165,7 @@ static void broadwell_init_clock_gating(struct drm_device
*dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe;
+ uint32_t misccpctl;
ilk_init_lp_watermarks(dev);
@@ -6195,6 +6196,15 @@ static void broadwell_init_clock_gating(struct
drm_device *dev)
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+ /*
+* WaProgramL3SqcReg1Default:bdw
+* WaTempDisableDOPClkGating:bdw
+*/
+ misccpctl = I915_READ(GEN7_MISCCPCTL);
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
+ I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+
lpt_init_clock_gating(dev);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9b96ed7..50cdd67 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -853,9 +853,6 @@ static int bdw_init_workarounds(struct intel_engine_cs
*ring)
GEN6_WIZ_HASHING_MASK,
GEN6_WIZ_HASHING_16x4);
- /* WaProgramL3SqcReg1Default:bdw */
- WA_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
-
return 0;
}
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/atomic: add commit_planes_on_crtc helper
On Tue, May 19, 2015 at 04:41:01PM +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_commit_planes calls all atomic_begin's first,
> then updates all planes, finally calling atomic_flush.
>
> Some drivers may want to things like disabling irq's
> from their atomic_begin, in which case a second call to atomic_begin
> will splat. By using commit_planes_on_crtc on each crtc in the
> atomic state they'll evade that issue.
>
> Signed-off-by: Maarten Lankhorst
All three patches merged to topic/drm-misc, thanks.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 53
> +
> include/drm/drm_atomic_helper.h | 1 +
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index b82ef6262469..2da8a16bb39e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1181,6 +1181,59 @@ void drm_atomic_helper_commit_planes(struct drm_device
> *dev,
> EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
>
> /**
> + * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a crtc
> + * @old_crtc_state: atomic state object with the old crtc state
> + *
> + * This function commits the new plane state using the plane and atomic
> helper
> + * functions for planes on the specific crtc. It assumes that the atomic
> state
> + * has already been pushed into the relevant object state pointers, since
> this
> + * step can no longer fail.
> + *
> + * This function can only be used when planes are not allowed to move between
> + * different crtc's.
> + */
> +void
> +drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state
> *old_crtc_state)
> +{
> + const struct drm_crtc_helper_funcs *crtc_funcs;
> + struct drm_crtc *crtc = old_crtc_state->crtc;
> + struct drm_atomic_state *old_state = old_crtc_state->state;
> + struct drm_plane *plane;
> + unsigned plane_mask;
> +
> + plane_mask = old_crtc_state->plane_mask;
> + plane_mask |= crtc->state->plane_mask;
> +
> + crtc_funcs = crtc->helper_private;
> + if (crtc_funcs && crtc_funcs->atomic_begin)
> + crtc_funcs->atomic_begin(crtc);
> +
> + drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
> + struct drm_plane_state *old_plane_state =
> + drm_atomic_get_existing_plane_state(old_state, plane);
> + const struct drm_plane_helper_funcs *plane_funcs;
> +
> + plane_funcs = plane->helper_private;
> +
> + if (!old_plane_state || !plane_funcs)
> + continue;
> +
> + WARN_ON(plane->state->crtc && plane->state->crtc != crtc);
> +
> + if (drm_atomic_plane_disabling(plane, old_plane_state) &&
> + plane_funcs->atomic_disable)
> + plane_funcs->atomic_disable(plane, old_plane_state);
> + else if (plane->state->crtc ||
> + drm_atomic_plane_disabling(plane, old_plane_state))
> + plane_funcs->atomic_update(plane, old_plane_state);
> + }
> +
> + if (crtc_funcs->atomic_flush)
> + crtc_funcs->atomic_flush(crtc);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
> +
> +/**
> * drm_atomic_helper_cleanup_planes - cleanup plane resources after commit
> * @dev: DRM device
> * @old_state: atomic state object with old state structures
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 6ee0ee5b6143..cc1fee8a12d0 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -58,6 +58,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
>struct drm_atomic_state *state);
> void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
> struct drm_atomic_state *old_state);
> +void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state
> *old_crtc_state);
>
> void drm_atomic_helper_swap_state(struct drm_device *dev,
> struct drm_atomic_state *state);
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4 v3] drm/i915: Tighten the exposure ARGB/ABGR 8888 formats
On Tue, May 19, 2015 at 12:29:16PM +0100, Damien Lespiau wrote:
> ARGB is used for cursors on all platforms so we need to allow it
> everywhere.
>
> ABGR is currently only honoured:
> - on VLV/CHV in sprite planes
> - on SKL+ for primary and sprite planes
> so only allow it for those platforms.
>
> Note that we only support ARGB/ABGR on the primary plane for
> SKL/BXT because we have in line of sight the pipe bottom color on those
> platforms and because the primary plane programming on VLV/CHV doesn't
> anything different for those formats today.
>
> v2: Fix the logic to forbid the creation ABGR2101010 fbs (Ville)
> v3: Still allow the creation of ARGB fbs now that cursor planes use
> real fb objects (found by PRTS).
>
> Reviewed-by: Ville Syrjälä
> Signed-off-by: Damien Lespiau
Let's retry. Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 33 +++--
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8a3d936..9d2d6fb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -51,7 +51,6 @@ static const uint32_t i8xx_primary_formats[] = {
> DRM_FORMAT_RGB565,
> DRM_FORMAT_XRGB1555,
> DRM_FORMAT_XRGB,
> - DRM_FORMAT_ARGB,
> };
>
> /* Primary plane formats for gen >= 4 */
> @@ -60,6 +59,15 @@ static const uint32_t i965_primary_formats[] = {
> DRM_FORMAT_RGB565,
> DRM_FORMAT_XRGB,
> DRM_FORMAT_XBGR,
> + DRM_FORMAT_XRGB2101010,
> + DRM_FORMAT_XBGR2101010,
> +};
> +
> +static const uint32_t skl_primary_formats[] = {
> + DRM_FORMAT_C8,
> + DRM_FORMAT_RGB565,
> + DRM_FORMAT_XRGB,
> + DRM_FORMAT_XBGR,
> DRM_FORMAT_ARGB,
> DRM_FORMAT_ABGR,
> DRM_FORMAT_XRGB2101010,
> @@ -2704,11 +2712,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> dspcntr |= DISPPLANE_BGRX565;
> break;
> case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> dspcntr |= DISPPLANE_BGRX888;
> break;
> case DRM_FORMAT_XBGR:
> - case DRM_FORMAT_ABGR:
> dspcntr |= DISPPLANE_RGBX888;
> break;
> case DRM_FORMAT_XRGB2101010:
> @@ -2810,11 +2816,9 @@ static void ironlake_update_primary_plane(struct
> drm_crtc *crtc,
> dspcntr |= DISPPLANE_BGRX565;
> break;
> case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> dspcntr |= DISPPLANE_BGRX888;
> break;
> case DRM_FORMAT_XBGR:
> - case DRM_FORMAT_ABGR:
> dspcntr |= DISPPLANE_RGBX888;
> break;
> case DRM_FORMAT_XRGB2101010:
> @@ -13293,12 +13297,15 @@ static struct drm_plane
> *intel_primary_plane_create(struct drm_device *dev,
> if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
> primary->plane = !pipe;
>
> - if (INTEL_INFO(dev)->gen <= 3) {
> - intel_primary_formats = i8xx_primary_formats;
> - num_formats = ARRAY_SIZE(i8xx_primary_formats);
> - } else {
> + if (INTEL_INFO(dev)->gen >= 9) {
> + intel_primary_formats = skl_primary_formats;
> + num_formats = ARRAY_SIZE(skl_primary_formats);
> + } else if (INTEL_INFO(dev)->gen >= 4) {
> intel_primary_formats = i965_primary_formats;
> num_formats = ARRAY_SIZE(i965_primary_formats);
> + } else {
> + intel_primary_formats = i8xx_primary_formats;
> + num_formats = ARRAY_SIZE(i8xx_primary_formats);
> }
>
> drm_universal_plane_init(dev, &primary->base, 0,
> @@ -13985,8 +13992,14 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> return -EINVAL;
> }
> break;
> - case DRM_FORMAT_XBGR:
> case DRM_FORMAT_ABGR:
> + if (!IS_VALLEYVIEW(dev) && INTEL_INFO(dev)->gen < 9) {
> + DRM_DEBUG("unsupported pixel format: %s\n",
> + drm_get_format_name(mode_cmd->pixel_format));
> + return -EINVAL;
> + }
> + break;
> + case DRM_FORMAT_XBGR:
> case DRM_FORMAT_XRGB2101010:
> case DRM_FORMAT_XBGR2101010:
> if (INTEL_INFO(dev)->gen < 4) {
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/12] drm/i915: Add NV12 support to intel_framebuffer_init
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Tuesday, May 19, 2015 1:24 AM
> To: Konduru, Chandra
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville
> Subject: Re: [Intel-gfx] [PATCH 08/12] drm/i915: Add NV12 support to
> intel_framebuffer_init
>
> On Sun, May 17, 2015 at 10:11:01PM -0700, Chandra Konduru wrote:
> > This patch adds NV12 as supported format to
> > intel_framebuffer_init and performs various checks.
> >
> > Signed-off-by: Chandra Konduru
> > Testcase: igt/kms_nv12
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 27 +++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> > index 42924a6..41cd26f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14043,6 +14043,33 @@ static int intel_framebuffer_init(struct
> drm_device *dev,
> > return -EINVAL;
> > }
> > break;
> > + case DRM_FORMAT_NV12:
> > + if (INTEL_INFO(dev)->gen < 9) {
> > + DRM_DEBUG("unsupported pixel format: %s\n",
> > + drm_get_format_name(mode_cmd-
> >pixel_format));
> > + return -EINVAL;
> > + }
> > + if (!mode_cmd->offsets[1]) {
> > + DRM_DEBUG("uv start offset not set\n");
> > + return -EINVAL;
> > + }
>
> Nope. It's perfectly ok to have NV12 with a 0 offset for the uv plane, if
> it's e.g. in a separate buffer object. Which is the part this series seems
> to be completely missing - there's no code at all to look up (and store in
> intel_framebuffer the 2nd i915_bo pointer) the 2nd buffer handle afaics.
>
> You should also change your igts to use 2 separate buffers, just for test
> coverage.
> -Daniel
Hi Daniel,
Agree, in general that is very well ok. But as skl hw requires uv to be after
y in gtt. This can be guaranteed by having a single bo and y and uv offsets
into it. Above sanity checks in i915 specific fb init call are for that reason.
There are definitely ways to guarantee uv to be after y even with two
separate bos (by uv remapping), but I see that is unnecessary
complication and not sure value by allowing that. Or am I missing
something here?
>
> > + if (mode_cmd->pitches[0] != mode_cmd->pitches[1] ||
> > + mode_cmd->handles[0] != mode_cmd->handles[1]) {
> > + DRM_DEBUG("y and uv subplanes have different
> parameters\n");
> > + return -EINVAL;
> > + }
> > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED
> &&
> > + (mode_cmd->offsets[1] & 0xFFF)) {
> > + DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on
> new tile-row\n",
> > + mode_cmd->offsets[1]);
> > + return -EINVAL;
> > + }
> > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED
> &&
> > + ((mode_cmd->offsets[1] % mode_cmd->pitches[1]) %
> 4)) {
> > + DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line
> aligned\n",
> > + mode_cmd->offsets[1]);
> > + }
> > + break;
> > default:
> > DRM_DEBUG("unsupported pixel format: %s\n",
> > drm_get_format_name(mode_cmd->pixel_format));
> > --
> > 1.7.9.5
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
From: Jim Bride According to the HSW b-spec we need to try clock divisors of 63 and 72, each 3 or more times, when attempting DP AUX channel communication on a server chipset. This actually wasn't happening due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit in status rather than checking that the operation was done and that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set. Signed-off-by: Jim Bride --- drivers/gpu/drm/i915/intel_dp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0edc305..c01a3f9 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, if (status & DP_AUX_CH_CTL_DONE) break; } - if (status & DP_AUX_CH_CTL_DONE) + if ((status & DP_AUX_CH_CTL_DONE) && + !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR)) break; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: make link rate printing prettier
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6427 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 234/234 234/234 ILK 262/262 262/262 SNB -1 282/282 281/282 IVB 300/300 300/300 BYT 254/254 254/254 BDW -1 275/275 274/275 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(6)PASS(1) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x *BDW igt@gem_flink@basic PASS(1) DMESG_WARN(1) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane assertion_failure@assertion failure WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU
On Fri, May 15, 2015 at 02:07:29AM +0100, Robert Bragg wrote: > On Fri, May 8, 2015 at 5:24 PM, Peter Zijlstra wrote: > > On Thu, May 07, 2015 at 03:15:43PM +0100, Robert Bragg wrote: > > > >> I've changed the uapi for configuring the i915_oa specific attributes > >> when calling perf_event_open(2) whereby instead of cramming lots of > >> bitfields into the perf_event_attr config members, I'm now > >> daisy-chaining a drm_i915_oa_event_attr_t structure off of a single > >> config member that's extensible and validated in the same way as the > >> perf_event_attr struct. I've found this much nicer to work with while > >> being neatly extensible too. > > > > This worries me a bit.. is there more background for this? > > Would it maybe be helpful to see the before and after? I had kept this > uapi change in a separate patch for a while locally but in the end > decided to squash it before sending out my updated series. > > Although I did find it a bit awkward with the bitfields, I was mainly > concerned about the extensibility of packing logically separate > attributes into the config members and had heard similar concerns from > a few others who had been experimenting with my patches too. > > A few simple attributes I can think of a.t.m that we might want to add > in the future are: > - control of the OABUFFER size > - a way to ask the kernel to collect reports at the beginning and end > of batch buffers, in addition to periodic reports > - alternative ways to uniquely identify a context to support tools > profiling a single context not necessarily owned by the current > process > > It could also be interesting to expose some counter configuration > through these attributes too. E.g. on Broadwell+ we have 14 'Flexible > EU' counters included in the OA unit reports, each with a 16bit > configuration. > > In a more extreme case it might also be useful to allow userspace to > specify a complete counter config, which (depending on the > configuration) could be over 100 32bit values to select the counter > signals + configure the corresponding combining logic. > > Since this pmu is in a device driver it also seemed reasonably > appropriate to de-couple it slightly from the core perf_event_attr > structure by allowing driver extensible attributes. > > I wonder if it might be less worrisome if the i915_oa_copy_attr() code > were instead a re-usable utility perhaps maintained in events/core.c, > so if other pmu drivers were to follow suite there would be less risk > of a mistake being made here? So I had a peek at: https://01.org/sites/default/files/documentation/observability_performance_counters_haswell.pdf In an attempt to inform myself of how the hardware worked. But the document is rather sparse (and does not include broadwell). So from what I can gather there's two ways to observe the counters, through MMIO or trough the ring-buffer, which in turn seems triggered by a MI_REPORT_PERF_COUNT command. [ Now the document talks about shortcomings of this scheme, where the MI_REPORT_PERF_COUNT command can only be placed every other command, but a single command can contain so much computation that this is not fine grained enough -- leading to the suggestion of a timer/cycle based reporting, but that is not actually mentioned afaict ] Now the MI_REPORT_PERF_COUNT can select a vector (Counter Select) of which events it will write out. This covers the regular 'A' counters. Is this correct? Then there are the 'B' counters, which appear to be programmable through the CEC MMIO registers. These B events can also be part of the MI_REPORT_PERF_COUNT vector. Right? So for me the 'natural' way to represent this in perf would be through event groups. Create a perf event for every single event -- yes this is 53 events. Use the MMIO reads for the regular read() interface, and use a hrtimer placing MI_REPORT_PERF_COUNT commands, with a counter select mask covering the all events in the current group, for sampling. Then obtain the vector values using PERF_SAMPLE_READ and PERF_FORMAT_READ -- and use the read txn support to consume the ring-buffer vectors instead of the MMIO registers. You can use the perf_event_attr::config to select the counter (A0-A44, B0-B7) and use perf_event_attr::config1 (low and high dword) for the corresponding CEC registers. This does not require random per driver ABI extentions for perf_event_attr, nor your custom output format. Am I missing something obvious here? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/bxt: limit WaDisableMaskBasedCammingInRCC to stepping A
On Tue, May 19, 2015 at 05:52:27PM +0300, Imre Deak wrote: > On ti, 2015-05-19 at 15:46 +0100, Damien Lespiau wrote: > > On Tue, May 19, 2015 at 03:39:25PM +0100, Damien Lespiau wrote: > > > On Tue, May 19, 2015 at 03:04:59PM +0300, Imre Deak wrote: > > > > Also make the WA comment consistent with the rest, where the stepping > > > > info is not shown. > > > > > > > > Signed-off-by: Imre Deak > > > > --- > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++-- > > > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > > > > > [ The patchset is a follow-up to: > > > > http://lists.freedesktop.org/archives/intel-gfx/2015-May/065989.html ] > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > index 9b96ed7..461b9be 100644 > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > @@ -961,12 +961,9 @@ static int gen9_init_workarounds(struct > > > > intel_engine_cs *ring) > > > > WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, > > > > GEN9_CCS_TLB_PREFETCH_ENABLE); > > > > > > > > - /* > > > > -* FIXME: don't apply the following on BXT for stepping C. On > > > > BXT A0 > > > > -* the flag reads back as 0. > > > > -*/ > > > > - /* WaDisableMaskBasedCammingInRCC:sklC,bxtA */ > > > > - if (INTEL_REVID(dev) == SKL_REVID_C0 || IS_BROXTON(dev)) > > > > + /* WaDisableMaskBasedCammingInRCC:skl,bxt */ > > > > > > For the record, there seem to be some confusion in the W/A db: > > > > > > - The W/A seems to have been renamed to > > > WaDisablePixelMaskBasedCammingInRcpbe > > > and indeed there's a bit to do that, but the bug in question talks > > > about bit 14 of 7308, which is the disabling bit for the RCC unit > > > - The W/A isn't listed in the W/A db for BXT > > > > > > In doubt, defaulting to trusting the spec and bug db is probably the > > > saner option, so: > > > > > > Reviewed-by: Damien Lespiau > > > > Spoke too soon. This register is in the render context so has to be > > written from the ring... > > Not sure what you mean. I thought all WAs inited here are written from > the ring. Oh, yes, excellent! r-b restored! Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/bxt: limit WaDisableMaskBasedCammingInRCC to stepping A
On ti, 2015-05-19 at 15:46 +0100, Damien Lespiau wrote: > On Tue, May 19, 2015 at 03:39:25PM +0100, Damien Lespiau wrote: > > On Tue, May 19, 2015 at 03:04:59PM +0300, Imre Deak wrote: > > > Also make the WA comment consistent with the rest, where the stepping > > > info is not shown. > > > > > > Signed-off-by: Imre Deak > > > --- > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++-- > > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > > > [ The patchset is a follow-up to: > > > http://lists.freedesktop.org/archives/intel-gfx/2015-May/065989.html ] > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > index 9b96ed7..461b9be 100644 > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > @@ -961,12 +961,9 @@ static int gen9_init_workarounds(struct > > > intel_engine_cs *ring) > > > WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, > > > GEN9_CCS_TLB_PREFETCH_ENABLE); > > > > > > - /* > > > - * FIXME: don't apply the following on BXT for stepping C. On BXT A0 > > > - * the flag reads back as 0. > > > - */ > > > - /* WaDisableMaskBasedCammingInRCC:sklC,bxtA */ > > > - if (INTEL_REVID(dev) == SKL_REVID_C0 || IS_BROXTON(dev)) > > > + /* WaDisableMaskBasedCammingInRCC:skl,bxt */ > > > > For the record, there seem to be some confusion in the W/A db: > > > > - The W/A seems to have been renamed to > > WaDisablePixelMaskBasedCammingInRcpbe > > and indeed there's a bit to do that, but the bug in question talks > > about bit 14 of 7308, which is the disabling bit for the RCC unit > > - The W/A isn't listed in the W/A db for BXT > > > > In doubt, defaulting to trusting the spec and bug db is probably the > > saner option, so: > > > > Reviewed-by: Damien Lespiau > > Spoke too soon. This register is in the render context so has to be > written from the ring... Not sure what you mean. I thought all WAs inited here are written from the ring. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/bxt: limit WaDisableMaskBasedCammingInRCC to stepping A
On Tue, May 19, 2015 at 03:39:25PM +0100, Damien Lespiau wrote: > On Tue, May 19, 2015 at 03:04:59PM +0300, Imre Deak wrote: > > Also make the WA comment consistent with the rest, where the stepping > > info is not shown. > > > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++-- > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > [ The patchset is a follow-up to: > > http://lists.freedesktop.org/archives/intel-gfx/2015-May/065989.html ] > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 9b96ed7..461b9be 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -961,12 +961,9 @@ static int gen9_init_workarounds(struct > > intel_engine_cs *ring) > > WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, > > GEN9_CCS_TLB_PREFETCH_ENABLE); > > > > - /* > > -* FIXME: don't apply the following on BXT for stepping C. On BXT A0 > > -* the flag reads back as 0. > > -*/ > > - /* WaDisableMaskBasedCammingInRCC:sklC,bxtA */ > > - if (INTEL_REVID(dev) == SKL_REVID_C0 || IS_BROXTON(dev)) > > + /* WaDisableMaskBasedCammingInRCC:skl,bxt */ > > For the record, there seem to be some confusion in the W/A db: > > - The W/A seems to have been renamed to > WaDisablePixelMaskBasedCammingInRcpbe > and indeed there's a bit to do that, but the bug in question talks > about bit 14 of 7308, which is the disabling bit for the RCC unit > - The W/A isn't listed in the W/A db for BXT > > In doubt, defaulting to trusting the spec and bug db is probably the > saner option, so: > > Reviewed-by: Damien Lespiau Spoke too soon. This register is in the render context so has to be written from the ring... -- Damien ___ 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/skl: add F0 stepping ID
On Tue, May 19, 2015 at 03:05:00PM +0300, Imre Deak wrote:
> Signed-off-by: Imre Deak
Reviewed-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 840f08f..731b5ce 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2378,6 +2378,7 @@ struct drm_i915_cmd_table {
> #define SKL_REVID_C0 (0x2)
> #define SKL_REVID_D0 (0x3)
> #define SKL_REVID_E0 (0x4)
> +#define SKL_REVID_F0 (0x5)
>
> #define BXT_REVID_A0 (0x0)
> #define BXT_REVID_B0 (0x3)
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Unconditionally flush writes before execbuffer
On Mon, May 11, 2015 at 04:25:52PM +0100, Chris Wilson wrote: > On Mon, May 11, 2015 at 12:34:37PM +0200, Daniel Vetter wrote: > > On Mon, May 11, 2015 at 08:51:36AM +0100, Chris Wilson wrote: > > > With the advent of mmap(wc), we have a path to write directly into > > > active GPU buffers. When combined with async updates (i.e. avoiding the > > > explicit domain management along with the memory barriers and GPU > > > stalls) we start to see the GPU read the wrong values from memory - i.e. > > > we have insufficient memory barriers along the execbuffer path. Writes > > > through the GTT should have been naturally serialised with execution > > > through the GTT as well and so the impact only seems to be from the WC > > > paths. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Akash Goel > > > Cc: sta...@vger.kernel.org > > > > Do we have a nasty igt for this? Bugzilla? > > I've added igt/gem_streaming_writes. > > That wmb() is not enough for !llc. Since the wmb() made piglit happy it > is quite possible I haven't hit the same path exactly, but it's going to > take some investigation to see if igt/gem_streaming_writes can possibly > work on !llc. Humbug. Found the bug in gem_streaming_writes, even though I still think the wmb() is strictly required, it runs fine without (presumably I haven't managed to avoid all barriers in the execbuffer path yet). However, I think can improve the stress by inserting extra gpu load -- that should help make the CPU writes / GPU reads of the buffer concurrent? -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 2/3] drm/atomic: add drm_atomic_add_affected_planes
This is a convenience function to add all planes for a crtc,
similar to add_affected_connectors. This will be used in
drm_atomic_helper_check_modeset, but drivers can call it too
when they need to recalculate all state.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_atomic.c | 32
include/drm/drm_atomic.h | 4
2 files changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index cd1b16b25716..c48b7db438b8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -956,6 +956,38 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state
*state,
EXPORT_SYMBOL(drm_atomic_add_affected_connectors);
/**
+ * drm_atomic_add_affected_planes - add planes for crtc
+ * @state: atomic state
+ * @crtc: DRM crtc
+ *
+ * This function walks the current configuration and adds all planes
+ * currently used by @crtc to the atomic configuration @state.
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK
+ * then the w/w mutex code has detected a deadlock and the entire atomic
+ * sequence must be restarted. All other errors are fatal.
+ */
+int
+drm_atomic_add_affected_planes(struct drm_atomic_state *state,
+ struct drm_crtc *crtc)
+{
+ struct drm_plane *plane;
+
+ WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+
+ drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) {
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_plane_state(state, plane);
+
+ if (IS_ERR(plane_state))
+ return PTR_ERR(plane_state);
+ }
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_add_affected_planes);
+
+/**
* drm_atomic_connectors_for_crtc - count number of connected outputs
* @state: atomic state
* @crtc: DRM crtc
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index f0d3a7387d99..e89db0c377ba 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -120,6 +120,10 @@ drm_atomic_set_crtc_for_connector(struct
drm_connector_state *conn_state,
int __must_check
drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
struct drm_crtc *crtc);
+int __must_check
+drm_atomic_add_affected_planes(struct drm_atomic_state *state,
+ struct drm_crtc *crtc);
+
int
drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,
struct drm_crtc *crtc);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/atomic: add commit_planes_on_crtc helper
drm_atomic_helper_commit_planes calls all atomic_begin's first,
then updates all planes, finally calling atomic_flush.
Some drivers may want to things like disabling irq's
from their atomic_begin, in which case a second call to atomic_begin
will splat. By using commit_planes_on_crtc on each crtc in the
atomic state they'll evade that issue.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_atomic_helper.c | 53 +
include/drm/drm_atomic_helper.h | 1 +
2 files changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index b82ef6262469..2da8a16bb39e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1181,6 +1181,59 @@ void drm_atomic_helper_commit_planes(struct drm_device
*dev,
EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
/**
+ * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a crtc
+ * @old_crtc_state: atomic state object with the old crtc state
+ *
+ * This function commits the new plane state using the plane and atomic helper
+ * functions for planes on the specific crtc. It assumes that the atomic state
+ * has already been pushed into the relevant object state pointers, since this
+ * step can no longer fail.
+ *
+ * This function can only be used when planes are not allowed to move between
+ * different crtc's.
+ */
+void
+drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state)
+{
+ const struct drm_crtc_helper_funcs *crtc_funcs;
+ struct drm_crtc *crtc = old_crtc_state->crtc;
+ struct drm_atomic_state *old_state = old_crtc_state->state;
+ struct drm_plane *plane;
+ unsigned plane_mask;
+
+ plane_mask = old_crtc_state->plane_mask;
+ plane_mask |= crtc->state->plane_mask;
+
+ crtc_funcs = crtc->helper_private;
+ if (crtc_funcs && crtc_funcs->atomic_begin)
+ crtc_funcs->atomic_begin(crtc);
+
+ drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
+ struct drm_plane_state *old_plane_state =
+ drm_atomic_get_existing_plane_state(old_state, plane);
+ const struct drm_plane_helper_funcs *plane_funcs;
+
+ plane_funcs = plane->helper_private;
+
+ if (!old_plane_state || !plane_funcs)
+ continue;
+
+ WARN_ON(plane->state->crtc && plane->state->crtc != crtc);
+
+ if (drm_atomic_plane_disabling(plane, old_plane_state) &&
+ plane_funcs->atomic_disable)
+ plane_funcs->atomic_disable(plane, old_plane_state);
+ else if (plane->state->crtc ||
+drm_atomic_plane_disabling(plane, old_plane_state))
+ plane_funcs->atomic_update(plane, old_plane_state);
+ }
+
+ if (crtc_funcs->atomic_flush)
+ crtc_funcs->atomic_flush(crtc);
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
+
+/**
* drm_atomic_helper_cleanup_planes - cleanup plane resources after commit
* @dev: DRM device
* @old_state: atomic state object with old state structures
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 6ee0ee5b6143..cc1fee8a12d0 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -58,6 +58,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
struct drm_atomic_state *state);
void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
struct drm_atomic_state *old_state);
+void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state
*old_crtc_state);
void drm_atomic_helper_swap_state(struct drm_device *dev,
struct drm_atomic_state *state);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/atomic: add all affected planes in drm_atomic_helper_check_modeset
Drivers may need to recalculate plane state when a modeset occurs, not reliably adding them might cause hard to debug bugs. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_atomic_helper.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2da8a16bb39e..4f5a427e9405 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -429,6 +429,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret != 0) return ret; + ret = drm_atomic_add_affected_planes(state, crtc); + if (ret != 0) + return ret; + num_connectors = drm_atomic_connectors_for_crtc(state, crtc); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/bxt: limit WaDisableMaskBasedCammingInRCC to stepping A
On Tue, May 19, 2015 at 03:04:59PM +0300, Imre Deak wrote: > Also make the WA comment consistent with the rest, where the stepping > info is not shown. > > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > [ The patchset is a follow-up to: > http://lists.freedesktop.org/archives/intel-gfx/2015-May/065989.html ] > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 9b96ed7..461b9be 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -961,12 +961,9 @@ static int gen9_init_workarounds(struct intel_engine_cs > *ring) > WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, > GEN9_CCS_TLB_PREFETCH_ENABLE); > > - /* > - * FIXME: don't apply the following on BXT for stepping C. On BXT A0 > - * the flag reads back as 0. > - */ > - /* WaDisableMaskBasedCammingInRCC:sklC,bxtA */ > - if (INTEL_REVID(dev) == SKL_REVID_C0 || IS_BROXTON(dev)) > + /* WaDisableMaskBasedCammingInRCC:skl,bxt */ For the record, there seem to be some confusion in the W/A db: - The W/A seems to have been renamed to WaDisablePixelMaskBasedCammingInRcpbe and indeed there's a bit to do that, but the bug in question talks about bit 14 of 7308, which is the disabling bit for the RCC unit - The W/A isn't listed in the W/A db for BXT In doubt, defaulting to trusting the spec and bug db is probably the saner option, so: Reviewed-by: Damien Lespiau -- Damien > + if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) == SKL_REVID_C0) || > + (IS_BROXTON(dev) && INTEL_REVID(dev) < BXT_REVID_B0)) > WA_SET_BIT_MASKED(SLICE_ECO_CHICKEN0, > PIXEL_MASK_CAMMING_DISABLE); > > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
v2: Addressed comments from Tim Gore
Signed-off-by: Derek Morton
---
lib/igt_core.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..62b1e6a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1104,7 +1104,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void)
{
- for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1121,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1229,8 @@ static void children_exit_handler(int sig)
{
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- && wait(&status))
;
@@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void)
{
- int i;
+ unsigned int i;
for (i = 0; i < ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig)
@@ -1419,7 +1423,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i < ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig)
@@ -1481,7 +1485,7 @@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn)
{
- int i;
+ unsigned int i;
for (i = 0; i < exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1525,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1728,8 @@ out:
static void igt_alarm_handler(int signal)
{
+ (void)signal; /* Not used, suppress warning */
+
igt_info("Timed out\n");
/* exit with failure status */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] topic/drm-misc
Hi Dave, Scattering of random drm core patches. Bunch of atomic prep work too, but the final bits for blob properties, atomic modesets and lifting the experimental tag on the atomic ioctl are still blocked on Daniel Stone finalizing and testing the weston support for it. I hope that we can get it all ready for 4.2 though. Cheers, Daniel The following changes since commit c0fe07aa50befe2e6e6525181e2080377a1c1494: drm/qxl: rewrite framebuffer support (2015-05-07 13:09:25 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2015-05-19 for you to fetch changes up to 036ef5733ba433760a3512bb5f7a155946e2df05: drm/atomic: Allow drivers to subclass drm_atomic_state, v3 (2015-05-18 16:39:41 +0200) Daniel Stone (5): drm/atomic: Don't open-code CRTC state destroy drm: Don't leak path blob property when updating drm: Introduce helper for replacing blob properties drm: Introduce blob_lock drm: Add reference counting to blob properties Daniel Vetter (2): drm/atomic-helpers: Update vblank timestamping constants drm/atomic-helpers: Export drm_atomic_helper_update_legacy_modeset_state Gustavo Padovan (1): drm/atomic: remove duplicated assignment of old_plane_state Jani Nikula (4): drm/sysfs: add a helper for extracting connector type from kobject drm/sysfs: make optional attribute groups per connector type drm/sysfs: split DVI-I and TV-out attributes drm/sysfs: remove unnecessary connector type checks Jon Hunter (1): drm/dp: Fix comment in DP helper Maarten Lankhorst (4): drm/i915: get rid of -Iinclude/drm drm/core: get rid of -Iinclude/drm drm/atomic: add drm_atomic_get_existing_*_state helpers drm/atomic: Allow drivers to subclass drm_atomic_state, v3 Tomasz Figa (1): drm/prime: Allow internal imports without import_sg_table Ville Syrjälä (4): drm/edid: Fix up DMT modes drm/edid: Add the DMT ID in the comments drm/edid: Add DMT modes with ID > 0x50 drm/edid: Add CEA modes before inferred modes drivers/gpu/drm/Makefile| 2 - drivers/gpu/drm/drm_atomic.c| 134 + drivers/gpu/drm/drm_atomic_helper.c | 29 ++- drivers/gpu/drm/drm_crtc.c | 325 ++-- drivers/gpu/drm/drm_crtc_helper.c | 7 +- drivers/gpu/drm/drm_edid.c | 206 +++- drivers/gpu/drm/drm_flip_work.c | 4 +- drivers/gpu/drm/drm_prime.c | 6 +- drivers/gpu/drm/drm_sysfs.c | 160 ++-- drivers/gpu/drm/i915/Makefile | 2 - drivers/gpu/drm/i915/i915_gem_userptr.c | 4 +- include/drm/drm_atomic.h| 55 ++ include/drm/drm_atomic_helper.h | 4 + include/drm/drm_crtc.h | 26 ++- include/drm/drm_dp_helper.h | 6 +- 15 files changed, 696 insertions(+), 274 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] list-workarounds: Print the line where the parsing error occured
Useful to understand the warnings the scripts prints.
Signed-off-by: Damien Lespiau
---
scripts/list-workarounds | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/scripts/list-workarounds b/scripts/list-workarounds
index 42af6a3..d11b6a9 100755
--- a/scripts/list-workarounds
+++ b/scripts/list-workarounds
@@ -19,10 +19,11 @@ def find_nth(haystack, needle, n):
valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw',
'chv', 'skl', 'bxt')
-def parse_platforms(p):
+def parse_platforms(line, p):
l = p.split(',')
for p in l:
if p not in valid_platforms:
+ sys.stdout.write("warning: %s\n" % line)
sys.stdout.write("unknown platform %s\n" % p)
return l
@@ -40,6 +41,7 @@ def parse(me):
# no platform has been specified
name = waname_re.search(line).group('name')
path = line[:find_nth(line, ':', 2)]
+ sys.stdout.write("warning: %s\n" % line)
sys.stdout.write("%s: no platform for %s\n"
% (path, name))
continue
@@ -48,12 +50,12 @@ def parse(me):
platforms = match.group('platforms')
if wa_name in workarounds:
- platforms = parse_platforms(platforms)
+ platforms = parse_platforms(line, platforms)
for p in platforms:
if not p in workarounds[wa_name]:
workarounds[wa_name].append(p)
else:
- workarounds[wa_name] = parse_platforms(platforms)
+ workarounds[wa_name] = parse_platforms(line, platforms)
def execute(cmd):
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH pre4/4] drm/i915/bxt: fix WaForceContextSaveRestoreNonCoherent on steppings B0+
On B0 and C0 steppings the workaround enable bit would be overriden by
default, so the overriding must be disabled.
The WA was added in
commit 83a24979c40ebbf0fa0cd14df16f74142f373cd3
Author: Nick Hoath
Date: Fri Apr 10 13:12:26 2015 +0100
drm/i915/bxt: Add WaForceContextSaveRestoreNonCoherent
Spotted-by: Mika Kuoppala
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 84af255..5afff3a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5791,6 +5791,7 @@ enum skl_disp_power_wells {
/* GEN8 chicken */
#define HDC_CHICKEN0 0x7300
+#define HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE(1<<15)
#define HDC_FENCE_DEST_SLM_DISABLE(1<<14)
#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11)
#define HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT (1<<5)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2e342db..643fe89 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1049,6 +1049,7 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ uint32_t tmp;
gen9_init_workarounds(ring);
@@ -1057,8 +1058,10 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
STALL_DOP_GATING_DISABLE);
/* WaForceContextSaveRestoreNonCoherent:bxt */
- WA_SET_BIT_MASKED(HDC_CHICKEN0,
- HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT);
+ tmp = HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT;
+ if (INTEL_REVID(dev) >= BXT_REVID_B0)
+ tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
+ WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
return 0;
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/4] drm/i915/skl: enable WaForceContextSaveRestoreNonCoherent
v2:
- set the override disable flag too on stepping F0 (mika)
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 643fe89..2472415 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -918,6 +918,7 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ uint32_t tmp;
/* WaDisablePartialInstShootdown:skl,bxt */
WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
@@ -973,6 +974,13 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1,
GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
+ /* WaForceContextSaveRestoreNonCoherent:skl,bxt */
+ tmp = HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT;
+ if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) == SKL_REVID_F0) ||
+ (IS_BROXTON(dev) && INTEL_REVID(dev) >= BXT_REVID_B0))
+ tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
+ WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
+
return 0;
}
@@ -1049,7 +1057,6 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint32_t tmp;
gen9_init_workarounds(ring);
@@ -1057,12 +1064,6 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
STALL_DOP_GATING_DISABLE);
- /* WaForceContextSaveRestoreNonCoherent:bxt */
- tmp = HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT;
- if (INTEL_REVID(dev) >= BXT_REVID_B0)
- tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
- WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
-
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 1/2] drm/i915: Detach hangcheck from request lists
On Tue, May 19, 2015 at 12:03:44PM +0100, Tomas Elf wrote: > >+if (ring->buffer && > >+ring->buffer->tail != tail && > >+waitqueue_active(&ring->irq_queue)) > >+return true; > >+ > > 1. For some reason going from one waitqueue_active() check in > i915_hangcheck_elapsed() to two separate calls in two separate > functions does not sit perfectly well with me. Maybe it's not that > important but would it make sense to take the body of > check_for_missed_irq() and integrate it in engine_idle(), call > waitqueue_active() once and use the result twice: first in the check > in the block above and then in the missing irq check that follows > immediately? No it is just that stop_rings is the wrong mechanism and that has lead to this kerfuffle with using waitqueue_active() as a test for engine busyness. That is plainly wrong. -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 i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
> -Original Message-
> From: Morton, Derek J
> Sent: Tuesday, May 19, 2015 12:21 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
> Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
>
> Fixed variables incorrectly declared as signed instead of unsigned.
>
> Fixed 'unused parameter' warning from signal handlers that were not using
> the signal parameter.
>
> Signed-off-by: Derek Morton
> ---
> lib/igt_core.c | 24 +---
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..fb0b634 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -440,6 +440,8 @@ static void low_mem_killer_disable(bool disable)
> bool igt_exit_called; static void common_exit_handler(int sig) {
> + (void)sig; /* Not used, suppress warning */
> +
> if (!igt_only_list_subtests()) {
> low_mem_killer_disable(false);
> }
> @@ -1104,7 +1106,7 @@ static pid_t helper_process_pids[] =
>
> static void reset_helper_process_list(void) {
> - for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
> + for (unsigned int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
> helper_process_pids[i] = -1;
> helper_process_count = 0;
> }
> @@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
>
> static void fork_helper_exit_handler(int sig) {
> + (void)sig; /* Not used, suppress warning */
> +
> /* Inside a signal handler, play safe */
> - for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
> + for (unsigned int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
> pid_t pid = helper_process_pids[i];
> if (pid != -1) {
> kill(pid, SIGTERM);
> @@ -1227,6 +1231,8 @@ static void children_exit_handler(int sig) {
> int status;
>
> + (void)sig; /* Not used, suppress warning */
> +
> /* The exit handler can be called from a fatal signal, so play safe */
> while (num_test_children-- && wait(&status))
> ;
> @@ -1376,16 +1382,18 @@ static void restore_sig_handler(int sig_num)
>
> static void restore_all_sig_handler(void) {
> - int i;
> + unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(orig_sig); i++)
> - restore_sig_handler(i);
> + restore_sig_handler((int)i);
> }
>
> static void call_exit_handlers(int sig) {
> int i;
>
> + (void)sig; /* Not used, suppress warning */
> +
> if (!exit_handler_count) {
> return;
> }
> @@ -1419,7 +1427,7 @@ static bool crash_signal(int sig)
>
> static void fatal_sig_handler(int sig)
> {
> - int i;
> + unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(handled_signals); i++) {
> if (handled_signals[i].number != sig) @@ -1481,7 +1489,7
> @@ static void fatal_sig_handler(int sig)
> */
> void igt_install_exit_handler(igt_exit_handler_t fn) {
> - int i;
> + unsigned int i;
>
> for (i = 0; i < exit_handler_count; i++)
> if (exit_handler_fn[i] == fn)
> @@ -1521,7 +1529,7 @@ err:
> void igt_disable_exit_handler(void)
> {
> sigset_t set;
> - int i;
> + unsigned int i;
>
> if (exit_handler_disabled)
> return;
> @@ -1724,6 +1732,8 @@ out:
>
> static void igt_alarm_handler(int signal) {
> + (void)signal; /* Not used, suppress warning */
> +
> igt_info("Timed out\n");
>
> /* exit with failure status */
> --
> 1.9.1
In two of the functions where you use (void)sig, sig is in fact used.
In common_exit_handler it is used in the assert at the end. If this creates
A warning (which I observe also) it indicates that asserts are off which we
Probably don't want. The build explicitly uses "-include check-ndebug.h"
To create a compile error if NDEBUG is set, but this does not seem to be
Working, maybe due to the Android.mk also specifying -UNDEBUG.
Sorting this is probably for a separate patch.but I think you should remove
The "(void)sig" from common_exit_handler, so the resulting warning will
Remind us to fix the assert issue.
Also, in call_exit_handlers the sig parameter is used, so the (void)sig is
Not needed.
Tim
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/skl: enable WaForceContextSaveRestoreNonCoherent
On ti, 2015-05-19 at 16:08 +0300, Mika Kuoppala wrote: > Imre Deak writes: > > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 2e342db..0d1522f 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -973,6 +973,10 @@ static int gen9_init_workarounds(struct > > intel_engine_cs *ring) > > WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1, > >GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE); > > > > + /* WaForceContextSaveRestoreNonCoherent:skl,bxt */ > > + WA_SET_BIT_MASKED(HDC_CHICKEN0, > > + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); > > + > > I think we need to also set bit 15 as it looks to be master switch > for this bit. Yes, thanks for catching it. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/skl: enable WaForceContextSaveRestoreNonCoherent
Imre Deak writes: > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 2e342db..0d1522f 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -973,6 +973,10 @@ static int gen9_init_workarounds(struct intel_engine_cs > *ring) > WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1, > GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE); > > + /* WaForceContextSaveRestoreNonCoherent:skl,bxt */ > + WA_SET_BIT_MASKED(HDC_CHICKEN0, > + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); > + I think we need to also set bit 15 as it looks to be master switch for this bit. -Mika > return 0; > } > > @@ -1056,10 +1060,6 @@ static int bxt_init_workarounds(struct intel_engine_cs > *ring) > WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > STALL_DOP_GATING_DISABLE); > > - /* WaForceContextSaveRestoreNonCoherent:bxt */ > - WA_SET_BIT_MASKED(HDC_CHICKEN0, > - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); > - > return 0; > } > > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915/skl: enable WaForceContextSaveRestoreNonCoherent
Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/intel_ringbuffer.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2e342db..0d1522f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -973,6 +973,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1, GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE); + /* WaForceContextSaveRestoreNonCoherent:skl,bxt */ + WA_SET_BIT_MASKED(HDC_CHICKEN0, + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); + return 0; } @@ -1056,10 +1060,6 @@ static int bxt_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE); - /* WaForceContextSaveRestoreNonCoherent:bxt */ - WA_SET_BIT_MASKED(HDC_CHICKEN0, - HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT); - return 0; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915/skl: enable WaDisableSbeCacheDispatchPortSharing
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 461b9be..2e342db 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -967,6 +967,12 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
WA_SET_BIT_MASKED(SLICE_ECO_CHICKEN0,
PIXEL_MASK_CAMMING_DISABLE);
+ /* WaDisableSbeCacheDispatchPortSharing:skl,bxt */
+ if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_F0) ||
+ (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0))
+ WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1,
+ GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
+
return 0;
}
@@ -1050,13 +1056,6 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
STALL_DOP_GATING_DISABLE);
- /* WaDisableSbeCacheDispatchPortSharing:bxt */
- if (INTEL_REVID(dev) <= BXT_REVID_B0) {
- WA_SET_BIT_MASKED(
- GEN7_HALF_SLICE_CHICKEN1,
- GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
- }
-
/* WaForceContextSaveRestoreNonCoherent:bxt */
WA_SET_BIT_MASKED(HDC_CHICKEN0,
HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915/skl: add F0 stepping ID
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 840f08f..731b5ce 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2378,6 +2378,7 @@ struct drm_i915_cmd_table {
#define SKL_REVID_C0 (0x2)
#define SKL_REVID_D0 (0x3)
#define SKL_REVID_E0 (0x4)
+#define SKL_REVID_F0 (0x5)
#define BXT_REVID_A0 (0x0)
#define BXT_REVID_B0 (0x3)
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915/bxt: limit WaDisableMaskBasedCammingInRCC to stepping A
Also make the WA comment consistent with the rest, where the stepping info is not shown. Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) [ The patchset is a follow-up to: http://lists.freedesktop.org/archives/intel-gfx/2015-May/065989.html ] diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 9b96ed7..461b9be 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -961,12 +961,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, GEN9_CCS_TLB_PREFETCH_ENABLE); - /* -* FIXME: don't apply the following on BXT for stepping C. On BXT A0 -* the flag reads back as 0. -*/ - /* WaDisableMaskBasedCammingInRCC:sklC,bxtA */ - if (INTEL_REVID(dev) == SKL_REVID_C0 || IS_BROXTON(dev)) + /* WaDisableMaskBasedCammingInRCC:skl,bxt */ + if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) == SKL_REVID_C0) || + (IS_BROXTON(dev) && INTEL_REVID(dev) < BXT_REVID_B0)) WA_SET_BIT_MASKED(SLICE_ECO_CHICKEN0, PIXEL_MASK_CAMMING_DISABLE); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH resend 2/3] drm/i915: Enable resource streamer bits on MI_BATCH_BUFFER_START
Adds support for executing the resource streamer on BDW and HSW v2: Add support for Execlists (Minu Mathai ) Reviewed-by: Chris Wilson Signed-off-by: Abdiel Janulgue --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_lrc.c| 3 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 -- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b522eb6..238bb25 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -356,6 +356,7 @@ #define MI_BATCH_BUFFER_START MI_INSTR(0x31, 0) #define MI_BATCH_GTT (2<<6) /* aliased with (1<<7) on gen4 */ #define MI_BATCH_BUFFER_START_GEN8 MI_INSTR(0x31, 1) +#define MI_BATCH_RESOURCE_STREAMER (1<<10) #define MI_PREDICATE_SRC0 (0x2400) #define MI_PREDICATE_SRC1 (0x2408) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index fcb074b..d523494 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1172,7 +1172,8 @@ static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf, return ret; /* FIXME(BDW): Address space and security selectors. */ - intel_logical_ring_emit(ringbuf, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8)); + intel_logical_ring_emit(ringbuf, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8) | + (I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0)); intel_logical_ring_emit(ringbuf, lower_32_bits(offset)); intel_logical_ring_emit(ringbuf, upper_32_bits(offset)); intel_logical_ring_emit(ringbuf, MI_NOOP); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 441e250..9045144 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2385,7 +2385,8 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs *ring, return ret; /* FIXME(BDW): Address space and security selectors. */ - intel_ring_emit(ring, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8)); + intel_ring_emit(ring, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8) | + (dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0)); intel_ring_emit(ring, lower_32_bits(offset)); intel_ring_emit(ring, upper_32_bits(offset)); intel_ring_emit(ring, MI_NOOP); @@ -2408,7 +2409,8 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs *ring, intel_ring_emit(ring, MI_BATCH_BUFFER_START | (dispatch_flags & I915_DISPATCH_SECURE ? -0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW)); +0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | + (dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0)); /* bit0-7 is the length on GEN6+ */ intel_ring_emit(ring, offset); intel_ring_advance(ring); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH resend 1/3] drm/i915: Expose I915_EXEC_RESOURCE_STREAMER flag
Ensures that the batch buffer is executed by the resource streamer
Testcase: igt/gem_exec_params
Reviewed-by: Chris Wilson
Signed-off-by: Abdiel Janulgue
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 +++
drivers/gpu/drm/i915/intel_ringbuffer.h| 1 +
include/uapi/drm/i915_drm.h| 7 ++-
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a3190e79..8a0abbb 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1485,6 +1485,21 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
return -EINVAL;
}
+ if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
+ if (!IS_HASWELL(dev) && INTEL_INFO(dev)->gen < 8) {
+ DRM_DEBUG("RS is only allowed for Haswell, Gen8 "
+ "and above\n");
+ return -EINVAL;
+ }
+ if (ring->id != RCS) {
+ DRM_DEBUG("RS is not available on %s\n",
+ring->name);
+ return -EINVAL;
+ }
+
+ dispatch_flags |= I915_DISPATCH_RS;
+ }
+
intel_runtime_pm_get(dev_priv);
ret = i915_mutex_lock_interruptible(dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c761fe0..3521bc0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -167,6 +167,7 @@ struct intel_engine_cs {
unsigned dispatch_flags);
#define I915_DISPATCH_SECURE 0x1
#define I915_DISPATCH_PINNED 0x2
+#define I915_DISPATCH_RS 0x4
void(*cleanup)(struct intel_engine_cs *ring);
/* GEN8 signal/wait table - never trust comments!
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 551b673..a4c1a5c 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -760,7 +760,12 @@ struct drm_i915_gem_execbuffer2 {
#define I915_EXEC_BSD_RING1(1<<13)
#define I915_EXEC_BSD_RING2(2<<13)
-#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15)
+/** Tell the kernel that the batchbuffer is processed by
+ * the resource streamer.
+ */
+#define I915_EXEC_RESOURCE_STREAMER (1<<16)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER <<1)
#define I915_EXEC_CONTEXT_ID_MASK (0x)
#define i915_execbuffer2_set_context_id(eb2, context) \
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH resend 3/3] drm/i915: Enable Resource Streamer state save/restore in HSW
Also clarify comments on context size that the extra state for
Resource Streamer is included.
v2: Don't remove the extended save/restore enabled for older
platforms. (Ville)
Use new MI_SET_CONTEXT defines for HSW RS save/restore state
instead of extended save/restore. (Daniel)
Suggested-by: Chris Wilson
Reviewed-by: Chris Wilson
Signed-off-by: Abdiel Janulgue
---
drivers/gpu/drm/i915/i915_gem_context.c | 4 +++-
drivers/gpu/drm/i915/i915_reg.h | 5 -
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f3e84c4..1a521dd 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -509,7 +509,9 @@ mi_set_context(struct intel_engine_cs *ring,
}
/* These flags are for resource streamer on HSW+ */
- if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
+ if (IS_HASWELL(ring->dev))
+ flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN);
+ else if (INTEL_INFO(ring->dev)->gen < 8)
flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 238bb25..2b1321d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -316,6 +316,8 @@
#define MI_RESTORE_EXT_STATE_EN (1<<2)
#define MI_FORCE_RESTORE (1<<1)
#define MI_RESTORE_INHIBIT (1<<0)
+#define HSW_MI_RS_SAVE_STATE_EN (1<<3)
+#define HSW_MI_RS_RESTORE_STATE_EN(1<<2)
#define MI_SEMAPHORE_SIGNALMI_INSTR(0x1b, 0) /* GEN8+ */
#define MI_SEMAPHORE_TARGET(engine) ((engine)<<15)
#define MI_SEMAPHORE_WAIT MI_INSTR(0x1c, 2) /* GEN8+ */
@@ -2498,7 +2500,8 @@ enum skl_disp_power_wells {
* valid. Now, docs explain in dwords what is in the context object. The full
* size is 70720 bytes, however, the power context and execlist context will
* never be saved (power context is stored elsewhere, and execlists don't work
- * on HSW) - so the final size is 66944 bytes, which rounds to 17 pages.
+ * on HSW) - so the final size, including the extra state required for the
+ * Resource Streamer, is 66944 bytes, which rounds to 17 pages.
*/
#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE)
/* Same as Haswell, but 72064 bytes now. */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Experiencing FIFO underruns on Intel Skylake platform
+intel-gfx On Tue, 19 May 2015, Rainer Koenig wrote: > Hi, > > I'm testing the vanilla kernel on prototype boards for Intel Skylake. > The graphics adapter on those boards identifies like this: > > 00:02.0 0300: 8086:1912 (rev 04) (prog-if 00 [VGA controller]) > Subsystem: 1734:121c > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR- FastB2B- DisINTx+ > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR-Latency: 0 > Interrupt: pin A routed to IRQ 123 > Region 0: Memory at d000 (64-bit, non-prefetchable) [size=16M] > Region 2: Memory at c000 (64-bit, prefetchable) [size=256M] > Region 4: I/O ports at f000 [size=64] > Expansion ROM at [disabled] > Capabilities: [40] Vendor Specific Information: Len=0c > Capabilities: [70] Express (v2) Root Complex Integrated Endpoint, MSI 00 > DevCap: MaxPayload 128 bytes, PhantFunc 0 > ExtTag- RBE+ > DevCtl: Report errors: Correctable- Non-Fatal- Fatal- > Unsupported- > RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- > MaxPayload 128 bytes, MaxReadReq 128 bytes > DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- > TransPend- > DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR-, > OBFF > Not Supported > DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, > OBFF > Disabled > Capabilities: [ac] MSI: Enable+ Count=1/1 Maskable- 64bit- > Address: fee00018 Data: > Capabilities: [d0] Power Management version 2 > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA > PME(D0-,D1-,D2-,D3hot-,D3cold-) > Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- > Capabilities: [100 v1] #1b > Capabilities: [200 v1] Address Translation Service (ATS) > ATSCap: Invalidate Queue Depth: 00 > ATSCtl: Enable-, Smallest Translation Unit: 00 > Capabilities: [300 v1] #13 > Kernel driver in use: i915 > Kernel modules: i915 > > > I'm running both Fedora 21 and OpenSUSE 13.2 on those boards at the > moment, both with a self compiled Kernel 4.1-rc4 (latest git pull was > today). > > Problem: Whenever power management kicks in and blanks the screen the > systems behave very strange when trying to get the screen back. The > screen is not fully restored then, but when I move the mouse (Gnome 3 or > XFCE desktops) up and down the image rolls up and down like a curtain. I > see no mouse cursor anymore and can't resume to normal operation. > > When this happens I see the following line in dmesg: > > [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO > underrun > > Hardware is still the first prototypes for Skylake, BIOS is beta too. > But I wanted to give a note about that problem here, maybe someone has > an idea what is missing. For brand new platforms you should be running drm-intel-nightly branch from http://cgit.freedesktop.org/drm-intel. Double check that you have either i915.preliminary_hw_support=1 module parameter set or CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT=y config option set. BR, Jani. > > Regards > Rainer > -- > Dipl.-Inf. (FH) Rainer Koenig > Project Manager Linux Clients > Dept. PDG WPS R&D SW OSE > > Fujitsu Technology Solutions > Bürgermeister-Ullrich-Str. 100 > 86199 Augsburg > Germany > > Telephone: +49-821-804-3321 > Telefax: +49-821-804-2131 > Mail: mailto:rainer.koe...@ts.fujitsu.com > > Internet ts.fujtsu.com > Company Details ts.fujitsu.com/imprint.html > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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 3/4 v3] drm/i915: Tighten the exposure ARGB/ABGR 8888 formats
ARGB is used for cursors on all platforms so we need to allow it
everywhere.
ABGR is currently only honoured:
- on VLV/CHV in sprite planes
- on SKL+ for primary and sprite planes
so only allow it for those platforms.
Note that we only support ARGB/ABGR on the primary plane for
SKL/BXT because we have in line of sight the pipe bottom color on those
platforms and because the primary plane programming on VLV/CHV doesn't
anything different for those formats today.
v2: Fix the logic to forbid the creation ABGR2101010 fbs (Ville)
v3: Still allow the creation of ARGB fbs now that cursor planes use
real fb objects (found by PRTS).
Reviewed-by: Ville Syrjälä
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_display.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8a3d936..9d2d6fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -51,7 +51,6 @@ static const uint32_t i8xx_primary_formats[] = {
DRM_FORMAT_RGB565,
DRM_FORMAT_XRGB1555,
DRM_FORMAT_XRGB,
- DRM_FORMAT_ARGB,
};
/* Primary plane formats for gen >= 4 */
@@ -60,6 +59,15 @@ static const uint32_t i965_primary_formats[] = {
DRM_FORMAT_RGB565,
DRM_FORMAT_XRGB,
DRM_FORMAT_XBGR,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+};
+
+static const uint32_t skl_primary_formats[] = {
+ DRM_FORMAT_C8,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
DRM_FORMAT_ARGB,
DRM_FORMAT_ABGR,
DRM_FORMAT_XRGB2101010,
@@ -2704,11 +2712,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
dspcntr |= DISPPLANE_BGRX565;
break;
case DRM_FORMAT_XRGB:
- case DRM_FORMAT_ARGB:
dspcntr |= DISPPLANE_BGRX888;
break;
case DRM_FORMAT_XBGR:
- case DRM_FORMAT_ABGR:
dspcntr |= DISPPLANE_RGBX888;
break;
case DRM_FORMAT_XRGB2101010:
@@ -2810,11 +2816,9 @@ static void ironlake_update_primary_plane(struct
drm_crtc *crtc,
dspcntr |= DISPPLANE_BGRX565;
break;
case DRM_FORMAT_XRGB:
- case DRM_FORMAT_ARGB:
dspcntr |= DISPPLANE_BGRX888;
break;
case DRM_FORMAT_XBGR:
- case DRM_FORMAT_ABGR:
dspcntr |= DISPPLANE_RGBX888;
break;
case DRM_FORMAT_XRGB2101010:
@@ -13293,12 +13297,15 @@ static struct drm_plane
*intel_primary_plane_create(struct drm_device *dev,
if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
primary->plane = !pipe;
- if (INTEL_INFO(dev)->gen <= 3) {
- intel_primary_formats = i8xx_primary_formats;
- num_formats = ARRAY_SIZE(i8xx_primary_formats);
- } else {
+ if (INTEL_INFO(dev)->gen >= 9) {
+ intel_primary_formats = skl_primary_formats;
+ num_formats = ARRAY_SIZE(skl_primary_formats);
+ } else if (INTEL_INFO(dev)->gen >= 4) {
intel_primary_formats = i965_primary_formats;
num_formats = ARRAY_SIZE(i965_primary_formats);
+ } else {
+ intel_primary_formats = i8xx_primary_formats;
+ num_formats = ARRAY_SIZE(i8xx_primary_formats);
}
drm_universal_plane_init(dev, &primary->base, 0,
@@ -13985,8 +13992,14 @@ static int intel_framebuffer_init(struct drm_device
*dev,
return -EINVAL;
}
break;
- case DRM_FORMAT_XBGR:
case DRM_FORMAT_ABGR:
+ if (!IS_VALLEYVIEW(dev) && INTEL_INFO(dev)->gen < 9) {
+ DRM_DEBUG("unsupported pixel format: %s\n",
+ drm_get_format_name(mode_cmd->pixel_format));
+ return -EINVAL;
+ }
+ break;
+ case DRM_FORMAT_XBGR:
case DRM_FORMAT_XRGB2101010:
case DRM_FORMAT_XBGR2101010:
if (INTEL_INFO(dev)->gen < 4) {
--
2.1.0
___
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: Make hangcheck logging more compact
On 08/05/2015 14:39, Mika Kuoppala wrote:
With commit aaecdf611a05 ("drm/i915: Stop gathering error
states for CS error interrupts") we only call i915_handle_error()
on call sites where there is a stuck/hung gpu. So there is
no more need to carry around extra information into dmesg.
Emit one loud bang into dmesg with first hanging ring as
culprit. Rest of the details will be in error state.
Based-on-patch-by: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_gpu_error.c | 4 +---
drivers/gpu/drm/i915/i915_irq.c | 26 --
2 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 9c0db19..292cf1f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1251,9 +1251,7 @@ static void i915_error_capture_msg(struct drm_device *dev,
error->ring[ring_id].pid);
scnprintf(error->error_msg + len, sizeof(error->error_msg) - len,
- ", reason: %s, action: %s",
- error_msg,
- wedged ? "reset" : "continue");
+ ", %s", error_msg);
}
Once you've removed the reference to the wedged parameter from the
scnprintf statement I can't see any other references to it anywhere else
in the function. How about we remove that parameter entirely from the
function signature?
Thanks,
Tomas
static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a3244bd..a3b5001 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2924,14 +2924,12 @@ static bool check_for_missed_irq(struct intel_engine_cs
*ring)
return true;
}
-static bool hangcheck_handle_stuck_ring(struct intel_engine_cs *ring, u64
acthd)
+static void hangcheck_handle_stuck_ring(struct intel_engine_cs *ring, u64
acthd)
{
#define BUSY 1
#define KICK 5
#define HUNG 20
-
struct intel_ring_hangcheck *hc = &ring->hangcheck;
- bool there_is_hope = true;
/* We always increment the hangcheck score
* if the ring is busy and still processing
@@ -2964,11 +2962,8 @@ static bool hangcheck_handle_stuck_ring(struct
intel_engine_cs *ring, u64 acthd)
break;
case HANGCHECK_HUNG:
hc->score += HUNG;
- there_is_hope = false;
break;
}
-
- return there_is_hope;
}
/*
@@ -2987,8 +2982,7 @@ static void i915_hangcheck_elapsed(struct work_struct
*work)
struct drm_device *dev = dev_priv->dev;
struct intel_engine_cs *ring;
int i;
- int busy_count = 0, rings_hung = 0;
- bool stuck[I915_NUM_RINGS] = { 0 };
+ int busy_count = 0, ring_hung = -1;
if (!i915.enable_hangcheck)
return;
@@ -3043,19 +3037,15 @@ engine_check_done:
hc->acthd = acthd;
hc->start = start;
busy_count += busy;
- }
- for_each_ring(ring, dev_priv, i) {
- if (ring->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG) {
- DRM_INFO("%s on %s\n",
-stuck[i] ? "stuck" : "no progress",
-ring->name);
- rings_hung++;
- }
+ if (ring_hung == -1 &&
+ ring->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG)
+ ring_hung = i;
}
- if (rings_hung)
- return i915_handle_error(dev, true, "Ring hung");
+ if (ring_hung != -1)
+ return i915_handle_error(dev, true, "%s hung",
+dev_priv->ring[ring_hung].name);
if (busy_count)
/* Reset timer case chip hangs without another request
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
Signed-off-by: Derek Morton
---
lib/igt_core.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..fb0b634 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -440,6 +440,8 @@ static void low_mem_killer_disable(bool disable)
bool igt_exit_called;
static void common_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
+
if (!igt_only_list_subtests()) {
low_mem_killer_disable(false);
}
@@ -1104,7 +1106,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void)
{
- for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1231,8 @@ static void children_exit_handler(int sig)
{
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- && wait(&status))
;
@@ -1376,16 +1382,18 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void)
{
- int i;
+ unsigned int i;
for (i = 0; i < ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig)
{
int i;
+ (void)sig; /* Not used, suppress warning */
+
if (!exit_handler_count) {
return;
}
@@ -1419,7 +1427,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i < ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig)
@@ -1481,7 +1489,7 @@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn)
{
- int i;
+ unsigned int i;
for (i = 0; i < exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1529,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1732,8 @@ out:
static void igt_alarm_handler(int signal)
{
+ (void)signal; /* Not used, suppress warning */
+
igt_info("Timed out\n");
/* exit with failure status */
--
1.9.1
___
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: Detach hangcheck from request lists
On 08/05/2015 14:39, Mika Kuoppala wrote:
Hangcheck tries to peek into request list to see
if the ring was busy or not. But that leads to race
against the list addition in request submission.
And hangcheck saw a ring being idle, when in fact work was
just being submitted.
There is strong desire to keep hangcheck without
locks of any kind as it is our last line of defense
against failures that surpass our imagination.
Rework the hangcheck logic so that we find out
the ring busyness by inspecting hw engine state
and omit the request->list inspection.
v2: start is u32, push for 80 cols (Chris)
References: https://bugs.freedesktop.org/show_bug.cgi?id=89931
References: https://bugs.freedesktop.org/show_bug.cgi?id=89644
References: https://bugs.freedesktop.org/show_bug.cgi?id=88984
References: https://bugs.freedesktop.org/show_bug.cgi?id=88981
References: https://bugs.freedesktop.org/show_bug.cgi?id=87730
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_debugfs.c | 10 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 4 +-
drivers/gpu/drm/i915/i915_irq.c | 214
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +
4 files changed, 149 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index adbbdda..8c647bf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1295,6 +1295,7 @@ static int i915_hangcheck_info(struct seq_file *m, void
*unused)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
u64 acthd[I915_NUM_RINGS];
+ u32 start[I915_NUM_RINGS];
u32 seqno[I915_NUM_RINGS];
int i;
@@ -1308,6 +1309,7 @@ static int i915_hangcheck_info(struct seq_file *m, void
*unused)
for_each_ring(ring, dev_priv, i) {
seqno[i] = ring->get_seqno(ring, false);
acthd[i] = intel_ring_get_active_head(ring);
+ start[i] = I915_READ_START(ring);
}
intel_runtime_pm_put(dev_priv);
@@ -1328,8 +1330,14 @@ static int i915_hangcheck_info(struct seq_file *m, void
*unused)
(long long)acthd[i]);
seq_printf(m, "\tmax ACTHD = 0x%08llx\n",
(long long)ring->hangcheck.max_acthd);
+ seq_printf(m, "\tSTART = 0x%08x [current 0x%08x]\n",
+ ring->hangcheck.start,
+ start[i]);
+
seq_printf(m, "\tscore = %d\n", ring->hangcheck.score);
- seq_printf(m, "\taction = %d\n", ring->hangcheck.action);
+ seq_printf(m, "\taction = %s [%d]\n",
+ i915_hangcheck_action_to_str(ring->hangcheck.action),
+ ring->hangcheck.action);
}
Based on feedback I have seen in the past it seems like we want to keep
unrelated changes to separate patches. So in this case maybe we should
move the debugfs changes into its own patch rather keep it together with
the hangcheck changes?
return 0;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index a3e330d..9c0db19 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -220,7 +220,7 @@ static void print_error_buffers(struct
drm_i915_error_state_buf *m,
}
}
-static const char *hangcheck_action_to_str(enum intel_ring_hangcheck_action a)
+const char *i915_hangcheck_action_to_str(enum intel_ring_hangcheck_action a)
{
switch (a) {
case HANGCHECK_IDLE:
@@ -301,7 +301,7 @@ static void i915_ring_error_state(struct
drm_i915_error_state_buf *m,
err_printf(m, " ring->head: 0x%08x\n", ring->cpu_ring_head);
err_printf(m, " ring->tail: 0x%08x\n", ring->cpu_ring_tail);
err_printf(m, " hangcheck: %s [%d]\n",
- hangcheck_action_to_str(ring->hangcheck_action),
+ i915_hangcheck_action_to_str(ring->hangcheck_action),
ring->hangcheck_score);
See above. Maybe you could put this change together with the debugfs
changes in a separate patch seeing as it's more about presenting the
hangcheck action in a more informative way rather than anything related
to the hangcheck logic itself.
}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9da955e..a3244bd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2685,20 +2685,6 @@ static void gen8_disable_vblank(struct drm_device *dev,
int pipe)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
-static struct drm_i915_gem_request *
-ring_last_request(struct intel_engine_cs *ring)
-{
- return list_entry(ring->request_list.prev,
- struct drm_i915_gem_request, list);
-}
-
-static bool
-ring_idle(struct intel_engine_cs *ring)
-
[Intel-gfx] [PATCH v2] tests/gem_exec_params: check invalid flags for Resource Streamer
Make sure resource streamer flags works only in correct ring in
addition to checking next flag after the RS boundary fails.
v2: Make sure we reject RS on pre-hsw.
Cc: Daniel Vetter
Signed-off-by: Abdiel Janulgue
---
tests/gem_exec_params.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
index 54f0dc3..f374226 100644
--- a/tests/gem_exec_params.c
+++ b/tests/gem_exec_params.c
@@ -48,6 +48,7 @@
#define LOCAL_I915_EXEC_BSD_MASK (3<<13)
#define LOCAL_I915_EXEC_BSD_RING1 (1<<13)
#define LOCAL_I915_EXEC_BSD_RING2 (2<<13)
+#define LOCAL_I915_EXEC_RESOURCE_STREAMER (1<<16)
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_exec_object2 gem_exec[1];
@@ -220,7 +221,7 @@ igt_main
/* HANDLE_LUT and NO_RELOC are already exercised by gem_exec_lut_handle
*/
igt_subtest("invalid-flag") {
- execbuf.flags = I915_EXEC_RENDER | (I915_EXEC_HANDLE_LUT << 1);
+ execbuf.flags = I915_EXEC_RENDER |
(LOCAL_I915_EXEC_RESOURCE_STREAMER << 1);
RUN_FAIL(EINVAL);
}
@@ -234,6 +235,30 @@ igt_main
execbuf.num_cliprects = 0;
}
+ igt_subtest("rs-invalid-on-bsd-ring") {
+ igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
+ execbuf.flags = I915_EXEC_BSD |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
+ igt_subtest("rs-invalid-on-blt-ring") {
+ igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
+ execbuf.flags = I915_EXEC_BLT |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
+ igt_subtest("rs-invalid-on-vebox-ring") {
+ igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
+ execbuf.flags = I915_EXEC_VEBOX |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
+ igt_subtest("rs-invalid-gen") {
+ igt_require(!IS_HASWELL(devid) && intel_gen(devid) < 8);
+ execbuf.flags = I915_EXEC_RENDER |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
#define DIRT(name) \
igt_subtest(#name "-dirt") { \
execbuf.flags = 0; \
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/8] drm/i915/vlv/chv: Move resume_prepare() after uncore_early_sanitize()
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 5cc57f2..5a9399c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -808,17 +808,17 @@ static int i915_drm_resume_early(struct drm_device
> > *dev)
> >
> > pci_set_master(dev->pdev);
> >
> > - if (IS_VALLEYVIEW(dev_priv))
> > - ret = vlv_resume_prepare(dev_priv, false);
> > - if (ret)
> > - DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
> > -
> > intel_uncore_early_sanitize(dev, true);
> >
> > - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > - hsw_disable_pc8(dev_priv);
> > - else if (IS_SKYLAKE(dev_priv))
> > + if (IS_SKYLAKE(dev_priv))
> > ret = skl_resume_prepare(dev_priv);
> > + else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > + hsw_disable_pc8(dev_priv);
> > + else if (IS_VALLEYVIEW(dev_priv))
> > + ret = vlv_resume_prepare(dev_priv, false);
> > +
> > + if (ret)
> > + DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
>
> vlv_resume_prepare() needs to run before intel_uncore_early_sanitize(),
> as the former includes the steps to wake the HW from RC6 for the first
> time after suspend. At the time intel_resume_prepare() was removed, I
> suggested to instead split out the part from vlv_resume_prepare() that
> needs to be done early into a separate function, so that would be one
> way to go about this.
Oh, that was a carefully crafted trap! Will see what I can do.
Thankfully the active ingredients of that series don't depend on that
refactoring.
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: make link rate printing prettier
On Mon, May 18, 2015 at 04:01:45PM +0300, Jani Nikula wrote: > Turn > > [drm:intel_dp_print_rates] source rates: 162000,27,54, > [drm:intel_dp_print_rates] sink rates: 162000,27, > [drm:intel_dp_print_rates] common rates: 162000,27, > > into > > [drm:intel_dp_print_rates] source rates: 162000, 27, 54 > [drm:intel_dp_print_rates] sink rates: 162000, 27 > [drm:intel_dp_print_rates] common rates: 162000, 27 Wouldn't aligning the sink rates with the other two rates look nicer (either by right-aligning the entire text, or by just right-aligning the colon? Admittedly this is just bikeshedding, but... Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Flag the test as failing after a segfault
I will take a look and submit a test as a separate patch.
//Derek
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, May 18, 2015 4:14 PM
To: Morton, Derek J
Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas
Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core.c: Flag the test as failing
after a segfault
On Mon, May 18, 2015 at 02:37:31PM +0100, Derek Morton wrote:
> fatal_signal_handler() was trapping fatal errors but not flagging the
> test as failing or setting an exit code.
> The result was that the test would return Ok or Skipped depending on
> what the other subtests did even though one of the subtests had
> segfaulted.
>
> Signed-off-by: Derek Morton
This isn't the first trouble with our signal handler and test results. Can you
perhaps write a library unit test for this bug?
They're in lib/tests and executed with make check.
Thanks, Daniel
> ---
> lib/igt_core.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..b29f7e3
> 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1433,8 +1433,15 @@ static void fatal_sig_handler(int sig)
> igt_assert_eq(write(STDERR_FILENO, ".\n", 2), 2);
> }
>
> - if (in_subtest && crash_signal(sig))
> + if (in_subtest && crash_signal(sig)) {
> + /* Linux standard to return exit code as 128 + signal */
> + if (!failed_one)
> + igt_exitcode = 128 + sig;
> +
> + failed_one = true;
> +
> exit_subtest("CRASH");
> + }
> break;
> }
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/gem_exec_params: check invalid flags for Resource Streamer
On Tue, May 19, 2015 at 11:30:44AM +0300, Abdiel Janulgue wrote:
> Make sure resource streamer flags works only in correct ring in
> addition to checking next flag after the RS boundary fails.
>
> Cc: Daniel Vetter
> Signed-off-by: Abdiel Janulgue
> ---
> tests/gem_exec_params.c | 21 -
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
> index 54f0dc3..08ee330 100644
> --- a/tests/gem_exec_params.c
> +++ b/tests/gem_exec_params.c
> @@ -48,6 +48,7 @@
> #define LOCAL_I915_EXEC_BSD_MASK (3<<13)
> #define LOCAL_I915_EXEC_BSD_RING1 (1<<13)
> #define LOCAL_I915_EXEC_BSD_RING2 (2<<13)
> +#define LOCAL_I915_EXEC_RESOURCE_STREAMER (1<<16)
>
> struct drm_i915_gem_execbuffer2 execbuf;
> struct drm_i915_gem_exec_object2 gem_exec[1];
> @@ -220,7 +221,7 @@ igt_main
> /* HANDLE_LUT and NO_RELOC are already exercised by gem_exec_lut_handle
> */
>
> igt_subtest("invalid-flag") {
> - execbuf.flags = I915_EXEC_RENDER | (I915_EXEC_HANDLE_LUT << 1);
> + execbuf.flags = I915_EXEC_RENDER |
> (LOCAL_I915_EXEC_RESOURCE_STREAMER << 1);
> RUN_FAIL(EINVAL);
> }
>
> @@ -234,6 +235,24 @@ igt_main
> execbuf.num_cliprects = 0;
> }
>
> + igt_subtest("rs-invalid-on-bsd-ring") {
> + igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
> + execbuf.flags = I915_EXEC_BSD |
> LOCAL_I915_EXEC_RESOURCE_STREAMER;
> + RUN_FAIL(EINVAL);
> + }
> +
> + igt_subtest("rs-invalid-on-blt-ring") {
> + igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
> + execbuf.flags = I915_EXEC_BLT |
> LOCAL_I915_EXEC_RESOURCE_STREAMER;
> + RUN_FAIL(EINVAL);
> + }
> +
> + igt_subtest("rs-invalid-on-vebox-ring") {
> + igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
> + execbuf.flags = I915_EXEC_VEBOX |
> LOCAL_I915_EXEC_RESOURCE_STREAMER;
> + RUN_FAIL(EINVAL);
> + }
Please also add some checks to make sure we reject RS on pre-hsw on the
render ring. lgtm otherwise.
Cheers, Daniel
> +
> #define DIRT(name) \
> igt_subtest(#name "-dirt") { \
> execbuf.flags = 0; \
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
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 v3 1/3] drm/i915: Expose I915_EXEC_RESOURCE_STREAMER flag
On 05/18/2015 05:55 PM, Chris Wilson wrote: > On Mon, May 18, 2015 at 11:31:54AM +0300, Abdiel Janulgue wrote: >> Ensures that the batch buffer is executed by the resource streamer >> >> Signed-off-by: Abdiel Janulgue > > 1-3: > Reviewed-by: Chris Wilson > > Now all you have to do is satisfy Daniel with a few igt. > -Chris > Thanks for the review! :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Enable Resource Streamer state save/restore in HSW
On 05/19/2015 11:26 AM, Daniel Vetter wrote: > On Tue, May 19, 2015 at 09:58:52AM +0300, Abdiel Janulgue wrote: >> >> >> On 05/18/2015 07:07 PM, Ville Syrjälä wrote: >>> On Mon, May 18, 2015 at 04:41:51PM +0100, Chris Wilson wrote: On Mon, May 18, 2015 at 06:36:18PM +0300, Ville Syrjälä wrote: > On Mon, May 18, 2015 at 11:31:56AM +0300, Abdiel Janulgue wrote: >> Also clarify comments on context size that the extra state for >> Resource Streamer is included. >> >> Suggested-by: Chris Wilson >> Signed-off-by: Abdiel Janulgue >> --- >> drivers/gpu/drm/i915/i915_gem_context.c | 2 +- >> drivers/gpu/drm/i915/i915_reg.h | 3 ++- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c >> b/drivers/gpu/drm/i915/i915_gem_context.c >> index f3e84c4..1db107a 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_context.c >> +++ b/drivers/gpu/drm/i915/i915_gem_context.c >> @@ -509,7 +509,7 @@ mi_set_context(struct intel_engine_cs *ring, >> } >> >> /* These flags are for resource streamer on HSW+ */ >> -if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8) >> +if (IS_HASWELL(ring->dev)) >> flags |= (MI_SAVE_EXT_STATE_EN | >> MI_RESTORE_EXT_STATE_EN); > > I don't get it. Previously we told the hardware to save the extended > context on !hsw, and now we don't. That doesn't seem correct to me. We don't use the extended state elsewhere. >>> >>> Umm. On SNB at least 3DSTATE_POLY_STIPPLE_PATTERN seems to be part of >>> the extended state, and on IVB/VLV SOL state is there. Mesa uses all of >>> that. >>> I'd always been dubious of the origins/intentions of this line of code since it claims only to be for enabling RS on HSW... i.e. commit e80f14b6d36e3e07111cf2ab084ef8dd5d015ce2 Author: Ben Widawsky Date: Mon Aug 18 10:35:28 2014 -0700 drm/i915: Don't save/restore RS when not used was backwards. >>> >>> ? It did exactly what it said, ie. avoid setting the RS save/restore >>> bits on HSW+ while leaving the ext save/restore enabled on older >>> platforms. >>> >> >> Another option is to enable extended state save restore for both HSW and >> the older platforms so we would get both features (RS save/restore and >> Extended State Save enable)? >> >> Seems the reason for this confusion is that the we have the exact same >> bit positions in MI_SET_CONTEXT but it got renamed starting from HSW and up. > > If the bit has been renamed it might be good to add at least a new #define > with a _HSw suffix and better name, just to make it clear what's going on. > gcc will see through this and fold down the different conditions to just > one. I'll do that in the next revision and also add the igt tag you require. In the meantime, please check the igt approach I sent is the correct one. Thanks! -Abdiel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests/gem_exec_params: check invalid flags for Resource Streamer
Make sure resource streamer flags works only in correct ring in
addition to checking next flag after the RS boundary fails.
Cc: Daniel Vetter
Signed-off-by: Abdiel Janulgue
---
tests/gem_exec_params.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
index 54f0dc3..08ee330 100644
--- a/tests/gem_exec_params.c
+++ b/tests/gem_exec_params.c
@@ -48,6 +48,7 @@
#define LOCAL_I915_EXEC_BSD_MASK (3<<13)
#define LOCAL_I915_EXEC_BSD_RING1 (1<<13)
#define LOCAL_I915_EXEC_BSD_RING2 (2<<13)
+#define LOCAL_I915_EXEC_RESOURCE_STREAMER (1<<16)
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_exec_object2 gem_exec[1];
@@ -220,7 +221,7 @@ igt_main
/* HANDLE_LUT and NO_RELOC are already exercised by gem_exec_lut_handle
*/
igt_subtest("invalid-flag") {
- execbuf.flags = I915_EXEC_RENDER | (I915_EXEC_HANDLE_LUT << 1);
+ execbuf.flags = I915_EXEC_RENDER |
(LOCAL_I915_EXEC_RESOURCE_STREAMER << 1);
RUN_FAIL(EINVAL);
}
@@ -234,6 +235,24 @@ igt_main
execbuf.num_cliprects = 0;
}
+ igt_subtest("rs-invalid-on-bsd-ring") {
+ igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
+ execbuf.flags = I915_EXEC_BSD |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
+ igt_subtest("rs-invalid-on-blt-ring") {
+ igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
+ execbuf.flags = I915_EXEC_BLT |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
+ igt_subtest("rs-invalid-on-vebox-ring") {
+ igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
+ execbuf.flags = I915_EXEC_VEBOX |
LOCAL_I915_EXEC_RESOURCE_STREAMER;
+ RUN_FAIL(EINVAL);
+ }
+
#define DIRT(name) \
igt_subtest(#name "-dirt") { \
execbuf.flags = 0; \
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Enable Resource Streamer state save/restore in HSW
On Tue, May 19, 2015 at 09:58:52AM +0300, Abdiel Janulgue wrote: > > > On 05/18/2015 07:07 PM, Ville Syrjälä wrote: > > On Mon, May 18, 2015 at 04:41:51PM +0100, Chris Wilson wrote: > >> On Mon, May 18, 2015 at 06:36:18PM +0300, Ville Syrjälä wrote: > >>> On Mon, May 18, 2015 at 11:31:56AM +0300, Abdiel Janulgue wrote: > Also clarify comments on context size that the extra state for > Resource Streamer is included. > > Suggested-by: Chris Wilson > Signed-off-by: Abdiel Janulgue > --- > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index f3e84c4..1db107a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -509,7 +509,7 @@ mi_set_context(struct intel_engine_cs *ring, > } > > /* These flags are for resource streamer on HSW+ */ > -if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8) > +if (IS_HASWELL(ring->dev)) > flags |= (MI_SAVE_EXT_STATE_EN | > MI_RESTORE_EXT_STATE_EN); > >>> > >>> I don't get it. Previously we told the hardware to save the extended > >>> context on !hsw, and now we don't. That doesn't seem correct to me. > >> > >> We don't use the extended state elsewhere. > > > > Umm. On SNB at least 3DSTATE_POLY_STIPPLE_PATTERN seems to be part of > > the extended state, and on IVB/VLV SOL state is there. Mesa uses all of > > that. > > > >> I'd always been dubious of > >> the origins/intentions of this line of code since it claims only to be > >> for enabling RS on HSW... > >> > >> i.e. commit e80f14b6d36e3e07111cf2ab084ef8dd5d015ce2 > >> Author: Ben Widawsky > >> Date: Mon Aug 18 10:35:28 2014 -0700 > >> > >> drm/i915: Don't save/restore RS when not used > >> > >> was backwards. > > > > ? It did exactly what it said, ie. avoid setting the RS save/restore > > bits on HSW+ while leaving the ext save/restore enabled on older > > platforms. > > > > Another option is to enable extended state save restore for both HSW and > the older platforms so we would get both features (RS save/restore and > Extended State Save enable)? > > Seems the reason for this confusion is that the we have the exact same > bit positions in MI_SET_CONTEXT but it got renamed starting from HSW and up. If the bit has been renamed it might be good to add at least a new #define with a _HSw suffix and better name, just to make it clear what's going on. gcc will see through this and fold down the different conditions to just one. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 08/12] drm/i915: Add NV12 support to intel_framebuffer_init
On Sun, May 17, 2015 at 10:11:01PM -0700, Chandra Konduru wrote:
> This patch adds NV12 as supported format to
> intel_framebuffer_init and performs various checks.
>
> Signed-off-by: Chandra Konduru
> Testcase: igt/kms_nv12
> ---
> drivers/gpu/drm/i915/intel_display.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 42924a6..41cd26f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14043,6 +14043,33 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> return -EINVAL;
> }
> break;
> + case DRM_FORMAT_NV12:
> + if (INTEL_INFO(dev)->gen < 9) {
> + DRM_DEBUG("unsupported pixel format: %s\n",
> + drm_get_format_name(mode_cmd->pixel_format));
> + return -EINVAL;
> + }
> + if (!mode_cmd->offsets[1]) {
> + DRM_DEBUG("uv start offset not set\n");
> + return -EINVAL;
> + }
Nope. It's perfectly ok to have NV12 with a 0 offset for the uv plane, if
it's e.g. in a separate buffer object. Which is the part this series seems
to be completely missing - there's no code at all to look up (and store in
intel_framebuffer the 2nd i915_bo pointer) the 2nd buffer handle afaics.
You should also change your igts to use 2 separate buffers, just for test
coverage.
-Daniel
> + if (mode_cmd->pitches[0] != mode_cmd->pitches[1] ||
> + mode_cmd->handles[0] != mode_cmd->handles[1]) {
> + DRM_DEBUG("y and uv subplanes have different
> parameters\n");
> + return -EINVAL;
> + }
> + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> + (mode_cmd->offsets[1] & 0xFFF)) {
> + DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new
> tile-row\n",
> + mode_cmd->offsets[1]);
> + return -EINVAL;
> + }
> + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
> + ((mode_cmd->offsets[1] % mode_cmd->pitches[1]) % 4)) {
> + DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line
> aligned\n",
> + mode_cmd->offsets[1]);
> + }
> + break;
> default:
> DRM_DEBUG("unsupported pixel format: %s\n",
> drm_get_format_name(mode_cmd->pixel_format));
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] DC6 already programmed to be disabled
On Tue, May 19, 2015 at 04:53:16AM +, Kamath, Sunil wrote: > Sure Damien. We will come back with solution for the same. Please check out my reply to Animesh' patch in https://tango.freedesktop.org/patch/49084/ I think this is the proper fix and the design much more in-line with other parts of i915. -Daniel > > - Sunil > > >-Original Message- > >From: Lespiau, Damien > >Sent: Monday, May 18, 2015 4:14 PM > >To: Daniel Vetter > >Cc: Konduru, Chandra; intel-gfx@lists.freedesktop.org; Shah, Suketu J; > >Kamath, Sunil; Manna, Animesh > >Subject: Re: [Intel-gfx] DC6 already programmed to be disabled > > > >On Mon, May 18, 2015 at 10:38:03AM +0200, Daniel Vetter wrote: > >> On Fri, May 15, 2015 at 11:22:27PM +, Konduru, Chandra wrote: > >> > Hi, > >> > I have been seeing below warning on skylake system on which dmc fw isn't > >> > placed. > >> > Is below warning expected? If so what is it conveying? > >> > >> Seems to be another fallout from the current design of how we prevent > >> dc5/6 when the firmware is not (yet) loaded. I've detailed how this > >> should be fixed. We need to prevent the rpm code from ever trying to > >> shut down that specific power well instead of just not obeying the > >> request. Not obeying the request means the rpm code is out of sync > >> with reality, leading to WARN_ON fun like the one you've hit here. > > > >Hey all, > > > >Would anyone of you (Sunil, Animesh, Suketu) have time to fix this? (the > >warning when DMC firmware isn't there). We should be able to work when > >failing to load the DMC firmware. > > > >What Daniel says is not quite accurate, bear in mind we still can shut down > >all power wells and do PC10 with screens off when the DMC isn't loaded. We > >could also decide to disable run-time PM entirely when the >DMC firmware > >isn't there. That's something that can be fixed later on though, right now > >the most immediate issue is not to dump lots of warnings when failing to > >load the firmware. > > > >By default I'll fix it, I have this on my TODO list, it's quite low though. > > > >Thanks, > > > >-- > >Damien -- Daniel Vetter Software Engineer, Intel Corporation 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 16/17] drm/i915: Use crtc->hwmode for vblanks.
On Tue, May 19, 2015 at 08:10:46AM +0200, Maarten Lankhorst wrote: > Op 18-05-15 om 18:28 schreef Ville Syrjälä: > > On Mon, May 18, 2015 at 05:49:23PM +0200, Daniel Vetter wrote: > >> On Wed, May 13, 2015 at 10:23:46PM +0200, Maarten Lankhorst wrote: > >>> intel_crtc->config will be removed eventually, so use crtc->hwmode. > >>> drm_atomic_helper_update_legacy_modeset_state updates hwmode, > >>> but crtc->active will eventually be gone too. Set dotclock to zero > >>> to indicate the crtc is inactive. > >>> > >>> Signed-off-by: Maarten Lankhorst > >> I think adding a code comment to our assignment of crtc->hw_mode that we > >> need this for i915_get_vblank_timestamp (and only for that) would be > >> really good. Especially since I can't find it with a quick grep, at least > >> in current upstream ;-) > > I don't particularly like resurrecting this zombie. Why we can't just use > > crtc->state->adjusted_mode (or wherever the current adjusted mode is kept)? > > > Because we want to get rid of intel_crtc->config, and if drm_atomic_swap_state > is moved to be done before any call to then crtc->state->adjusted_mode will > not > be in sync with the hw state, and any wai tfor vblank will produce funny > results. > > Since I don't think you should want to pass a state to vblank you would have > to use > some crtc local variable somewhere, in this case I chose to use hwmode for > that. I guess plan be could be to the required values (and only those we need, not the entire mode struct) in struct intel_crtc. Not sure whether that's worth the bother. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 11/17] drm/i915: Use global atomic state for staged pll config
On Mon, May 18, 2015 at 06:27:51PM +0200, Maarten Lankhorst wrote:
> Op 18-05-15 om 17:45 schreef Daniel Vetter:
> > On Wed, May 13, 2015 at 10:23:41PM +0200, Maarten Lankhorst wrote:
> >> From: Ander Conselvan de Oliveira
> >>
> >> Now that we can subclass drm_atomic_state we can also use it to keep
> >> track of all the pll settings. atomic_state is a better place to hold
> >> all shared state than keeping pll->new_config everywhere.
> >>
> >> Signed-off-by: Maarten Lankhorst
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 1 -
> >> drivers/gpu/drm/i915/intel_atomic.c | 49
> >> drivers/gpu/drm/i915/intel_display.c | 111
> >> +++
> >> drivers/gpu/drm/i915/intel_drv.h | 13
> >> 4 files changed, 95 insertions(+), 79 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> b/drivers/gpu/drm/i915/i915_drv.h
> >> index a0e4868653f2..0e200018c9aa 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -319,7 +319,6 @@ struct intel_shared_dpll_config {
> >>
> >> struct intel_shared_dpll {
> >>struct intel_shared_dpll_config config;
> >> - struct intel_shared_dpll_config *new_config;
> >>
> >>int active; /* count of number of active CRTCs (i.e. DPMS on) */
> >>bool on; /* is the PLL actually active? Disabled during modeset */
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> >> b/drivers/gpu/drm/i915/intel_atomic.c
> >> index 7ed8033aae60..aff87054406c 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -421,3 +421,52 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
> >>
> >>return 0;
> >> }
> >> +
> >> +static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
> >> +struct intel_atomic_state *state)
> >> +{
> >> + struct drm_i915_private *dev_priv = to_i915(dev);
> >> + struct intel_shared_dpll *pll;
> >> + enum intel_dpll_id i;
> >> +
> >> + state->dpll_set = true;
> > The ww mutex locking dance here is missing. For simplicity I think we
> > could just repurpose the dev->mode_config.connection_mutex. We need that
> > anyway full modesets, and dpll changes should only be required in those.
> > Which means this wont introduce any unecessary parallelism.
> >
> > It means though that we need to wire up a proper error code to all callers
> > of duplicate/get_dpll_state, like with all the other atomic states. Might
> > be best to follow the design for connector/crtc/planes an pass around
> > pointers with error codes explicitly (instead of returning
> > state->shared_dpll below since that can only cope with NULL and not with
> > -EDEADLK).
> >
> > Sorry that I didn't spot this earlier.
> >
> The dpll state should only ever be done during a modeset in which case the
> connection_mutex is guaranteed to be held.
> Instead of making this return a value can we add a lockdep_assert_held ?
The problem is that the atomic core might only grab the crtc state and
crtc mutex if e.g. userspace only changes the mode. But I've forgotten
that we're using the helpers to handle modesets, and those will acquire
all the needed connector states and hence the connection_mutex.
A locking check is therefore indeed enough. But please use
WARN_ON(!mutex_is_locked) since imo a locking check which can be compiled
out is useless. And the additional correctness lockdep provides isn't
needed - we'll catch the bug since no one always hits the race by doing
modesets in parallel ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable
On Mon, May 18, 2015 at 06:35:59PM +0200, Maarten Lankhorst wrote:
> Op 18-05-15 om 17:30 schreef Daniel Vetter:
> > On Wed, May 13, 2015 at 10:23:37PM +0200, Maarten Lankhorst wrote:
> >> crtc_state->enable means a crtc is configured, but it may be turned
> >> off for dpms. Until the previous commit crtc_state->active was not
> >> updated on crtc off, but now that we do we should use that for tracking
> >> whether a crtc is scanning out or not.
> >>
> >> At this point crtc->active should mirror crtc_state->active,
> >> so some paranoia from the crtc_disable functions can be removed.
> >>
> >> Note that intel_set_mode_setup_plls still checks for ->enable,
> >> because all resources that are needed have to be calculated, so
> >> dpms changes will still succeed.
> >>
> >> Signed-off-by: Maarten Lankhorst
> > A few detail comments below.
> > -Daniel
> >
> >> ---
> >> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> >> drivers/gpu/drm/i915/intel_display.c | 44
> >> ++--
> >> 2 files changed, 23 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> >> b/drivers/gpu/drm/i915/i915_irq.c
> >> index 557af8877a2e..ca457317a8ac 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device
> >> *dev, int pipe,
> >>return -EINVAL;
> >>}
> >>
> >> - if (!crtc->state->enable) {
> >> + if (!crtc->state->active) {
> > This change looks unjustified I think.
> Why? Can you get a vblank on crtc that is dpms off? I think not..
>
> Either way later on I use hwmode->crtc_clock which renders this moot.
> >>
> >> @@ -5742,7 +5738,7 @@ static int valleyview_modeset_global_pipes(struct
> >> drm_atomic_state *state)
> >>
> >>/* add all active pipes to the state */
> >>for_each_crtc(state->dev, crtc) {
> >> - if (!crtc->state->enable)
> >> + if (!crtc->state->active)
> > This is a functional change to the cdclk code and might break it and/or
> > conflict with the ongoing cdclk work from Ville/Mika. Definitely needs to
> > be split out.
> No this function just sets mode_changed on all active crtc's.
>
> This is done to turn them off while changing the clock. In case they're
> already turned off you don't have to turn them off.
>
> >>continue;
> >>
> >>crtc_state = drm_atomic_get_crtc_state(state, crtc);
> >> @@ -5752,7 +5748,7 @@ static int valleyview_modeset_global_pipes(struct
> >> drm_atomic_state *state)
> >>
> >>/* disable/enable all currently active pipes while we change cdclk */
> >>for_each_crtc_in_state(state, crtc, crtc_state, i)
> >> - if (crtc_state->enable)
> >> + if (crtc_state->active)
> >>crtc_state->mode_changed = true;
> > Same here.
> >
> > Hm, aside of all that maybe we should drop vlv_modeset_global_pipes and
> > instead just look at crtc_state->mode_changed? That way we don't need to
> > duplicate the same checks twice, once to set ->mode_changed and once to
> > for the prepare_pipes mask. Or is that duplication already getting
> > removed?
> I think we could get rid of a lot of those extra loops if we want to later,
> but for now readability is more important.
Hm makes sense with your replies - there's a few cases indeed where we
need to switch from checking ->enable to ->active if we start using the
mode_set machinery for dpms. I think it'd be good to explain that a bit in
the commit message for all the different cases, so that reviewers can go
hunting for more and make sure there aren't ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 0/2] strace/drm: Add i915 ioctls to strace
On Wed, May 13, 2015 at 01:10:17AM +0300, Dmitry V. Levin wrote: > On Tue, May 12, 2015 at 07:37:59PM +0200, Gabriel Laskar wrote: > > On Tue, 12 May 2015 14:35:28 +0200, Patrik Jakobsson wrote: > > > On Mon, May 11, 2015 at 08:08:19PM +0200, Gabriel Laskar wrote: > > > > On Mon, 11 May 2015 15:54:24 +0200, Patrik Jakobsson wrote: > > > > > On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote: > > > > > > On Wed, 6 May 2015 16:48:01 +0200, Patrik Jakobsson wrote: > > > > > > > > > > > > > This patch set aims to make strace more useful when tracing i915 > > > > > > > ioctls. > > > > > > > The ioctl type is first checked for being drm and then the driver > > > > > > > backing the opened device is identified by looking at sysfs. Other > > > > > > > drivers than i915 can easily be added. > > > > > > > > > > > > > > Only a subset of the i915 ioctls are included. I will extend this > > > > > > > patch > > > > > > > set if the approach looks ok. The generic drm ioctls are also > > > > > > > missing. > > > > > > > > > > > > > > Give it a spin with: > > > > > > > strace -e trace=ioctl -p `pidof X` > > > > > > > > > > > > > > Patrik Jakobsson (2): > > > > > > > strace/drm: Print extended info for drm and i915 ioctls > > > > > > > strace/drm: Print args for most common i915 ioctls > > > > > > > > > > > > > > Makefile.am| 2 + > > > > > > > defs.h | 2 + > > > > > > > drm.c | 104 + > > > > > > > drm_i915.c | 278 > > > > > > > + > > > > > > > ioctl.c| 5 + > > > > > > > xlat/drm_i915_getparams.in | 28 + > > > > > > > xlat/drm_i915_ioctls.in| 51 + > > > > > > > xlat/drm_i915_setparams.in | 4 + > > > > > > > 8 files changed, 474 insertions(+) > > > > > > > create mode 100644 drm.c > > > > > > > create mode 100644 drm_i915.c > > > > > > > create mode 100644 xlat/drm_i915_getparams.in > > > > > > > create mode 100644 xlat/drm_i915_ioctls.in > > > > > > > create mode 100644 xlat/drm_i915_setparams.in > > > > > > > > > > > > > > > > > > > This is a great start! We need this kind of decoding. Do you plan to > > > > > > add also the generic drm ioctl decoding? > > > > > > > > > > Thanks for the review. Yes, my plan is to add generic drm ioctls as > > > > > well. > > > > > > > > > > > > > > > > > Some issues though: > > > > > > > > > > > > * The way you avoid the ioctl request decoding is quite hard to > > > > > > follow, > > > > > > but it seems that you don't have much of a choice, except that in > > > > > > drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It > > > > > > seems it > > > > > > needs some work here, to allow a simpler code path. Maybe this > > > > > > would > > > > > > be clearer if the decoding/drm_get_driver_name, etc… was in > > > > > > ioctl_decode_command_number(). Also, with the actual code, if you > > > > > > are > > > > > > on i915 with an invalid ioctl number, it will be printed as > > > > > > "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) > > > > > > This > > > > > > will also add an inconsistent result depending whether /sys is > > > > > > mounted or not. > > > > > > > > > > Yes, moving it to ioctl_decode_command_number() makes sense. I'll do > > > > > that. > > > > > And I'll make the output consistent and skip the I915_IOCTL_???. It > > > > > comes with > > > > > the drawback of possibly duplicated entries when doing the lookup > > > > > even though we > > > > > know we're talking to i915, but it is still nicer than _???. > > > > > > > > If you call all the request decoding code from > > > > ioctl_decode_command_number() you will still be able to determine if > > > > you are on i915, and write the correct request, but the fallback code > > > > will be no longer duplicated. You will have to call xlookup() instead of > > > > printxval(), in order to be able to know when the decoding fail though. > > > > > > Unfortunately I cannot check for i915 in _decode_command_number since I > > > don't > > > have the full tcb context (cannot figure out the path, etc.). That's > > > probably > > > why I had to duplicate the decoding in the first place. The only other > > > solution > > > I see is to detect i915 early and store it globally somewhere but that is > > > rather > > > nasty. Not sure how to do this in a better way. What am I missing? > > > > Oh yes, I see. We can add tcb context to ioctl_decode_command_number(), > > it should not be a problem. We didn't have the need for it for the > > moment, that's all. > > > > Dmitry? What do you think of this? > > I had no chance to have a look at the code, but anyway, passing > tcb context down to ioctl_decode_command_number() shouldn't be a problem > at all. Sounds good. Then I'll use ioctl_decode_command_number as the dispatcher. Thanks Patrik > > > -- > ldv _
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/bxt: Move around lane stagger calculation
On Mon, May 18, 2015 at 07:28:04PM +0300, Imre Deak wrote:
> On ke, 2015-05-13 at 12:20 +0530, Vandana Kannan wrote:
> > Making lane stagger calculation common for HDMI and DP
> >
> > v2: Imre's comments addressed
> > - Remove lane stagger from bxt_clk_div and make it a local variable in
> > ddi_pll_select
> >
> > Signed-off-by: Vandana Kannan
>
> Reviewed-by: Imre Deak
Both patches applied, thanks.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 40
> >
> > 1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index a56613c..aadd29e 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1333,18 +1333,17 @@ struct bxt_clk_div {
> > uint32_t m2_frac;
> > bool m2_frac_en;
> > uint32_t n;
> > - uint32_t lanestagger;
> > };
> >
> > /* pre-calculated values for DP linkrates */
> > static struct bxt_clk_div bxt_dp_clk_val[7] = {
> > - /* 162 */ {4, 2, 32, 1677722, 1, 1, 0xd},
> > - /* 270 */ {4, 1, 27, 0, 0, 1, 0xd},
> > - /* 540 */ {2, 1, 27, 0, 0, 1, 0x18},
> > - /* 216 */ {3, 2, 32, 1677722, 1, 1, 0xd},
> > - /* 243 */ {4, 1, 24, 1258291, 1, 1, 0xd},
> > - /* 324 */ {4, 1, 32, 1677722, 1, 1, 0x18},
> > - /* 432 */ {3, 1, 32, 1677722, 1, 1, 0x18}
> > + /* 162 */ {4, 2, 32, 1677722, 1, 1},
> > + /* 270 */ {4, 1, 27, 0, 0, 1},
> > + /* 540 */ {2, 1, 27, 0, 0, 1},
> > + /* 216 */ {3, 2, 32, 1677722, 1, 1},
> > + /* 243 */ {4, 1, 24, 1258291, 1, 1},
> > + /* 324 */ {4, 1, 32, 1677722, 1, 1},
> > + /* 432 */ {3, 1, 32, 1677722, 1, 1}
> > };
> >
> > static bool
> > @@ -1357,7 +1356,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> > struct bxt_clk_div clk_div = {0};
> > int vco = 0;
> > uint32_t prop_coef, int_coef, gain_ctl, targ_cnt;
> > - uint32_t dcoampovr_en_h, dco_amp;
> > + uint32_t dcoampovr_en_h, dco_amp, lanestagger;
> >
> > if (intel_encoder->type == INTEL_OUTPUT_HDMI) {
> > intel_clock_t best_clock;
> > @@ -1382,16 +1381,6 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> > clk_div.m2_frac_en = clk_div.m2_frac != 0;
> >
> > vco = best_clock.vco;
> > - if (clock > 27)
> > - clk_div.lanestagger = 0x18;
> > - else if (clock > 135000)
> > - clk_div.lanestagger = 0x0d;
> > - else if (clock > 67000)
> > - clk_div.lanestagger = 0x07;
> > - else if (clock > 33000)
> > - clk_div.lanestagger = 0x04;
> > - else
> > - clk_div.lanestagger = 0x02;
> > } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> > intel_encoder->type == INTEL_OUTPUT_EDP) {
> > struct drm_encoder *encoder = &intel_encoder->base;
> > @@ -1439,6 +1428,17 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> > return false;
> > }
> >
> > + if (clock > 27)
> > + lanestagger = 0x18;
> > + else if (clock > 135000)
> > + lanestagger = 0x0d;
> > + else if (clock > 67000)
> > + lanestagger = 0x07;
> > + else if (clock > 33000)
> > + lanestagger = 0x04;
> > + else
> > + lanestagger = 0x02;
> > +
> > crtc_state->dpll_hw_state.ebb0 =
> > PORT_PLL_P1(clk_div.p1) | PORT_PLL_P2(clk_div.p2);
> > crtc_state->dpll_hw_state.pll0 = clk_div.m2_int;
> > @@ -1462,7 +1462,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> > crtc_state->dpll_hw_state.pll10 |= PORT_PLL_DCO_AMP(dco_amp);
> >
> > crtc_state->dpll_hw_state.pcsdw12 =
> > - LANESTAGGER_STRAP_OVRD | clk_div.lanestagger;
> > + LANESTAGGER_STRAP_OVRD | lanestagger;
> >
> > pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> > if (pll == NULL) {
>
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Don't expose RGB/BGR 8888 formats on primary planes before SKL
On Fri, May 15, 2015 at 08:15:42PM +0100, Damien Lespiau wrote:
> We don't actually do anything different for the A version of the
> RGB formats before SKL. Don't let user space think we can support alpha
> blending.
>
> v2: Fix the logic to forbid the creation ABGR2101010 fbs (Ville)
>
> Reviewed-by: Ville Syrjälä
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/intel_display.c | 35 ---
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5ebac76..f75505e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -52,7 +52,6 @@ static const uint32_t i8xx_primary_formats[] = {
> DRM_FORMAT_XRGB1555,
> DRM_FORMAT_ARGB1555,
> DRM_FORMAT_XRGB,
> - DRM_FORMAT_ARGB,
> };
>
> /* Primary plane formats for gen >= 4 */
> @@ -61,6 +60,15 @@ static const uint32_t i965_primary_formats[] = {
> DRM_FORMAT_RGB565,
> DRM_FORMAT_XRGB,
> DRM_FORMAT_XBGR,
> + DRM_FORMAT_XRGB2101010,
> + DRM_FORMAT_XBGR2101010,
> +};
> +
> +static const uint32_t skl_primary_formats[] = {
> + DRM_FORMAT_C8,
> + DRM_FORMAT_RGB565,
> + DRM_FORMAT_XRGB,
> + DRM_FORMAT_XBGR,
> DRM_FORMAT_ARGB,
> DRM_FORMAT_ABGR,
> DRM_FORMAT_XRGB2101010,
> @@ -2706,11 +2714,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> dspcntr |= DISPPLANE_BGRX565;
> break;
> case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> dspcntr |= DISPPLANE_BGRX888;
> break;
> case DRM_FORMAT_XBGR:
> - case DRM_FORMAT_ABGR:
> dspcntr |= DISPPLANE_RGBX888;
> break;
> case DRM_FORMAT_XRGB2101010:
> @@ -2812,11 +2818,9 @@ static void ironlake_update_primary_plane(struct
> drm_crtc *crtc,
> dspcntr |= DISPPLANE_BGRX565;
> break;
> case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> dspcntr |= DISPPLANE_BGRX888;
> break;
> case DRM_FORMAT_XBGR:
> - case DRM_FORMAT_ABGR:
> dspcntr |= DISPPLANE_RGBX888;
> break;
> case DRM_FORMAT_XRGB2101010:
> @@ -13278,12 +13282,15 @@ static struct drm_plane
> *intel_primary_plane_create(struct drm_device *dev,
> if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
> primary->plane = !pipe;
>
> - if (INTEL_INFO(dev)->gen <= 3) {
> - intel_primary_formats = i8xx_primary_formats;
> - num_formats = ARRAY_SIZE(i8xx_primary_formats);
> - } else {
> + if (INTEL_INFO(dev)->gen >= 9) {
> + intel_primary_formats = skl_primary_formats;
> + num_formats = ARRAY_SIZE(skl_primary_formats);
> + } else if (INTEL_INFO(dev)->gen >= 4) {
> intel_primary_formats = i965_primary_formats;
> num_formats = ARRAY_SIZE(i965_primary_formats);
> + } else {
> + intel_primary_formats = i8xx_primary_formats;
> + num_formats = ARRAY_SIZE(i8xx_primary_formats);
> }
>
> drm_universal_plane_init(dev, &primary->base, 0,
> @@ -13961,7 +13968,6 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> case DRM_FORMAT_C8:
> case DRM_FORMAT_RGB565:
> case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> break;
> case DRM_FORMAT_XRGB1555:
> case DRM_FORMAT_ARGB1555:
> @@ -13971,8 +13977,15 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> return -EINVAL;
> }
> break;
> - case DRM_FORMAT_XBGR:
> + case DRM_FORMAT_ARGB:
> case DRM_FORMAT_ABGR:
> + if (!IS_VALLEYVIEW(dev) && INTEL_INFO(dev)->gen < 9) {
> + DRM_DEBUG("unsupported pixel format: %s\n",
> + drm_get_format_name(mode_cmd->pixel_format));
> + return -EINVAL;
> + }
> + break;
> + case DRM_FORMAT_XBGR:
These two hunks break cursor support, since with universal planes we
really want to be able to create rgba for the cursor ...
I dropped the patch meanwhile, can you please resend?
Thanks, Daniel
> case DRM_FORMAT_XRGB2101010:
> case DRM_FORMAT_XBGR2101010:
> if (INTEL_INFO(dev)->gen < 4) {
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Breakage for Ironlake due to some watermarks changes in Linux 4.0+?
On Tue, 19 May 2015, Mario Kleiner wrote: > On 05/15/2015 11:00 AM, Jani Nikula wrote: >> On Fri, 15 May 2015, Mario Kleiner wrote: >>> Hi all, >>> >>> since Linux 4.0 i experience some massive display flicker problem on my >>> Intel HD Ironlake mobile (2010 MacBookPro6,2) under Waylands reference >>> compositor Weston. >>> >>> - Only happens on Linux >= 4.0 on intel-kms with the Intel HD, not under >>> nouveau-kms with the discrete NVidia gpu. Strangely on Linux 4.1-rc it >>> happens all the time, whereas on Linux 4.0 it can work normally for >>> quite a while, but once the problem starts only a reboot can cure it. >>> >>> - Almost only happens on Weston, but only very rarely under the XServer. >>> VT switching from Weston to XOrg makes the problem disappear, switching >>> back to Weston and it starts again immediately. >>> >>> - Only happens if a hardware cursor is displayed - hiding the cursor >>> stops the flicker immediately, showing the cursor starts the flicker. >>> >>> - The drm and desktop is completely idle during this - drm.debug=15 >>> shows no activity while this happens. >>> >>> Symptom: >>> >>> Up to the scanline where the cursor is located, the desktop image is >>> displayed, but jumps horizontally left and right by some random number >>> of pixels, maybe in the range 0 - 200 pixels with high frequency, making >>> the content unreadable. Starting with the scanline where scanout of the >>> cursor starts, the display goes blank, as if some display controller >>> fifo would underflow and the controller blanks the display in response. >>> Seems having to scanout the cursor plane in addition to the primary >>> plane is just enough to push it over some limit? >>> >>> I also see cpu and pch pipe a fifo underruns reported by the underflow >>> irq handlers. >>> >>> I saw there were many changes around Linux 4.0 in the kms driver wrt. >>> watermark calculations, so this might be related? >> >> Please try http://patchwork.freedesktop.org/patch/49314 and report back. >> >> BR, >> Jani. >> > > The patch fixes my flicker problem nicely. Thanks! If you want, you can > add a > > Tested-by: Mario Kleiner Thanks for testing, I've pushed the fix to drm-intel-fixes, headed to -rc5. BR, Jani. > > best, > -mario -- 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: fix screen flickering
On Thu, 14 May 2015, Matt Roper wrote:
> On Thu, May 14, 2015 at 09:16:39AM +0200, Thomas Gummerer wrote:
>> Commit c9f038a1a592 ("drm/i915: Don't assume primary & cursor are
>> always on for wm calculation (v4)") fixes a null pointer dereference.
>> Setting the primary and cursor panes to false in
>> ilk_compute_wm_parameters to false does however give the following
>> errors in the kernel log and causes the screen to flicker.
>>
>> [ 101.133716] [drm:intel_set_cpu_fifo_underrun_reporting [i915]]
>> *ERROR* uncleared fifo underrun on pipe A
>> [ 101.133725] [drm:intel_cpu_fifo_underrun_irq_handler [i915]]
>> *ERROR* CPU pipe A FIFO underrun
>>
>> Always setting the panes to enabled fixes this error.
>>
>> Helped-by: Matt Roper
>> Signed-off-by: Thomas Gummerer
>
> Seems like a reasonable short-term workaround and returns us to how the
> code used to behaves.
>
> Reviewed-by: Matt Roper
Pushed to drm-intel-fixes, thanks for the patch and review.
BR,
Jani.
>
>> ---
>>
>> > Sorry, I missed your patch when you first sent it. That type of fix
>> > looks like an okay workaround while we do a more in-depth rework of the
>> > watermark system. However I think your patch could cause a crash if we
>> > disable the primary plane via the universal plane interface; if we do
>> > that, p->pri.bytes_per_pixel is set to 0, but since we're now pretending
>> > the primary plane is always enabled, ilk_wm_fbc() can eventually get
>> > called and use that 0 in the denominator of a division operation.
>> >
>> > If you just squash the following change into your patch, I think it should
>> > be
>> > safe:
>> > [...]
>>
>> Thank you very much for the suggestion, here is an updated version of the
>> patch.
>>
>> drivers/gpu/drm/i915/intel_pm.c | 24 +++-
>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index fa4ccb3..555b896 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2045,22 +2045,20 @@ static void ilk_compute_wm_parameters(struct
>> drm_crtc *crtc,
>> p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
>> p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
>>
>> -if (crtc->primary->state->fb) {
>> -p->pri.enabled = true;
>> +if (crtc->primary->state->fb)
>> p->pri.bytes_per_pixel =
>> crtc->primary->state->fb->bits_per_pixel / 8;
>> -} else {
>> -p->pri.enabled = false;
>> -p->pri.bytes_per_pixel = 0;
>> -}
>> +else
>> +p->pri.bytes_per_pixel = 4;
>> +
>> +p->cur.bytes_per_pixel = 4;
>> +/*
>> + * TODO: for now, assume primary and cursor planes are always enabled.
>> + * Setting them to false makes the screen flicker.
>> + */
>> +p->pri.enabled = true;
>> +p->cur.enabled = true;
>>
>> -if (crtc->cursor->state->fb) {
>> -p->cur.enabled = true;
>> -p->cur.bytes_per_pixel = 4;
>> -} else {
>> -p->cur.enabled = false;
>> -p->cur.bytes_per_pixel = 0;
>> -}
>> p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
>> p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
>>
>> --
>> 2.4.0.184.g8e1974e
>>
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
