Re: [Intel-gfx] [PATCH] Revert "drm/i915/bdw: BDW Software Turbo"
On Mon, Sep 29, 2014 at 12:46:25PM -0700, Jesse Barnes wrote: > On Mon, 29 Sep 2014 09:52:38 -0700 > Rodrigo Vivi wrote: > > > On Mon, Sep 29, 2014 at 9:38 AM, Daniel Vetter wrote: > > > On Mon, Sep 29, 2014 at 08:48:53AM -0700, Jesse Barnes wrote: > > >> On Mon, 29 Sep 2014 15:11:51 +0200 > > >> Daniel Vetter wrote: > > >> > > >> > This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca. > > >> > > > >> > It's apparently too broken so that Rodrigo submitted a patch to add a > > >> > config option for it. Given that the design is also ... suboptimal and > > >> > that I've only merged this to get lead engineers and managers off my > > >> > back for one second let's just revert this. > > >> > > > >> > /me puts on combat gear again > > >> > > > >> > It was worth a shot ... > > >> > > >> I thought we had a fix for the runtime PM issue this created? And > > >> Rodrigo's fix is just a simple "only BDW needs this" patch, so I guess > > >> I don't see the big issue? > > > > Well, actually the issue is to stay with high busted gt frequency even on > > idle. > > Or this or an incompatibility with the actual rps interface... > > > > So my patch was actually a protect a feture under development with > > parameter. > > > > >> > > >> Or is there another bug you didn't mention in the s-o-b section you're > > >> worried about? > > > > The only issue I know is: > > References: https://bugs.freedesktop.org/show_bug.cgi?id=77869 > > Ah yeah, misread it thinking it was accidentally being enabled on > non-bdw too or something. > > But yeah I expect turbo to be broken even with your patch, since we > won't ramp up/down the freq (or at least not properly). The only thing > Daisy's patch addresses is making sure we at least try to ramp if we're > getting regular page flips. The more complete fix (which should > address the test failure too) would be to peroidically poll the hw busy > state and adjust the freq based on that, independent of the flip > frequency. That should allow GPGPU apps, offlien media encode/decode, > and these tests to work as they should, but it's a little more work. > We need to make sure the timer is shut down when we go into RC6 and > only fires frequently when the GPU is busy. > I object to reverting Daisy's "BDW Software Turbo" patch without also having an acceptable replacement. Yes, Daisy's patch is not a complete fix. Yes, a different design may have been better. But this patch did get turbo working on BDW systems with regular flips (such as those running Android). Taking this patch out without a replacement is a step in the wrong direction. -- Tom O'Rourke ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2 v3] drm/i915: intel_backlight scale() math WA
Improper truncated integer division in the scale() function causes
actual_brightness != brightness. This (partial) work-around should be
sufficient for a majority of use-cases, but it is by no means a complete
solution.
TODO: Determine how best to scale "user" values to "hw" values, and
vice-versa, when the ranges are of different sizes. That would be a
buggy scenario even with this work-around.
The issue was introduced in the following (v3.17-rc1) commit:
6dda730 drm/i915: respect the VBT minimum backlight brightness
v2: (thanks to Chris Wilson) clarify commit message, use rounded division
macro
v3: -DIV_ROUND_CLOSEST() fails to build with CONFIG_X86_32=y. (Jani)
-Use DIV_ROUND_CLOSEST_ULL() instead. (Damien)
-v1 and v2 originally authored by Joe Konno.
Signed-off-by: U. Artie Eoff
---
drivers/gpu/drm/i915/intel_panel.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index f17ada3..f7da913 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -398,6 +398,9 @@ intel_panel_detect(struct drm_device *dev)
}
}
+#define DIV_ROUND_CLOSEST_ULL(ll, d) \
+({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
+
/**
* scale - scale values from one range to another
*
@@ -419,9 +422,8 @@ static uint32_t scale(uint32_t source_val,
source_val = clamp(source_val, source_min, source_max);
/* avoid overflows */
- target_val = (uint64_t)(source_val - source_min) *
- (target_max - target_min);
- do_div(target_val, source_max - source_min);
+ target_val = DIV_ROUND_CLOSEST_ULL((uint64_t)(source_val - source_min) *
+ (target_max - target_min), source_max - source_min);
target_val += target_min;
return target_val;
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Move DIV_ROUND_CLOSEST_ULL macro to header
Move the duplicated DIV_ROUND_CLOSEST_ULL macro into the intel_drv.h
header file so that it can be shared between intel_display.c
and intel_panel.c.
Signed-off-by: U. Artie Eoff
---
drivers/gpu/drm/i915/intel_display.c | 3 ---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_panel.c | 3 ---
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5b05ddb..db800f2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -73,9 +73,6 @@ static const uint32_t intel_cursor_formats[] = {
DRM_FORMAT_ARGB,
};
-#define DIV_ROUND_CLOSEST_ULL(ll, d) \
-({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
-
static void intel_increase_pllclock(struct drm_device *dev,
enum pipe pipe);
static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1b72c15..8401ae3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -36,6 +36,9 @@
#include
#include
+#define DIV_ROUND_CLOSEST_ULL(ll, d) \
+({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
+
/**
* _wait_for - magic (register) wait macro
*
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index f7da913..fade0dd 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -398,9 +398,6 @@ intel_panel_detect(struct drm_device *dev)
}
}
-#define DIV_ROUND_CLOSEST_ULL(ll, d) \
-({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
-
/**
* scale - scale values from one range to another
*
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/2] drm/i915: WA intel_backlight scale math
The first patch is version 3 of the original patch authored by Joe Konno. Joe Konno is off-the-grid for the next week or so. Thus, I'm submitting version 3 to keep the momentum going on this. I elected to split v3 up into two patches so that the first can be cherry-picked easier (i.e. out of context of intel_display.c). U. Artie Eoff (2): drm/i915: intel_backlight scale() math WA drm/i915: Move DIV_ROUND_CLOSEST_ULL macro to header drivers/gpu/drm/i915/intel_display.c | 3 --- drivers/gpu/drm/i915/intel_drv.h | 3 +++ drivers/gpu/drm/i915/intel_panel.c | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: preserve other DP_TEST_SINK bits.
Hi Rodrigo, Looks good. Only thing that needs to be removed is that extra blank line between the last part of the function and the return statement. Otherwise... Reviewed-by: Todd Previte -Original Message- From: Rodrigo Vivi [mailto:rodrigo.v...@intel.com] Sent: Monday, September 29, 2014 3:30 PM To: intel-gfx@lists.freedesktop.org Cc: Rodrigo Vivi; Todd Previte Subject: [PATCH] drm/i915: preserve other DP_TEST_SINK bits. Sink crc was implemented based on dp 1.1 spec that had all TEST_SINK bits reserved reading all 0s. But when reviewing my latest changes on sink crc Todd warned me that on new specs we have other valid bits on this reg that we might want to preserve. Cc: Todd Previte Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_dp.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b8699b0..7d5fa2f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3821,8 +3821,9 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (!(buf & DP_TEST_CRC_SUPPORTED)) return -ENOTTY; + drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf); if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, - DP_TEST_SINK_START) < 0) + buf | DP_TEST_SINK_START) < 0) return -EIO; drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf); @@ -3841,7 +3842,10 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) return -EIO; - drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); + drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf); + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, + buf & ~DP_TEST_SINK_START); + return 0; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: preserve other DP_TEST_SINK bits.
Sink crc was implemented based on dp 1.1 spec that had all TEST_SINK bits reserved reading all 0s. But when reviewing my latest changes on sink crc Todd warned me that on new specs we have other valid bits on this reg that we might want to preserve. Cc: Todd Previte Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_dp.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b8699b0..7d5fa2f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3821,8 +3821,9 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (!(buf & DP_TEST_CRC_SUPPORTED)) return -ENOTTY; + drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf); if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, - DP_TEST_SINK_START) < 0) + buf | DP_TEST_SINK_START) < 0) return -EIO; drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf); @@ -3841,7 +3842,10 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) return -EIO; - drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); + drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf); + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, + buf & ~DP_TEST_SINK_START); + return 0; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2
On Mon, 2014-09-29 at 20:31 +0100, Damien Lespiau wrote: > On Mon, Sep 29, 2014 at 05:50:57PM +, Eoff, Ullysses A wrote: > > > -Original Message- > > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On > > > Behalf Of Jani Nikula > > > Sent: Monday, September 29, 2014 6:07 AM > > > To: Joe Konno; intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math > > > WA v2 > > > > > > On Wed, 24 Sep 2014, Joe Konno wrote: > > > > From: Joe Konno > > > > > > > > Improper truncated integer division in the scale() function causes > > > > actual_brightness != brightness. This (partial) work-around should be > > > > sufficient for a majority of use-cases, but it is by no means a complete > > > > solution. > > > > > > > > TODO: Determine how best to scale "user" values to "hw" values, and > > > > vice-versa, when the ranges are of different sizes. That would be a > > > > buggy scenario even with this work-around. > > > > > > > > The issue was introduced in the following (v3.17-rc1) commit: > > > > > > > > 6dda730 drm/i915: respect the VBT minimum backlight brightness > > > > > > > > v2: (thanks to Chris Wilson) clarify commit message, use rounded > > > > division > > > > macro > > > > > > > > Signed-off-by: Joe Konno > > > > --- > > > > drivers/gpu/drm/i915/intel_panel.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > > > b/drivers/gpu/drm/i915/intel_panel.c > > > > index f17ada3..dcdfbb3 100644 > > > > --- a/drivers/gpu/drm/i915/intel_panel.c > > > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > > > @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val, > > > > /* avoid overflows */ > > > > target_val = (uint64_t)(source_val - source_min) * > > > > (target_max - target_min); > > > > - do_div(target_val, source_max - source_min); > > > > + target_val = DIV_ROUND_CLOSEST(target_val, source_max - > > > > source_min); > > > > > > This fails to build with CONFIG_X86_32=y: > > > > > > drivers/built-in.o: In function `scale': > > > intel_panel.c:(.text+0x12ab38): undefined reference to `__udivdi3' > > > make: *** [vmlinux] Error 1 > > > > > > > > > > Do you have a recommended workaround? Should we just > > use the v1 technique instead? > > The problem is target_val is 64 bits and we're trying to do a 64 bits > division with the 32bits instruction set. That is usually handled by > __udivdi3 in libgcc (in userspace). We already have a > DIV_ROUND_CLOSEST_ULL() that uses do_div() in intel_display.c. > Ok, DIV_ROUND_CLOSEST_ULL() would be the one we want I take it. Would intel_drv.h be the appropriate header to move DIV_ROUND_CLOSEST_ULL() into so that we can use it in intel_panel.c, too? U. Artie ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Add IS_BDW_GT3 macro.
On Fri, 19 Sep 2014 20:16:26 -0400
Rodrigo Vivi wrote:
> It will be usefull to specify w/a that affects only BDW GT3.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5fce16c..a00214e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2099,6 +2099,8 @@ struct drm_i915_cmd_table {
>((INTEL_DEVID(dev) & 0xf) == 0x2 || \
>(INTEL_DEVID(dev) & 0xf) == 0x6 || \
>(INTEL_DEVID(dev) & 0xf) == 0xe))
> +#define IS_BDW_GT3(dev) (IS_BROADWELL(dev) && \
> + (INTEL_DEVID(dev) & 0x00F0) == 0x0020)
> #define IS_HSW_ULT(dev) (IS_HASWELL(dev) && \
>(INTEL_DEVID(dev) & 0xFF00) == 0x0A00)
> #define IS_ULT(dev) (IS_HSW_ULT(dev) || IS_BDW_ULT(dev))
Looks correct based on the configs I'm staring at...
Reviewed-by: Jesse Barnes
--
Jesse Barnes, 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] Revert "drm/i915/bdw: BDW Software Turbo"
On Mon, 29 Sep 2014 09:52:38 -0700 Rodrigo Vivi wrote: > On Mon, Sep 29, 2014 at 9:38 AM, Daniel Vetter wrote: > > On Mon, Sep 29, 2014 at 08:48:53AM -0700, Jesse Barnes wrote: > >> On Mon, 29 Sep 2014 15:11:51 +0200 > >> Daniel Vetter wrote: > >> > >> > This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca. > >> > > >> > It's apparently too broken so that Rodrigo submitted a patch to add a > >> > config option for it. Given that the design is also ... suboptimal and > >> > that I've only merged this to get lead engineers and managers off my > >> > back for one second let's just revert this. > >> > > >> > /me puts on combat gear again > >> > > >> > It was worth a shot ... > >> > >> I thought we had a fix for the runtime PM issue this created? And > >> Rodrigo's fix is just a simple "only BDW needs this" patch, so I guess > >> I don't see the big issue? > > Well, actually the issue is to stay with high busted gt frequency even on > idle. > Or this or an incompatibility with the actual rps interface... > > So my patch was actually a protect a feture under development with parameter. > > >> > >> Or is there another bug you didn't mention in the s-o-b section you're > >> worried about? > > The only issue I know is: > References: https://bugs.freedesktop.org/show_bug.cgi?id=77869 Ah yeah, misread it thinking it was accidentally being enabled on non-bdw too or something. But yeah I expect turbo to be broken even with your patch, since we won't ramp up/down the freq (or at least not properly). The only thing Daisy's patch addresses is making sure we at least try to ramp if we're getting regular page flips. The more complete fix (which should address the test failure too) would be to peroidically poll the hw busy state and adjust the freq based on that, independent of the flip frequency. That should allow GPGPU apps, offlien media encode/decode, and these tests to work as they should, but it's a little more work. We need to make sure the timer is shut down when we go into RC6 and only fires frequently when the GPU is busy. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2
On Mon, Sep 29, 2014 at 05:50:57PM +, Eoff, Ullysses A wrote: > > -Original Message- > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf > > Of Jani Nikula > > Sent: Monday, September 29, 2014 6:07 AM > > To: Joe Konno; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA > > v2 > > > > On Wed, 24 Sep 2014, Joe Konno wrote: > > > From: Joe Konno > > > > > > Improper truncated integer division in the scale() function causes > > > actual_brightness != brightness. This (partial) work-around should be > > > sufficient for a majority of use-cases, but it is by no means a complete > > > solution. > > > > > > TODO: Determine how best to scale "user" values to "hw" values, and > > > vice-versa, when the ranges are of different sizes. That would be a > > > buggy scenario even with this work-around. > > > > > > The issue was introduced in the following (v3.17-rc1) commit: > > > > > > 6dda730 drm/i915: respect the VBT minimum backlight brightness > > > > > > v2: (thanks to Chris Wilson) clarify commit message, use rounded division > > > macro > > > > > > Signed-off-by: Joe Konno > > > --- > > > drivers/gpu/drm/i915/intel_panel.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > > b/drivers/gpu/drm/i915/intel_panel.c > > > index f17ada3..dcdfbb3 100644 > > > --- a/drivers/gpu/drm/i915/intel_panel.c > > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > > @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val, > > > /* avoid overflows */ > > > target_val = (uint64_t)(source_val - source_min) * > > > (target_max - target_min); > > > - do_div(target_val, source_max - source_min); > > > + target_val = DIV_ROUND_CLOSEST(target_val, source_max - source_min); > > > > This fails to build with CONFIG_X86_32=y: > > > > drivers/built-in.o: In function `scale': > > intel_panel.c:(.text+0x12ab38): undefined reference to `__udivdi3' > > make: *** [vmlinux] Error 1 > > > > > > Do you have a recommended workaround? Should we just > use the v1 technique instead? The problem is target_val is 64 bits and we're trying to do a 64 bits division with the 32bits instruction set. That is usually handled by __udivdi3 in libgcc (in userspace). We already have a DIV_ROUND_CLOSEST_ULL() that uses do_div() in intel_display.c. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix Sink CRC
Hi Rodrigo,
This patch looks good.
Reviewed-by: Todd Previte
-T
-Original Message-
From: Rodrigo Vivi [mailto:rodrigo.v...@gmail.com]
Sent: Tuesday, September 16, 2014 4:18 PM
To: intel-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org; Rodrigo Vivi; Todd Previte; Daniel
Vetter; Jani Nikula
Subject: [PATCH] drm/i915: Fix Sink CRC
In some cases like when PSR just got enabled the panel need more vblank
times to calculate CRC. I figured that out with the new PSR test cases
facing some cases that I had a green screen but a blank CRC. Even with
2 vblank waits on kernel + 2 vblank waits on test case.
So let's give up to 6 vblank wait time. However we now check for
TEST_CRC_COUNT that shows when panel finished to calculate CRC and has it
ready.
v2: Jani pointed out attempts decrements was wrong and should never reach
the error condition. And Daniel pointed out that EIO is more appropriated
than EGAIN. Also I realized that I have to read test_crc_count after setting
test_sink
v3: Rebase and adding error message
Cc: Todd Previte
Cc: Daniel Vetter
Cc: Jani Nikula
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_dp.c | 23 +--
include/drm/drm_dp_helper.h | 5 +++--
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c index e4d0367..d9091dc7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3468,21 +3468,32 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
*crc)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct intel_crtc *intel_crtc =
to_intel_crtc(intel_dig_port->base.base.crtc);
- u8 buf[1];
+ u8 buf;
+ int test_crc_count;
+ int attempts = 6;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0)
+ if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
return -EIO;
- if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
+ if (!(buf & DP_TEST_CRC_SUPPORTED))
return -ENOTTY;
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
DP_TEST_SINK_START) < 0)
return -EIO;
- /* Wait 2 vblanks to be sure we will have the correct CRC value */
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
+ drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf);
+ test_crc_count = buf & DP_TEST_COUNT_MASK;
+
+ do {
+ drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf);
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ } while (--attempts && (buf & DP_TEST_COUNT_MASK) ==
test_crc_count);
+
+ if (attempts == 0) {
+ DRM_ERROR("Panel is unable to calculate CRC after 6
vblanks\n");
+ return -EIO;
+ }
if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
return -EIO;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
9305c71..8edeed0 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -303,7 +303,8 @@
#define DP_TEST_CRC_B_CB 0x244
#define DP_TEST_SINK_MISC 0x246
-#define DP_TEST_CRC_SUPPORTED (1 << 5)
+# define DP_TEST_CRC_SUPPORTED (1 << 5)
+# define DP_TEST_COUNT_MASK0x7
#define DP_TEST_RESPONSE 0x260
# define DP_TEST_ACK (1 << 0)
@@ -313,7 +314,7 @@
#define DP_TEST_EDID_CHECKSUM 0x261
#define DP_TEST_SINK 0x270
-#define DP_TEST_SINK_START (1 << 0)
+# define DP_TEST_SINK_START(1 << 0)
#define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */
# define DP_PAYLOAD_TABLE_UPDATED (1 << 0)
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2
On Mon, Sep 29, 2014 at 05:50:57PM +, Eoff, Ullysses A wrote: > > -Original Message- > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf > > Of Jani Nikula > > Sent: Monday, September 29, 2014 6:07 AM > > To: Joe Konno; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA > > v2 > > > > On Wed, 24 Sep 2014, Joe Konno wrote: > > > From: Joe Konno > > > > > > Improper truncated integer division in the scale() function causes > > > actual_brightness != brightness. This (partial) work-around should be > > > sufficient for a majority of use-cases, but it is by no means a complete > > > solution. > > > > > > TODO: Determine how best to scale "user" values to "hw" values, and > > > vice-versa, when the ranges are of different sizes. That would be a > > > buggy scenario even with this work-around. > > > > > > The issue was introduced in the following (v3.17-rc1) commit: > > > > > > 6dda730 drm/i915: respect the VBT minimum backlight brightness > > > > > > v2: (thanks to Chris Wilson) clarify commit message, use rounded division > > > macro > > > > > > Signed-off-by: Joe Konno > > > --- > > > drivers/gpu/drm/i915/intel_panel.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > > b/drivers/gpu/drm/i915/intel_panel.c > > > index f17ada3..dcdfbb3 100644 > > > --- a/drivers/gpu/drm/i915/intel_panel.c > > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > > @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val, > > > /* avoid overflows */ > > > target_val = (uint64_t)(source_val - source_min) * > > > (target_max - target_min); > > > - do_div(target_val, source_max - source_min); > > > + target_val = DIV_ROUND_CLOSEST(target_val, source_max - source_min); > > > > This fails to build with CONFIG_X86_32=y: > > > > drivers/built-in.o: In function `scale': > > intel_panel.c:(.text+0x12ab38): undefined reference to `__udivdi3' > > make: *** [vmlinux] Error 1 > > > > > > Do you have a recommended workaround? Should we just > use the v1 technique instead? Compromise and write DIV_ROUND_CLOSEST_ULL(). :| -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2
> -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of > Jani Nikula > Sent: Monday, September 29, 2014 6:07 AM > To: Joe Konno; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2 > > On Wed, 24 Sep 2014, Joe Konno wrote: > > From: Joe Konno > > > > Improper truncated integer division in the scale() function causes > > actual_brightness != brightness. This (partial) work-around should be > > sufficient for a majority of use-cases, but it is by no means a complete > > solution. > > > > TODO: Determine how best to scale "user" values to "hw" values, and > > vice-versa, when the ranges are of different sizes. That would be a > > buggy scenario even with this work-around. > > > > The issue was introduced in the following (v3.17-rc1) commit: > > > > 6dda730 drm/i915: respect the VBT minimum backlight brightness > > > > v2: (thanks to Chris Wilson) clarify commit message, use rounded division > > macro > > > > Signed-off-by: Joe Konno > > --- > > drivers/gpu/drm/i915/intel_panel.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > b/drivers/gpu/drm/i915/intel_panel.c > > index f17ada3..dcdfbb3 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val, > > /* avoid overflows */ > > target_val = (uint64_t)(source_val - source_min) * > > (target_max - target_min); > > - do_div(target_val, source_max - source_min); > > + target_val = DIV_ROUND_CLOSEST(target_val, source_max - source_min); > > This fails to build with CONFIG_X86_32=y: > > drivers/built-in.o: In function `scale': > intel_panel.c:(.text+0x12ab38): undefined reference to `__udivdi3' > make: *** [vmlinux] Error 1 > > Do you have a recommended workaround? Should we just use the v1 technique instead? U. Artie > BR, > Jani. > > > > target_val += target_min; > > > > return target_val; > > -- > > 2.1.0 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 74/89 v4] drm/i915/skl: Implement queue_flip
A few bits have changed in MI_DISPLAY_FLIP to accomodate the new planes.
DE_RRMR seems to have kept its plane flip bits backward compatible.
v2: Rebase on top of nightly
v3: Rebase on top of nightly (minor conflict in i915_reg.h)
v4: Remove code that is now part of intel_crtc_page_flip()
Don't use BUG() in default:
Use intel_crtc->unpin_work->gtt_offset
(Paulo)
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_reg.h | 10 ++
drivers/gpu/drm/i915/intel_display.c | 66
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 98413a8..bbbd4d8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -248,6 +248,16 @@
#define MI_DISPLAY_FLIP_IVB_SPRITE_B (3 << 19)
#define MI_DISPLAY_FLIP_IVB_PLANE_C (4 << 19)
#define MI_DISPLAY_FLIP_IVB_SPRITE_C (5 << 19)
+/* SKL ones */
+#define MI_DISPLAY_FLIP_SKL_PLANE_1_A(0 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_1_B(1 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_1_C(2 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_2_A(4 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_2_B(5 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_2_C(6 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_3_A(7 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_3_B(8 << 8)
+#define MI_DISPLAY_FLIP_SKL_PLANE_3_C(9 << 8)
#define MI_SEMAPHORE_MBOX MI_INSTR(0x16, 1) /* gen6, gen7 */
#define MI_SEMAPHORE_GLOBAL_GTT(1<<22)
#define MI_SEMAPHORE_UPDATE (1<<21)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 070c417..feed6d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9748,6 +9748,69 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
return 0;
}
+static int intel_gen9_queue_flip(struct drm_device *dev,
+struct drm_crtc *crtc,
+struct drm_framebuffer *fb,
+struct drm_i915_gem_object *obj,
+struct intel_engine_cs *ring,
+uint32_t flags)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ uint32_t plane = 0, stride;
+ int ret;
+
+ switch(intel_crtc->pipe) {
+ case PIPE_A:
+ plane = MI_DISPLAY_FLIP_SKL_PLANE_1_A;
+ break;
+ case PIPE_B:
+ plane = MI_DISPLAY_FLIP_SKL_PLANE_1_B;
+ break;
+ case PIPE_C:
+ plane = MI_DISPLAY_FLIP_SKL_PLANE_1_C;
+ break;
+ default:
+ WARN_ONCE(1, "unknown plane in flip command\n");
+ return -ENODEV;
+ }
+
+ switch (obj->tiling_mode) {
+ case I915_TILING_NONE:
+ stride = fb->pitches[0] >> 6;
+ break;
+ case I915_TILING_X:
+ stride = fb->pitches[0] >> 9;
+ break;
+ default:
+ WARN_ONCE(1, "unknown tiling in flip command\n");
+ return -ENODEV;
+ }
+
+ ret = intel_ring_begin(ring, 10);
+ if (ret)
+ return ret;
+
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ intel_ring_emit(ring, DERRMR);
+ intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE |
+ DERRMR_PIPEB_PRI_FLIP_DONE |
+ DERRMR_PIPEC_PRI_FLIP_DONE));
+ intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) |
+ MI_SRM_LRM_GLOBAL_GTT);
+ intel_ring_emit(ring, DERRMR);
+ intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
+ intel_ring_emit(ring, 0);
+
+ intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane);
+ intel_ring_emit(ring, stride << 6 | obj->tiling_mode);
+ intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
+
+ intel_mark_page_flip_active(intel_crtc);
+ __intel_ring_advance(ring);
+
+ return 0;
+}
+
static int intel_default_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
@@ -12678,6 +12741,9 @@ static void intel_init_display(struct drm_device *dev)
case 8: /* FIXME(BDW): Check that the gen8 RCS flip works. */
dev_priv->display.queue_flip = intel_gen7_queue_flip;
break;
+ case 9:
+ dev_priv->display.queue_flip = intel_gen9_queue_flip;
+ break;
}
intel_panel_init_backlight_funcs(dev);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
>> > So did you verify that the register really is a transcoder register? >> > Eg. set PIPE_MULT(A) to >1x and use pipe A to drive the EDP transcoder. >> >> I did not verify. This change was done based on the fact that the >> register does not exist in the VPG HTML version of the BPEC for >> Transcoder_EDP, only TRANS_MULT_A, _B, and _C are defined. >> >> Do we have an SI contact that can confirm? > >Cc:ing Art. > >Art, the confusion here is whether PIPE_MULT is a transcoder register >or a pipe register. BSpec seems to be telling us that it's a transcoder >register but the confusion comes from the fact that the EDP transcoder >doesn't have this register. My theory is that it is a transcoder register, >but since pixel repeat isn't needed for eDP the register isn't present >(or relevant) in the EDP transcoder. Can you clarify this? > >Although in this case it would be very easy to test this theory on >actual hardware as I previously suggested. You are correct. It's transcoder based. It only gets used in HDMI/DVI modes, so EDP doesn't get one. Broadwell was able to properly rename transcoder stuff, so these became TRANS_MULT. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 74/89] drm/i915/skl: Implement queue_flip
On Tue, Sep 23, 2014 at 05:06:35PM -0300, Paulo Zanoni wrote:
> 2014-09-04 8:27 GMT-03:00 Damien Lespiau :
> > A few bits have changed in MI_DISPLAY_FLIP to accomodate the new planes.
> > DE_RRMR seems to have kept its plane flip bits backward compatible.
> >
> > v2: Rebase on top of nightly
> > v2: Rebase on top of nightly (minor conflict in i915_reg.h)
v3
> > Signed-off-by: Damien Lespiau
Some answers below, others in v4 that should follow shortly.
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 10 +
> > drivers/gpu/drm/i915/intel_display.c | 78
> >
> > 2 files changed, 88 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 84a0de6..176e84e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -240,6 +240,16 @@
> > #define MI_DISPLAY_FLIP_IVB_SPRITE_B (3 << 19)
> > #define MI_DISPLAY_FLIP_IVB_PLANE_C (4 << 19)
> > #define MI_DISPLAY_FLIP_IVB_SPRITE_C (5 << 19)
> > +/* SKL ones */
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_1_A(0 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_1_B(1 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_1_C(2 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_2_A(4 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_2_B(5 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_2_C(6 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_3_A(7 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_3_B(8 << 8)
> > +#define MI_DISPLAY_FLIP_SKL_PLANE_3_C(9 << 8)
>
> BSpec seems to mention these bits on CHV too... Maybe we want the new
> function to run on CHV + GEN9? Ping Ville.
Funnily the bits at 21:19 are still documented on CHV. So maybe the
current code works? Considering we only use queue_flip() for the primary
planes at the moment ad we want to move to MMIO based flips in the near
future, probably doesn't matter too much here.
> > #define MI_SEMAPHORE_MBOX MI_INSTR(0x16, 1) /* gen6, gen7 */
> > #define MI_SEMAPHORE_GLOBAL_GTT(1<<22)
> > #define MI_SEMAPHORE_UPDATE (1<<21)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index abd4201..393bd19 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9913,6 +9913,81 @@ static int intel_queue_mmio_flip(struct drm_device
> > *dev,
> > return 0;
> > }
> >
> > +static int intel_gen9_queue_flip(struct drm_device *dev,
> > +struct drm_crtc *crtc,
> > +struct drm_framebuffer *fb,
> > +struct drm_i915_gem_object *obj,
> > +struct intel_engine_cs *ring,
> > +uint32_t flags)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + uint32_t plane = 0, stride;
> > + int ret;
> > +
> > + ring = obj->ring;
> > + if (ring == NULL || ring->id != BCS)
> > + ring = &dev_priv->ring[RCS];
>
> Why do we need these lines above? The other Gens don't have it. And it
> looks like ring shouldn't really be NULL, otherwise the other gens are
> going to crash.
>
>
> > +
> > + ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> > + if (ret)
> > + goto err;
>
> Our only caller (intel_crtc_page_flip) seems to do this for us
> already. Also, the other gens don't do this at their queue_flip
> implementations.
If you're looking for reasons, it's because the code elsewhere used to
look like that and rebasing still worked, so this patch implements
queue_flip() like it used to be implemented at the time of its writing.
Should be fixed now.
> > +
> > + switch(intel_crtc->pipe) {
> > + case PIPE_A:
> > + plane = MI_DISPLAY_FLIP_SKL_PLANE_1_A;
> > + break;
> > + case PIPE_B:
> > + plane = MI_DISPLAY_FLIP_SKL_PLANE_1_B;
> > + break;
> > + case PIPE_C:
> > + plane = MI_DISPLAY_FLIP_SKL_PLANE_1_C;
> > + break;
> > + default:
> > + BUG();
>
> The gen7 version does WARN_ONCE() and returns -ENODEV instead of
> BUG(). Seems more reasonable to just not update the screen instead of
> killing the whole machine.
>
>
> > + }
> > +
> > + switch (obj->tiling_mode) {
> > + case I915_TILING_NONE:
> > + stride = fb->pitches[0] >> 6;
> > + break;
> > + case I915_TILING_X:
> > + stride = fb->pitches[0] >> 9;
> > + break;
> > + default:
> > + BUG();
>
> Also replace this for a WARN and return an error code?
>
> > + }
> > +
> > + ret = intel_ring_begin(ring, 10);
> > + if (ret)
> > +
Re: [Intel-gfx] [PATCH i-g-t 1/4] tests/kms_flip: only print the activity indicator if output is a terminal
On Mon, Sep 29, 2014 at 04:28:15PM +0100, Thomas Wood wrote:
> Signed-off-by: Thomas Wood
> ---
> tests/kms_flip.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 3d3aa9b..8551f64 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -885,7 +885,10 @@ static unsigned int run_test_step(struct test_output *o)
> join_vblank_wait_thread();
> }
>
> - igt_info("."); fflush(stdout);
> + if (isatty(STDOUT_FILENO)) {
> + igt_info(".");
> + fflush(stdout);
> + }
Hm, igt_interactive_info() to wrap this?
-Daniel
>
> if (do_flip && (o->flags & TEST_HANG)) {
> hang = hang_gpu(drm_fd);
> --
> 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
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915/bdw: BDW Software Turbo"
On Mon, Sep 29, 2014 at 9:38 AM, Daniel Vetter wrote: > On Mon, Sep 29, 2014 at 08:48:53AM -0700, Jesse Barnes wrote: >> On Mon, 29 Sep 2014 15:11:51 +0200 >> Daniel Vetter wrote: >> >> > This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca. >> > >> > It's apparently too broken so that Rodrigo submitted a patch to add a >> > config option for it. Given that the design is also ... suboptimal and >> > that I've only merged this to get lead engineers and managers off my >> > back for one second let's just revert this. >> > >> > /me puts on combat gear again >> > >> > It was worth a shot ... >> >> I thought we had a fix for the runtime PM issue this created? And >> Rodrigo's fix is just a simple "only BDW needs this" patch, so I guess >> I don't see the big issue? Well, actually the issue is to stay with high busted gt frequency even on idle. Or this or an incompatibility with the actual rps interface... So my patch was actually a protect a feture under development with parameter. >> >> Or is there another bug you didn't mention in the s-o-b section you're >> worried about? The only issue I know is: References: https://bugs.freedesktop.org/show_bug.cgi?id=77869 > > Rodrigo's patch seems to set the new sw_turbo module option to 0 by > default, everywhere. I believe that protecting the behaviour but accepting the code merged is a better approach to convince people to continue contributing by feeling some sense of progress instead of just blocking all and forcing the constant rebase, etc. But I won't fight for it mainly because of the original Nack you mentioned. > So I've figured given Chris' very clear Nack on the > original patch it's kinda past the point where I can still sugar-coat > things with a straight enough face. > > Or maybe I've totally missing again what's going on. No, you are right! ;) > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915/bdw: BDW Software Turbo"
On Mon, Sep 29, 2014 at 08:48:53AM -0700, Jesse Barnes wrote: > On Mon, 29 Sep 2014 15:11:51 +0200 > Daniel Vetter wrote: > > > This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca. > > > > It's apparently too broken so that Rodrigo submitted a patch to add a > > config option for it. Given that the design is also ... suboptimal and > > that I've only merged this to get lead engineers and managers off my > > back for one second let's just revert this. > > > > /me puts on combat gear again > > > > It was worth a shot ... > > I thought we had a fix for the runtime PM issue this created? And > Rodrigo's fix is just a simple "only BDW needs this" patch, so I guess > I don't see the big issue? > > Or is there another bug you didn't mention in the s-o-b section you're > worried about? Rodrigo's patch seems to set the new sw_turbo module option to 0 by default, everywhere. So I've figured given Chris' very clear Nack on the original patch it's kinda past the point where I can still sugar-coat things with a straight enough face. Or maybe I've totally missing again what's going on. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] lib/igt_core: make single/simple tests use igt_exit
On Mon, Sep 29, 2014 at 03:18:10PM +, Gore, Tim wrote:
>
>
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Monday, September 29, 2014 3:58 PM
> > To: Gore, Tim
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2 1/2] lib/igt_core: make single/simple
> > tests
> > use igt_exit
> >
> > On Mon, Sep 29, 2014 at 01:34:30PM +0100, tim.g...@intel.com wrote:
> > > From: Tim Gore
> > >
> > > Currently tests that use igt_simple_main will simply call "exit()" if
> > > they pass, making it difficult to ensure that any required cleanup is
> > > done. At present this is not an issue, but it will be when I submit a
> > > patch to turn off the lowmemorykiller for all tests.
> > >
> > > Signed-off-by: Tim Gore
> >
> > Merged, with the api doc for igt_exit update. Please don't forget to adjust
> > documentation!
> >
> > Thanks, Daniel
>
> Fair comment about keeping docs up to date but bit confused by the last
> sentence.
> You mention some some docs in a previous mail, in your personal area on
> freedesktop,
> are those the ones you mean. Do I have write access to those ! There's a docs
> directory
> in igt, but this just has an xml file that refers to others that I cannot
> find.
> All hints gratefully received.
$ ./autogen.sh --enable-gtk-doc
$ make
... and you have them, too.
I just upload them for convience somewhere until someone gets less lazy
than me and makes an autobuilder+proper location for this.
-Daniel
> Tim
>
> > > ---
> > > lib/igt_core.c | 9 +
> > > lib/igt_core.h | 2 +-
> > > tests/igt_simulation.c | 2 +-
> > > 3 files changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c index 5d41468..9344815
> > > 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -641,7 +641,7 @@ bool igt_only_list_subtests(void) static bool
> > > skipped_one = false; static bool succeeded_one = false; static bool
> > > failed_one = false; -static int igt_exitcode;
> > > +static int igt_exitcode = IGT_EXIT_SUCCESS;
> > >
> > > static void exit_subtest(const char *) __attribute__((noreturn));
> > > static void exit_subtest(const char *result) @@ -692,7 +692,8 @@ void
> > > igt_skip(const char *f, ...)
> > > assert(in_fixture);
> > > __igt_fixture_end();
> > > } else {
> > > - exit(IGT_EXIT_SKIP);
> > > + igt_exitcode = IGT_EXIT_SKIP;
> > > + igt_exit();
> > > }
> > > }
> > >
> > > @@ -786,7 +787,7 @@ void igt_fail(int exitcode)
> > > __igt_fixture_end();
> > > }
> > >
> > > - exit(exitcode);
> > > + igt_exit();
> > > }
> > > }
> > >
> > > @@ -857,7 +858,7 @@ void igt_exit(void)
> > > exit(IGT_EXIT_SUCCESS);
> > >
> > > if (!test_with_subtests)
> > > - exit(IGT_EXIT_SUCCESS);
> > > + exit(igt_exitcode);
> > >
> > > /* Calling this without calling one of the above is a failure */
> > > assert(skipped_one || succeeded_one || failed_one); diff --git
> > > a/lib/igt_core.h b/lib/igt_core.h index d74cbf9..974a2fb 100644
> > > --- a/lib/igt_core.h
> > > +++ b/lib/igt_core.h
> > > @@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char
> > **argv,
> > > int main(int argc, char **argv) { \
> > > igt_simple_init(argc, argv); \
> > > igt_tokencat(__real_main, __LINE__)(); \
> > > - exit(0); \
> > > + igt_exit(); \
> > > } \
> > > static void igt_tokencat(__real_main, __LINE__)(void) \
> > >
> > > diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c index
> > > 13b2da9..e588959 100644
> > > --- a/tests/igt_simulation.c
> > > +++ b/tests/igt_simulation.c
> > > @@ -65,7 +65,7 @@ static int do_fork(void)
> > >
> > > igt_skip_on_simulation();
> > >
> > > - exit(0);
> > > + igt_exit();
> > > } else {
> > > if (list_subtests)
> > > igt_subtest_init(2, argv_list);
> > > --
> > > 2.1.1
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915/bdw: BDW Software Turbo"
On Mon, 29 Sep 2014 15:11:51 +0200 Daniel Vetter wrote: > This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca. > > It's apparently too broken so that Rodrigo submitted a patch to add a > config option for it. Given that the design is also ... suboptimal and > that I've only merged this to get lead engineers and managers off my > back for one second let's just revert this. > > /me puts on combat gear again > > It was worth a shot ... I thought we had a fix for the runtime PM issue this created? And Rodrigo's fix is just a simple "only BDW needs this" patch, so I guess I don't see the big issue? Or is there another bug you didn't mention in the s-o-b section you're worried about? Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/16] drm/i915: vlv: fix switcheroo/legacy suspend/resume
Reviewed-by: Sagar Kamble
On Wed, 2014-09-10 at 18:16 +0300, Imre Deak wrote:
> During switcheroo/legacy suspend we don't call the suspend_late handler but
> when resuming afterwards we call resume_early. This happened to work so far,
> since suspend_late only disabled the PCI device. This changed in
>
> commit 016970beb05da6285c2f3ed2bee1c676cb75972e
> Author: Sagar Kamble
> Date: Wed Aug 13 23:07:06 2014 +0530
>
> drm/i915: Sharing platform specific sequence between runtime and system
> susp
>
> after which we also saved/restored the VLV Gunit HW state in
> suspend_late/resume_early. So now since we don't save the state during
> suspend a following resume will restore a corrupted state.
>
> Fix this by calling the suspend_late handler during both switcheroo and
> legacy suspend.
>
> CC: Sagar Kamble
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_drv.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 89b63fc..ca74d6d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -648,11 +648,7 @@ int i915_suspend(struct drm_device *dev, pm_message_t
> state)
> if (error)
> return error;
>
> - /* Shut down the device */
> - pci_disable_device(dev->pdev);
> - pci_set_power_state(dev->pdev, PCI_D3hot);
> -
> - return 0;
> + return i915_drm_suspend_late(dev);
> }
>
> static int i915_drm_thaw_early(struct drm_device *dev)
> @@ -769,7 +765,7 @@ static int i915_resume_early(struct drm_device *dev)
> return i915_drm_thaw_early(dev);
> }
>
> -int i915_resume(struct drm_device *dev)
> +static int i915_drm_resume(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret;
> @@ -795,7 +791,12 @@ static int i915_resume_legacy(struct drm_device *dev)
> if (ret)
> return ret;
>
> - return i915_resume(dev);
> + return i915_drm_resume(dev);
> +}
> +
> +int i915_resume(struct drm_device *dev)
> +{
> + return i915_resume_legacy(dev);
> }
>
> /**
> @@ -980,7 +981,7 @@ static int i915_pm_resume(struct device *dev)
> struct pci_dev *pdev = to_pci_dev(dev);
> struct drm_device *drm_dev = pci_get_drvdata(pdev);
>
> - return i915_resume(drm_dev);
> + return i915_drm_resume(drm_dev);
> }
>
> static int i915_pm_freeze(struct device *dev)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/4] tests/kms_flip: only print the activity indicator if output is a terminal
Signed-off-by: Thomas Wood
---
tests/kms_flip.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 3d3aa9b..8551f64 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -885,7 +885,10 @@ static unsigned int run_test_step(struct test_output *o)
join_vblank_wait_thread();
}
- igt_info("."); fflush(stdout);
+ if (isatty(STDOUT_FILENO)) {
+ igt_info(".");
+ fflush(stdout);
+ }
if (do_flip && (o->flags & TEST_HANG)) {
hang = hang_gpu(drm_fd);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/16] drm/i915: vlv: fix gunit HW state corruption during S4 suspend
Thanks Imre.
Reviewed-by: Sagar Kamble
On Mon, 2014-09-15 at 20:42 +0300, Imre Deak wrote:
> The point of doing these in thaw_early is to work around an ordering
> issue wrt. to the hda driver, see the comment in i915_resume_early(). I
> don't see a problem of having them in thaw_early; if you meant that they
> lack the corresponding cleanup calls in freeze_late, it's just because
> they don't have any.
>
> On Mon, 2014-09-15 at 22:56 +0530, Sagar Arun Kamble wrote:
> > From DPM documentation, thaw_early should undo actions by freeze_late.
> > Can we move following snippet from thaw_early to thaw to comply with
> > this?
> >
> > intel_uncore_early_sanitize(dev, true);
> > intel_uncore_sanitize(dev);
> > intel_power_domains_init_hw(dev_priv);
> >
> > On Wed, 2014-09-10 at 18:16 +0300, Imre Deak wrote:
> > > During S4 freeze we don't call intel_suspend_complete(), which would
> > > save the gunit HW state, but during S4 thaw/restore events we call
> > > intel_resume_prepare() which restores it, thus ending up in a corrupted
> > > HW state.
> > >
> > > Fix this by calling intel_suspend_complete() from the corresponding
> > > freeze_late event handler.
> > >
> > > The issue was introduced in
> > > commit 016970beb05da6285c2f3ed2bee1c676cb75972e
> > > Author: Sagar Kamble
> > > Date: Wed Aug 13 23:07:06 2014 +0530
> > >
> > > CC: Sagar Kamble
> > > Signed-off-by: Imre Deak
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index b8bd008..2365875 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -987,6 +987,15 @@ static int i915_pm_freeze(struct device *dev)
> > > return i915_drm_freeze(drm_dev);
> > > }
> > >
> > > +static int i915_pm_freeze_late(struct device *dev)
> > > +{
> > > + struct pci_dev *pdev = to_pci_dev(dev);
> > > + struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > > + struct drm_i915_private *dev_priv = drm_dev->dev_private;
> > > +
> > > + return intel_suspend_complete(dev_priv);
> > > +}
> > > +
> > > static int i915_pm_thaw_early(struct device *dev)
> > > {
> > > struct pci_dev *pdev = to_pci_dev(dev);
> > > @@ -1571,6 +1580,7 @@ static const struct dev_pm_ops i915_pm_ops = {
> > > .resume_early = i915_pm_resume_early,
> > > .resume = i915_pm_resume,
> > > .freeze = i915_pm_freeze,
> > > + .freeze_late = i915_pm_freeze_late,
> > > .thaw_early = i915_pm_thaw_early,
> > > .thaw = i915_pm_thaw,
> > > .poweroff = i915_pm_poweroff,
> >
> >
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/4] tests/sysfs_l3_parity: fix warnings in test enumeration
Source drm_lib.sh before skipping the test to ensure that subtest
enumeration is always handled correctly.
Signed-off-by: Thomas Wood
---
tests/sysfs_l3_parity | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/sysfs_l3_parity b/tests/sysfs_l3_parity
index e9d4411..9bd1724 100755
--- a/tests/sysfs_l3_parity
+++ b/tests/sysfs_l3_parity
@@ -1,13 +1,13 @@
#!/bin/bash
+SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
+. $SOURCE_DIR/drm_lib.sh
+
if ! find /sys/class/drm/card*/ | grep l3_parity > /dev/null ; then
echo "no l3_parity interface, skipping test"
exit 77
fi
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
#Check that we can remap a row
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/4] lib: ensure any buffers are flushed before fork
Flush any buffers before forking to prevent duplicated output.
Signed-off-by: Thomas Wood
---
lib/igt_core.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index f2b4560..6e1c51a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -928,6 +928,10 @@ bool __igt_fork_helper(struct igt_helper_process *proc)
*/
tmp_count = exit_handler_count;
exit_handler_count = 0;
+
+ /* ensure any buffers are flushed before fork */
+ fflush(NULL);
+
switch (pid = fork()) {
case -1:
exit_handler_count = tmp_count;
@@ -1019,6 +1023,9 @@ bool __igt_fork(void)
igt_assert(test_children);
}
+ /* ensure any buffers are flushed before fork */
+ fflush(NULL);
+
switch (test_children[num_test_children++] = fork()) {
case -1:
igt_assert(0);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/4] doc: various spelling and typo fixes
Signed-off-by: Thomas Wood --- lib/igt_aux.c| 12 ++-- lib/igt_core.c | 22 +++--- lib/igt_core.h | 4 ++-- lib/igt_debugfs.c| 6 +++--- lib/igt_debugfs.h| 2 +- lib/igt_kms.h| 2 +- lib/intel_mmio.c | 8 lib/intel_os.c | 2 +- lib/ioctl_wrappers.c | 20 ++-- 9 files changed, 39 insertions(+), 39 deletions(-) diff --git a/lib/igt_aux.c b/lib/igt_aux.c index 180c274..c0ea0e2 100644 --- a/lib/igt_aux.c +++ b/lib/igt_aux.c @@ -59,7 +59,7 @@ /** * SECTION:igt_aux - * @short_description: Auxiliary libararies and support functions + * @short_description: Auxiliary libraries and support functions * @title: i-g-t aux * @include: igt_aux.h * @@ -222,7 +222,7 @@ void igt_permute_array(void *array, unsigned size, * * This function draws a progress indicator, which is useful for running * long-winded tests manually on the console. To avoid spamming logfiles in - * automated runs the progress indicator is supressed when not running on a + * automated runs the progress indicator is suppressed when not running on a * terminal. */ void igt_progress(const char *header, uint64_t i, uint64_t total) @@ -325,11 +325,11 @@ void igt_system_suspend_autoresume(void) } /** - * igt_drop_roo: + * igt_drop_root: * - * Drop root priviledges and make sure it actually worked. Useful for tests + * Drop root privileges and make sure it actually worked. Useful for tests * which need to check security constraints. Note that this should only be - * called from manually forked processes, since the lack of root priviledges + * called from manually forked processes, since the lack of root privileges * will wreak havoc with the automatic cleanup handlers. */ void igt_drop_root(void) @@ -350,7 +350,7 @@ void igt_drop_root(void) * Waits for a key press when run interactively and when the corresponding debug * key is set in the IGT_DEBUG_INTERACTIVE environment variable. Multiple keys * can be specified as a comma-separated list or alternatively "all" if a wait - * should happen for all keys. When not connected to a terminal the enviroment + * should happen for all keys. When not connected to a terminal the environment * setting is ignored and execution immediately continues. * * This is useful for display tests where under certain situation manual diff --git a/lib/igt_core.c b/lib/igt_core.c index 5d41468..f2b4560 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -67,7 +67,7 @@ * @title: i-g-t core * @include: igt_core.h * - * This libary implements the core of the i-g-t test support infrastructure. + * This library implements the core of the i-g-t test support infrastructure. * Main features are the subtest enumeration, cmdline option parsing helpers for * subtest handling and various helpers to structure testcases with subtests and * handle subtest test results. @@ -141,9 +141,9 @@ * conditions, but instead such assumptions should be written in a declarative * style. Use one of the many macros which encapsulate i-g-t's implicit * control flow. Pick the most suitable one to have as much debug output as - * possible without polluting the code unecessarily. For example + * possible without polluting the code unnecessarily. For example * igt_assert_cmpint() for comparing integers or do_ioctl() for running ioctls - * and checking their results. Feel free to add new ones to the libary or + * and checking their results. Feel free to add new ones to the library or * wrap up a set of checks into a private function to further condense your * test logic. * @@ -197,7 +197,7 @@ * run as non-root and doesn't require the i915 driver to be loaded (or any * intel gpu to be present). Then individual subtests can be run with * "--run-subtest". Usage help for tests with subtests can be obtained with the - * "--help" commandline option. + * "--help" command line option. */ static unsigned int exit_handler_count; @@ -733,7 +733,7 @@ void __igt_skip_check(const char *file, const int line, /** * igt_success: * - * Complete a (subtest) as successfull + * Complete a (subtest) as successful * * This bails out of a subtests and marks it as successful. For global tests it * it won't bail out of anything. @@ -1041,9 +1041,9 @@ bool __igt_fork(void) * Wait for all children forked with igt_fork. * * The magic here is that exit codes from children will be correctly propagated - * to the main thread, including the relevant exitcode if a child thread failed. - * Of course if multiple children failed with differen exitcodes the resulting - * exitcode will be non-deterministic. + * to the main thread, including the relevant exit code if a child thread failed. + * Of course if multiple children failed with different exit codes the resulting + * exit code will be non-deterministic. * * Note that igt_skip() will not be forwarded, feature tests
Re: [Intel-gfx] [PATCH v2 1/2] lib/igt_core: make single/simple tests use igt_exit
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, September 29, 2014 3:58 PM
> To: Gore, Tim
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 1/2] lib/igt_core: make single/simple tests
> use igt_exit
>
> On Mon, Sep 29, 2014 at 01:34:30PM +0100, tim.g...@intel.com wrote:
> > From: Tim Gore
> >
> > Currently tests that use igt_simple_main will simply call "exit()" if
> > they pass, making it difficult to ensure that any required cleanup is
> > done. At present this is not an issue, but it will be when I submit a
> > patch to turn off the lowmemorykiller for all tests.
> >
> > Signed-off-by: Tim Gore
>
> Merged, with the api doc for igt_exit update. Please don't forget to adjust
> documentation!
>
> Thanks, Daniel
Fair comment about keeping docs up to date but bit confused by the last
sentence.
You mention some some docs in a previous mail, in your personal area on
freedesktop,
are those the ones you mean. Do I have write access to those ! There's a docs
directory
in igt, but this just has an xml file that refers to others that I cannot find.
All hints gratefully received.
Tim
> > ---
> > lib/igt_core.c | 9 +
> > lib/igt_core.h | 2 +-
> > tests/igt_simulation.c | 2 +-
> > 3 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/igt_core.c b/lib/igt_core.c index 5d41468..9344815
> > 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -641,7 +641,7 @@ bool igt_only_list_subtests(void) static bool
> > skipped_one = false; static bool succeeded_one = false; static bool
> > failed_one = false; -static int igt_exitcode;
> > +static int igt_exitcode = IGT_EXIT_SUCCESS;
> >
> > static void exit_subtest(const char *) __attribute__((noreturn));
> > static void exit_subtest(const char *result) @@ -692,7 +692,8 @@ void
> > igt_skip(const char *f, ...)
> > assert(in_fixture);
> > __igt_fixture_end();
> > } else {
> > - exit(IGT_EXIT_SKIP);
> > + igt_exitcode = IGT_EXIT_SKIP;
> > + igt_exit();
> > }
> > }
> >
> > @@ -786,7 +787,7 @@ void igt_fail(int exitcode)
> > __igt_fixture_end();
> > }
> >
> > - exit(exitcode);
> > + igt_exit();
> > }
> > }
> >
> > @@ -857,7 +858,7 @@ void igt_exit(void)
> > exit(IGT_EXIT_SUCCESS);
> >
> > if (!test_with_subtests)
> > - exit(IGT_EXIT_SUCCESS);
> > + exit(igt_exitcode);
> >
> > /* Calling this without calling one of the above is a failure */
> > assert(skipped_one || succeeded_one || failed_one); diff --git
> > a/lib/igt_core.h b/lib/igt_core.h index d74cbf9..974a2fb 100644
> > --- a/lib/igt_core.h
> > +++ b/lib/igt_core.h
> > @@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char
> **argv,
> > int main(int argc, char **argv) { \
> > igt_simple_init(argc, argv); \
> > igt_tokencat(__real_main, __LINE__)(); \
> > - exit(0); \
> > + igt_exit(); \
> > } \
> > static void igt_tokencat(__real_main, __LINE__)(void) \
> >
> > diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c index
> > 13b2da9..e588959 100644
> > --- a/tests/igt_simulation.c
> > +++ b/tests/igt_simulation.c
> > @@ -65,7 +65,7 @@ static int do_fork(void)
> >
> > igt_skip_on_simulation();
> >
> > - exit(0);
> > + igt_exit();
> > } else {
> > if (list_subtests)
> > igt_subtest_init(2, argv_list);
> > --
> > 2.1.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Correctly reject invalid flags for wait_ioctl
Not having checks for this isn't good. I've checked igt and libdrm and they all already clear flags properly. So we're lucky and should be able to sneak this ABI clarification in. Testcase: igt/gem_wait/invalid-flags Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 2fb87cfa5b82..6891522c5d3b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2811,6 +2811,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) u32 seqno = 0; int ret = 0; + if (args->flags != 0) + return -EINVAL; + ret = i915_mutex_lock_interruptible(dev); if (ret) return ret; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] lib/igt_core: make single/simple tests use igt_exit
On Mon, Sep 29, 2014 at 01:34:30PM +0100, tim.g...@intel.com wrote:
> From: Tim Gore
>
> Currently tests that use igt_simple_main will simply call
> "exit()" if they pass, making it difficult to ensure that
> any required cleanup is done. At present this is not an
> issue, but it will be when I submit a patch to turn off the
> lowmemorykiller for all tests.
>
> Signed-off-by: Tim Gore
Merged, with the api doc for igt_exit update. Please don't forget to
adjust documentation!
Thanks, Daniel
> ---
> lib/igt_core.c | 9 +
> lib/igt_core.h | 2 +-
> tests/igt_simulation.c | 2 +-
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 5d41468..9344815 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -641,7 +641,7 @@ bool igt_only_list_subtests(void)
> static bool skipped_one = false;
> static bool succeeded_one = false;
> static bool failed_one = false;
> -static int igt_exitcode;
> +static int igt_exitcode = IGT_EXIT_SUCCESS;
>
> static void exit_subtest(const char *) __attribute__((noreturn));
> static void exit_subtest(const char *result)
> @@ -692,7 +692,8 @@ void igt_skip(const char *f, ...)
> assert(in_fixture);
> __igt_fixture_end();
> } else {
> - exit(IGT_EXIT_SKIP);
> + igt_exitcode = IGT_EXIT_SKIP;
> + igt_exit();
> }
> }
>
> @@ -786,7 +787,7 @@ void igt_fail(int exitcode)
> __igt_fixture_end();
> }
>
> - exit(exitcode);
> + igt_exit();
> }
> }
>
> @@ -857,7 +858,7 @@ void igt_exit(void)
> exit(IGT_EXIT_SUCCESS);
>
> if (!test_with_subtests)
> - exit(IGT_EXIT_SUCCESS);
> + exit(igt_exitcode);
>
> /* Calling this without calling one of the above is a failure */
> assert(skipped_one || succeeded_one || failed_one);
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index d74cbf9..974a2fb 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char **argv,
> int main(int argc, char **argv) { \
> igt_simple_init(argc, argv); \
> igt_tokencat(__real_main, __LINE__)(); \
> - exit(0); \
> + igt_exit(); \
> } \
> static void igt_tokencat(__real_main, __LINE__)(void) \
>
> diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
> index 13b2da9..e588959 100644
> --- a/tests/igt_simulation.c
> +++ b/tests/igt_simulation.c
> @@ -65,7 +65,7 @@ static int do_fork(void)
>
> igt_skip_on_simulation();
>
> - exit(0);
> + igt_exit();
> } else {
> if (list_subtests)
> igt_subtest_init(2, argv_list);
> --
> 2.1.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/2] Disable Android low memory killer
On Mon, Sep 29, 2014 at 01:47:49PM +, Gore, Tim wrote: > > > > -Original Message- > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > > Vetter > > Sent: Monday, September 29, 2014 2:35 PM > > To: Gore, Tim > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH v2 0/2] Disable Android low memory killer > > > > On Mon, Sep 29, 2014 at 01:34:29PM +0100, tim.g...@intel.com wrote: > > > From: Tim Gore > > > > > > For some tests that put pressure on memory, the Android > > > lowmemorykiller needs to be disabled for the test to run to > > > completion. The first patch is a simple bit of preparation to ensure > > > that all (well written) "simple" tests exit via a call to igt_exit, in > > > the same way as tests with subtests do. > > > This is to make sure we can clean up by re-enabling the > > > lowmemorykiller. > > > The second patch is to disable the Android lowmemorykiller during the > > > common initialisation code (in oom_adjust_for_doom to be exact) and to > > > re-enstate it in igt_exit. > > > > > > v1: As above > > > > > > v2: Remove the call to disable the lowmemorykiller from > > > oom_adjust_for_doom. lowmemorykiller is not disabled > > > by default now; it is up to each individual test to > > > call low_mem_killer_disable() if it needs to. > > > > See my late replies (I was off for an extended w/e). Summary: > > - I think we should just do this unconditionally since it's a hack and > > pointless to burden tests with it. > > - proper exit handler and you can gc patch 1. > > > > Cheers, Daniel > > OK. Igt already has an exit handler, although it just checks that igt_exit > has been called, > and does not get installed for simple tests. Would you be OK with me > extending this > exit handler to re-instate the low memory killer, or do want to keep the > possibility of > simple tests calling exit() rather than igt_exit(). ? Oops, missed that the exit handler stuff doesn't work for simple tests. Yeah, in that case rolling out igt_exit for simple tests makes sense indeed. -Daniel > > I agree that the lowmemorykiller seems to be the problem, but don't feel > confident > to go changing it yet. > > Tim > > > > > > Tim Gore (2): > > > lib/igt_core: make single/simple tests use igt_exit > > > lib/igt_core.c: add function to disable lowmemorykiller > > > > > > lib/igt_core.c | 79 > > +++--- > > > lib/igt_core.h | 2 +- > > > tests/igt_simulation.c | 2 +- > > > 3 files changed, 77 insertions(+), 6 deletions(-) > > > > > > -- > > > 2.1.1 > > > > > > ___ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't spam dmesg with rps messages on vlv/chv
On Tue, Sep 02, 2014 at 03:38:44PM +0300, Jani Nikula wrote:
> On Tue, 02 Sep 2014, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > If the GPU frequency isn't going to change don't spam dmesg with
> > debug messages about it.
> >
> > Signed-off-by: Ville Syrjälä
>
> Oh yes please!
>
> Reviewed-by: Jani Nikula
Queued for -next-fixes, thanks for the patch.
-Daniel
>
>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 13 +++--
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 78e39f8..9bc44f0 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3488,17 +3488,18 @@ void valleyview_set_rps(struct drm_device *dev, u8
> > val)
> > WARN_ON(val > dev_priv->rps.max_freq_softlimit);
> > WARN_ON(val < dev_priv->rps.min_freq_softlimit);
> >
> > - DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz (%u)\n",
> > -vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
> > -dev_priv->rps.cur_freq,
> > -vlv_gpu_freq(dev_priv, val), val);
> > -
> > if (WARN_ONCE(IS_CHERRYVIEW(dev) && (val & 1),
> > "Odd GPU freq value\n"))
> > val &= ~1;
> >
> > - if (val != dev_priv->rps.cur_freq)
> > + if (val != dev_priv->rps.cur_freq) {
> > + DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz
> > (%u)\n",
> > +vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
> > +dev_priv->rps.cur_freq,
> > +vlv_gpu_freq(dev_priv, val), val);
> > +
> > vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> > + }
> >
> > I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
> >
> > --
> > 1.8.5.5
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] tests/gem_wait_render_timeout: Convert to subtests
I want to add a bunch of api tests besides the functional
"render-timeout" testcase.
Signed-off-by: Daniel Vetter
---
tests/.gitignore| 2 +-
tests/Makefile.sources | 2 +-
tests/gem_wait.c| 232
tests/gem_wait_render_timeout.c | 219 -
4 files changed, 234 insertions(+), 221 deletions(-)
create mode 100644 tests/gem_wait.c
delete mode 100644 tests/gem_wait_render_timeout.c
diff --git a/tests/.gitignore b/tests/.gitignore
index c6a99bed926b..bb90685ba3cc 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -104,7 +104,7 @@ gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
gem_userptr_blits
-gem_wait_render_timeout
+gem_wait
gem_write_read_ring_switch
gem_workarounds
gen3_mixed_blits
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d18d7b5818ff..5202ab2642c2 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -136,7 +136,7 @@ TESTS_progs = \
gem_tiling_max_stride \
gem_unfence_active_buffers \
gem_unref_active_buffers \
- gem_wait_render_timeout \
+ gem_wait \
gem_workarounds \
gen3_mixed_blits \
gen3_render_linear_blits \
diff --git a/tests/gem_wait.c b/tests/gem_wait.c
new file mode 100644
index ..0a8ccf62150d
--- /dev/null
+++ b/tests/gem_wait.c
@@ -0,0 +1,232 @@
+/*
+ * Copyright © 2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Ben Widawsky
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "intel_bufmgr.h"
+#include "intel_batchbuffer.h"
+#include "intel_io.h"
+#include "intel_chipset.h"
+#include "igt_aux.h"
+
+#define MSEC_PER_SEC 1000L
+#define USEC_PER_MSEC 1000L
+#define NSEC_PER_USEC 1000L
+#define NSEC_PER_MSEC 100L
+#define USEC_PER_SEC 100L
+#define NSEC_PER_SEC 10L
+
+#define ENOUGH_WORK_IN_SECONDS 2
+#define BUF_SIZE (8<<20)
+#define BUF_PAGES ((8<<20)>>12)
+drm_intel_bo *dst, *dst2;
+
+/* returns time diff in milliseconds */
+static int64_t
+do_time_diff(struct timespec *end, struct timespec *start)
+{
+ int64_t ret;
+ ret = (MSEC_PER_SEC * difftime(end->tv_sec, start->tv_sec)) +
+ ((end->tv_nsec/NSEC_PER_MSEC) - (start->tv_nsec/NSEC_PER_MSEC));
+ return ret;
+}
+
+static int
+gem_bo_wait_timeout(int fd, uint32_t handle, uint64_t *timeout_ns)
+{
+ struct drm_i915_gem_wait wait;
+ int ret;
+
+ igt_assert(timeout_ns);
+
+ wait.bo_handle = handle;
+ wait.timeout_ns = *timeout_ns;
+ wait.flags = 0;
+ ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
+ *timeout_ns = wait.timeout_ns;
+
+ return ret ? -errno : 0;
+}
+
+static void blt_color_fill(struct intel_batchbuffer *batch,
+ drm_intel_bo *buf,
+ const unsigned int pages)
+{
+ const unsigned short height = pages/4;
+ const unsigned short width = 4096;
+
+ COLOR_BLIT_COPY_BATCH_START(COLOR_BLT_WRITE_ALPHA |
+ XY_COLOR_BLT_WRITE_RGB);
+ OUT_BATCH((3 << 24) | /* 32 Bit Color */
+ (0xF0 << 16) | /* Raster OP copy background register */
+ 0); /* Dest pitch is 0 */
+ OUT_BATCH(0);
+ OUT_BATCH(width << 16 |
+ height);
+ OUT_RELOC_FENCED(buf, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
0);
+ OUT_BATCH(rand()); /* random pattern */
+ ADVANCE_BATCH();
+}
+
+static void render_timeout(int fd)
+{
+ drm_intel_bufmgr *bufmgr;
+ struct intel_batchbuffer *batch;
+ uint64_t timeout = ENOUGH_WORK_IN_SECO
[Intel-gfx] [PATCH 1/3] tests/gem_wait_render_timeout: Drop local structs
We're long past the point where libdrm has these.
Signed-off-by: Daniel Vetter
---
tests/gem_wait_render_timeout.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/tests/gem_wait_render_timeout.c b/tests/gem_wait_render_timeout.c
index e05b7ae203a5..86640faf442a 100644
--- a/tests/gem_wait_render_timeout.c
+++ b/tests/gem_wait_render_timeout.c
@@ -69,21 +69,10 @@ do_time_diff(struct timespec *end, struct timespec *start)
return ret;
}
-/* to avoid stupid depencies on libdrm, copy&paste */
-struct local_drm_i915_gem_wait {
- /** Handle of BO we shall wait on */
- __u32 bo_handle;
- __u32 flags;
- /** Number of nanoseconds to wait, Returns time remaining. */
- __u64 timeout_ns;
-};
-
-# define WAIT_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2c, struct
local_drm_i915_gem_wait)
-
static int
gem_bo_wait_timeout(int fd, uint32_t handle, uint64_t *timeout_ns)
{
- struct local_drm_i915_gem_wait wait;
+ struct drm_i915_gem_wait wait;
int ret;
igt_assert(timeout_ns);
@@ -91,7 +80,7 @@ gem_bo_wait_timeout(int fd, uint32_t handle, uint64_t
*timeout_ns)
wait.bo_handle = handle;
wait.timeout_ns = *timeout_ns;
wait.flags = 0;
- ret = drmIoctl(fd, WAIT_IOCTL, &wait);
+ ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
*timeout_ns = wait.timeout_ns;
return ret ? -errno : 0;
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] tests/gem_wait: argument validation tests
Shockingly we don't check for 0 flags!
Signed-off-by: Daniel Vetter
---
tests/gem_wait.c | 36
1 file changed, 36 insertions(+)
diff --git a/tests/gem_wait.c b/tests/gem_wait.c
index 0a8ccf62150d..83ddb97216ab 100644
--- a/tests/gem_wait.c
+++ b/tests/gem_wait.c
@@ -216,6 +216,37 @@ static void render_timeout(int fd)
close(fd);
}
+static void invalid_flags(int fd)
+{
+ struct drm_i915_gem_wait wait;
+ int ret;
+ uint32_t handle;
+
+ handle = gem_create(fd, 4096);
+
+ wait.bo_handle = handle;
+ wait.timeout_ns = 1;
+ wait.flags = 0x;
+ ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
+
+ igt_assert(ret != 0 && errno == EINVAL);
+
+ gem_close(fd, handle);
+}
+
+static void invalid_buf(int fd)
+{
+ struct drm_i915_gem_wait wait;
+ int ret;
+
+ wait.bo_handle = 0;
+ wait.timeout_ns = 1;
+ wait.flags = 0;
+ ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
+
+ igt_assert(ret != 0 && errno == ENOENT);
+}
+
int drm_fd;
igt_main
@@ -226,6 +257,11 @@ igt_main
igt_subtest("render_timeout")
render_timeout(drm_fd);
+ igt_subtest("invalid-flags")
+ invalid_flags(drm_fd);
+
+ igt_subtest("invalid-buf")
+ invalid_buf(drm_fd);
igt_fixture
close(drm_fd);
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: fetch, enable/disable pfit as needed v2
On Thu, Sep 25, 2014 at 03:06:17PM -0300, Paulo Zanoni wrote: > 2014-09-25 14:58 GMT-03:00 Jesse Barnes : > > This moved around on SKL, so we need to make sure we read/write the > > correct regs. > > > > v2: fixup WIN_POS offsets (Paulo) > > zero out WIN_POS reg at disable time (Paulo) > > Reviewed-by: Paulo Zanoni Thanks for the patch and review, I've queued it in my SKL branch that's queued for Daniel. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/2] Disable Android low memory killer
> -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Monday, September 29, 2014 2:35 PM > To: Gore, Tim > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2 0/2] Disable Android low memory killer > > On Mon, Sep 29, 2014 at 01:34:29PM +0100, tim.g...@intel.com wrote: > > From: Tim Gore > > > > For some tests that put pressure on memory, the Android > > lowmemorykiller needs to be disabled for the test to run to > > completion. The first patch is a simple bit of preparation to ensure > > that all (well written) "simple" tests exit via a call to igt_exit, in > > the same way as tests with subtests do. > > This is to make sure we can clean up by re-enabling the > > lowmemorykiller. > > The second patch is to disable the Android lowmemorykiller during the > > common initialisation code (in oom_adjust_for_doom to be exact) and to > > re-enstate it in igt_exit. > > > > v1: As above > > > > v2: Remove the call to disable the lowmemorykiller from > > oom_adjust_for_doom. lowmemorykiller is not disabled > > by default now; it is up to each individual test to > > call low_mem_killer_disable() if it needs to. > > See my late replies (I was off for an extended w/e). Summary: > - I think we should just do this unconditionally since it's a hack and > pointless to burden tests with it. > - proper exit handler and you can gc patch 1. > > Cheers, Daniel OK. Igt already has an exit handler, although it just checks that igt_exit has been called, and does not get installed for simple tests. Would you be OK with me extending this exit handler to re-instate the low memory killer, or do want to keep the possibility of simple tests calling exit() rather than igt_exit(). ? I agree that the lowmemorykiller seems to be the problem, but don't feel confident to go changing it yet. Tim > > > > Tim Gore (2): > > lib/igt_core: make single/simple tests use igt_exit > > lib/igt_core.c: add function to disable lowmemorykiller > > > > lib/igt_core.c | 79 > +++--- > > lib/igt_core.h | 2 +- > > tests/igt_simulation.c | 2 +- > > 3 files changed, 77 insertions(+), 6 deletions(-) > > > > -- > > 2.1.1 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 80/89 v2] drm/i915/skl: Augment the latency debugfs files for SKL
v2: Use the gen >= 9 in the debugfs file condition (Ville)
Reviewed-by: Ville Syrjälä
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_debugfs.c | 76 ++---
1 file changed, 62 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 4e629e1..069b6a6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3513,7 +3513,7 @@ static const struct file_operations
i915_display_crc_ctl_fops = {
.write = display_crc_ctl_write
};
-static void wm_latency_show(struct seq_file *m, const uint16_t wm[5])
+static void wm_latency_show(struct seq_file *m, const uint16_t wm[8])
{
struct drm_device *dev = m->private;
int num_levels = ilk_wm_max_level(dev) + 1;
@@ -3524,13 +3524,17 @@ static void wm_latency_show(struct seq_file *m, const
uint16_t wm[5])
for (level = 0; level < num_levels; level++) {
unsigned int latency = wm[level];
- /* WM1+ latency values in 0.5us units */
- if (level > 0)
+ /*
+* - WM1+ latency values in 0.5us units
+* - latencies are in us on gen9
+*/
+ if (INTEL_INFO(dev)->gen >= 9)
+ latency *= 10;
+ else if (level > 0)
latency *= 5;
seq_printf(m, "WM%d %u (%u.%u usec)\n",
- level, wm[level],
- latency / 10, latency % 10);
+ level, wm[level], latency / 10, latency % 10);
}
drm_modeset_unlock_all(dev);
@@ -3539,8 +3543,15 @@ static void wm_latency_show(struct seq_file *m, const
uint16_t wm[5])
static int pri_wm_latency_show(struct seq_file *m, void *data)
{
struct drm_device *dev = m->private;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ const uint16_t *latencies;
+
+ if (INTEL_INFO(dev)->gen >= 9)
+ latencies = dev_priv->wm.skl_latency;
+ else
+ latencies = to_i915(dev)->wm.pri_latency;
- wm_latency_show(m, to_i915(dev)->wm.pri_latency);
+ wm_latency_show(m, latencies);
return 0;
}
@@ -3548,8 +3559,15 @@ static int pri_wm_latency_show(struct seq_file *m, void
*data)
static int spr_wm_latency_show(struct seq_file *m, void *data)
{
struct drm_device *dev = m->private;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ const uint16_t *latencies;
+
+ if (INTEL_INFO(dev)->gen >= 9)
+ latencies = dev_priv->wm.skl_latency;
+ else
+ latencies = to_i915(dev)->wm.spr_latency;
- wm_latency_show(m, to_i915(dev)->wm.spr_latency);
+ wm_latency_show(m, latencies);
return 0;
}
@@ -3557,8 +3575,15 @@ static int spr_wm_latency_show(struct seq_file *m, void
*data)
static int cur_wm_latency_show(struct seq_file *m, void *data)
{
struct drm_device *dev = m->private;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ const uint16_t *latencies;
+
+ if (INTEL_INFO(dev)->gen >= 9)
+ latencies = dev_priv->wm.skl_latency;
+ else
+ latencies = to_i915(dev)->wm.cur_latency;
- wm_latency_show(m, to_i915(dev)->wm.cur_latency);
+ wm_latency_show(m, latencies);
return 0;
}
@@ -3594,11 +3619,11 @@ static int cur_wm_latency_open(struct inode *inode,
struct file *file)
}
static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
- size_t len, loff_t *offp, uint16_t wm[5])
+ size_t len, loff_t *offp, uint16_t wm[8])
{
struct seq_file *m = file->private_data;
struct drm_device *dev = m->private;
- uint16_t new[5] = { 0 };
+ uint16_t new[8] = { 0 };
int num_levels = ilk_wm_max_level(dev) + 1;
int level;
int ret;
@@ -3612,7 +3637,9 @@ static ssize_t wm_latency_write(struct file *file, const
char __user *ubuf,
tmp[len] = '\0';
- ret = sscanf(tmp, "%hu %hu %hu %hu %hu", &new[0], &new[1], &new[2],
&new[3], &new[4]);
+ ret = sscanf(tmp, "%hu %hu %hu %hu %hu %hu %hu %hu",
+&new[0], &new[1], &new[2], &new[3],
+&new[4], &new[5], &new[6], &new[7]);
if (ret != num_levels)
return -EINVAL;
@@ -3632,8 +3659,15 @@ static ssize_t pri_wm_latency_write(struct file *file,
const char __user *ubuf,
{
struct seq_file *m = file->private_data;
struct drm_device *dev = m->private;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ uint16_t *latencies;
- return wm_latency_write(file, ubuf, len, offp,
to_i915(dev)->wm.pri_latency);
+ if (INTEL_INFO(dev)->gen >= 9)
+ latencies = dev_priv->wm.skl_latency;
+ else
+
Re: [Intel-gfx] [PATCH v2 0/2] Disable Android low memory killer
On Mon, Sep 29, 2014 at 01:34:29PM +0100, tim.g...@intel.com wrote: > From: Tim Gore > > For some tests that put pressure on memory, the Android > lowmemorykiller needs to be disabled for the test to run to > completion. The first patch is a simple bit of preparation > to ensure that all (well written) "simple" tests exit via a > call to igt_exit, in the same way as tests with subtests do. > This is to make sure we can clean up by re-enabling the > lowmemorykiller. > The second patch is to disable the Android lowmemorykiller > during the common initialisation code (in oom_adjust_for_doom > to be exact) and to re-enstate it in igt_exit. > > v1: As above > > v2: Remove the call to disable the lowmemorykiller from > oom_adjust_for_doom. lowmemorykiller is not disabled > by default now; it is up to each individual test to > call low_mem_killer_disable() if it needs to. See my late replies (I was off for an extended w/e). Summary: - I think we should just do this unconditionally since it's a hack and pointless to burden tests with it. - proper exit handler and you can gc patch 1. Cheers, Daniel > > Tim Gore (2): > lib/igt_core: make single/simple tests use igt_exit > lib/igt_core.c: add function to disable lowmemorykiller > > lib/igt_core.c | 79 > +++--- > lib/igt_core.h | 2 +- > tests/igt_simulation.c | 2 +- > 3 files changed, 77 insertions(+), 6 deletions(-) > > -- > 2.1.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/11] drm/i915: Clarify event_lock locking, irq&mixed context
On Mon, Sep 29, 2014 at 08:45:56PM +0800, Jike Song wrote: > On 09/29/2014 08:20 PM, Daniel Vetter wrote: > >On Mon, Sep 29, 2014 at 02:20:27PM +0800, Jike Song wrote: > >>On 09/15/2014 08:55 PM, Daniel Vetter wrote: > >>>Now we tackle the functions also called from interrupt handlers. > >>> > >>>- intel_check_page_flip is exclusively called from irq handlers, so a > >>> plain spin_lock is all we need. In i915_irq.c we have the convention > >>> to give all such functions an _irq_handler postfix, but that would > >>> look strange and als be a bit a misleading name. I've opted for a > >>> WARN_ON(!in_irq()) instead. > >> > >>Hi Daniel, > >> > >> Is it possible to use in_interrupt() instead? Sorry to tell that, in > >>our iGVT-g implementation, the host i915 irq handler needs to be called > >>in a non hardirq driven context. i.e. a tasklet or workqueue. > > > >Hm, why that? Depending upon how you do this you might break a lot of the > >interrupt related locking we have ... This is a crucial integration issue, > >which patch does that change? > >-Daniel > > The RFC patch set is not sent out yet, hopefully in 1~2 days :) > > Yes I know it's not a good implementation ... I also wish there would be > a better way to go :) Well, can you still please intrigue me with why you have to change our interrupt handling from hardirq to work item? It sounds like there's some crucial issue of the overall design hidden in there. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not leak pages when freeing userptr objects
On Fri, Sep 26, 2014 at 03:55:51PM +0100, Chris Wilson wrote: > On Fri, Sep 26, 2014 at 03:05:22PM +0100, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin > > > > sg_alloc_table_from_pages() can build us a table with coalesced ranges which > > means we need to iterate over pages and not sg table entries when releasing > > page references. > > > > Signed-off-by: Tvrtko Ursulin > > Cc: Chris Wilson > > Cc: "Barbalho, Rafael" > > Oh that's fun. I blame Imre for the recent invention of for_each_sg_page()! > Reviewed-by: Chris Wilson > Cc: sta...@vger.kernel.org Again merged to dinf for vetting time, thanks for patch&review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the PTEs after updating them before suspend
On Mon, 29 Sep 2014, Daniel Vetter wrote:
> On Thu, Sep 25, 2014 at 10:13:12AM +0100, Chris Wilson wrote:
>> As we use WC updates of the PTE, we are responsible for notifying the
>> hardware when to flush its TLBs. Do so after we zap all the PTEs before
>> suspend (and the BIOS tries to read our GTT).
>>
>> Fixes a regression from
>>
>> commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
>> Author: Ben Widawsky
>> Date: Wed Oct 16 09:21:30 2013 -0700
>>
>> drm/i915: Disable GGTT PTEs on GEN6+ suspend
>>
>> that survived and continue to cause harm even after
>>
>> commit e568af1c626031925465a5caaab7cca1303d55c7
>> Author: Daniel Vetter
>> Date: Wed Mar 26 20:08:20 2014 +0100
>>
>> drm/i915: Undo gtt scratch pte unmapping again
>>
>> v2: Trivial rebase.
>> v3: Fixes requires pointer dances.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
>> Tested-by: ming@intel.com
>> Signed-off-by: Chris Wilson
>> Cc: sta...@vger.kernel.org
>> Cc: Takashi Iwai
>> Cc: Paulo Zanoni
>> Cc: Todd Previte
>> Cc: Daniel Vetter
>
> Reviewed-by: Daniel Vetter
Pushed to drm-intel-fixes, thanks for the patch and review.
BR,
Jani.
>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 1411613f2174..e42925f76b4b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device
>> *dev)
>> POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
>> }
>>
>> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
>> +{
>> +if (INTEL_INFO(dev_priv->dev)->gen < 6) {
>> +intel_gtt_chipset_flush();
>> +} else {
>> +I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>> +POSTING_READ(GFX_FLSH_CNTL_GEN6);
>> +}
>> +}
>> +
>> void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device
>> *dev)
>> dev_priv->gtt.base.start,
>> dev_priv->gtt.base.total,
>> true);
>> +
>> +i915_ggtt_flush(dev_priv);
>> }
>>
>> void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>> @@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device
>> *dev)
>> gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>> }
>>
>> -i915_gem_chipset_flush(dev);
>> +i915_ggtt_flush(dev_priv);
>> }
>>
>> int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
>> --
>> 2.1.0
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the PTEs after updating them before suspend
On Mon, 29 Sep 2014, Daniel Vetter wrote:
> On Thu, Sep 25, 2014 at 10:13:12AM +0100, Chris Wilson wrote:
>> As we use WC updates of the PTE, we are responsible for notifying the
>> hardware when to flush its TLBs. Do so after we zap all the PTEs before
>> suspend (and the BIOS tries to read our GTT).
>>
>> Fixes a regression from
>>
>> commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
>> Author: Ben Widawsky
>> Date: Wed Oct 16 09:21:30 2013 -0700
>>
>> drm/i915: Disable GGTT PTEs on GEN6+ suspend
>>
>> that survived and continue to cause harm even after
>>
>> commit e568af1c626031925465a5caaab7cca1303d55c7
>> Author: Daniel Vetter
>> Date: Wed Mar 26 20:08:20 2014 +0100
>>
>> drm/i915: Undo gtt scratch pte unmapping again
>>
>> v2: Trivial rebase.
>> v3: Fixes requires pointer dances.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
>> Tested-by: ming@intel.com
>> Signed-off-by: Chris Wilson
>> Cc: sta...@vger.kernel.org
>> Cc: Takashi Iwai
>> Cc: Paulo Zanoni
>> Cc: Todd Previte
>> Cc: Daniel Vetter
>
> Reviewed-by: Daniel Vetter
Pushed to drm-intel-fixes, thanks for the patch and review.
BR,
Jani.
>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 1411613f2174..e42925f76b4b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device
>> *dev)
>> POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
>> }
>>
>> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
>> +{
>> +if (INTEL_INFO(dev_priv->dev)->gen < 6) {
>> +intel_gtt_chipset_flush();
>> +} else {
>> +I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>> +POSTING_READ(GFX_FLSH_CNTL_GEN6);
>> +}
>> +}
>> +
>> void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device
>> *dev)
>> dev_priv->gtt.base.start,
>> dev_priv->gtt.base.total,
>> true);
>> +
>> +i915_ggtt_flush(dev_priv);
>> }
>>
>> void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>> @@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device
>> *dev)
>> gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>> }
>>
>> -i915_gem_chipset_flush(dev);
>> +i915_ggtt_flush(dev_priv);
>> }
>>
>> int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
>> --
>> 2.1.0
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not store the error pointer for a failed userptr registration
On Fri, Sep 26, 2014 at 10:51:54AM +0100, Tvrtko Ursulin wrote:
>
> On 09/26/2014 10:31 AM, Chris Wilson wrote:
> >If we fail to create our mmu notification, we report the error back and
> >currently store the error inside the i915_mm_struct. This not only causes
> >subsequent registerations of the same mm to fail (an issue if the first
> >was interrupted by a signal and needed to be restarted) but also causes
> >us to eventually try and free the error pointer.
> >
> >[ 73.419599] BUG: unable to handle kernel NULL pointer dereference at
> >004c
> >[ 73.419831] IP: [] mmu_notifier_unregister+0x23/0x130
> >[ 73.420065] PGD 8650c067 PUD 870bb067 PMD 0
> >[ 73.420319] Oops: [#1] SMP DEBUG_PAGEALLOC
> >[ 73.420580] CPU: 0 PID: 42 Comm: kworker/0:1 Tainted: GW
> >3.17.0-rc6+ #1561
> >[ 73.420837] Hardware name: Intel Corporation SandyBridge
> >Platform/LosLunas CRB, BIOS ASNBCPT1.86C.0075.P00.1106281639 06/28/2011
> >[ 73.421405] Workqueue: events __i915_mm_struct_free__worker
> >[ 73.421724] task: 880088a81220 ti: 880088168000 task.ti:
> >880088168000
> >[ 73.422051] RIP: 0010:[] []
> >mmu_notifier_unregister+0x23/0x130
> >[ 73.422410] RSP: 0018:88008816bd50 EFLAGS: 00010286
> >[ 73.422765] RAX: 0003 RBX: 880086485400 RCX:
> >
> >[ 73.423137] RDX: 88016d80ee90 RSI: 880086485400 RDI:
> >0044
> >[ 73.423513] RBP: 88008816bd70 R08: 0001 R09:
> >
> >[ 73.423895] R10: 0320 R11: 0001 R12:
> >0044
> >[ 73.424282] R13: 880166e5f008 R14: 88016d815200 R15:
> >880166e5f040
> >[ 73.424682] FS: () GS:88016d80()
> >knlGS:
> >[ 73.425099] CS: 0010 DS: ES: CR0: 80050033
> >[ 73.425537] CR2: 004c CR3: 87f5f000 CR4:
> >000407f0
> >[ 73.426157] Stack:
> >[ 73.426597] 880088a81248 880166e5f038 fffc
> >880166e5f008
> >[ 73.427096] 88008816bd98 814a75f2 880166e5f038
> >8800880f8a28
> >[ 73.427603] 88016d812ac0 88008816be00 8106321a
> >810631af
> >[ 73.428119] Call Trace:
> >[ 73.428606] [] __i915_mm_struct_free__worker+0x42/0x80
> >[ 73.429116] [] process_one_work+0x1ba/0x610
> >[ 73.429632] [] ? process_one_work+0x14f/0x610
> >[ 73.430153] [] worker_thread+0x6b/0x4a0
> >[ 73.430671] [] ? trace_hardirqs_on+0xd/0x10
> >[ 73.431501] [] ? process_one_work+0x610/0x610
> >[ 73.432030] [] kthread+0xf6/0x110
> >[ 73.432561] [] ? __kthread_parkme+0x80/0x80
> >[ 73.433100] [] ret_from_fork+0x7c/0xb0
> >[ 73.433644] [] ? __kthread_parkme+0x80/0x80
> >[ 73.434194] Code: 0f 1f 84 00 00 00 00 00 66 66 66 66 90 8b 46 4c 85 c0
> >0f 8e 10 01 00 00 55 48 89 e5 41 55 41 54 53 48 89 f3 49 89 fc 48 83 ec 08
> ><48> 83 7f 08 00 0f 84 b1 00 00 00 48 c7 c7 40 e6 ac 82 e8 26 65
> >[ 73.435942] RIP [] mmu_notifier_unregister+0x23/0x130
> >[ 73.437017] RSP
> >[ 73.437704] CR2: 004c
> >
> >Fixes regression from commit ad46cb533d586fdb256855437af876617c6cf609
> >Author: Chris Wilson
> >Date: Thu Aug 7 14:20:40 2014 +0100
> >
> > drm/i915: Prevent recursive deadlock on releasing a busy userptr
> >
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84207
> >Testcase: igt/gem_render_copy_redux
> >Testcase: igt/gem_userptr_blits/create-destroy-sync
> >Signed-off-by: Chris Wilson
> >Cc: Jacek Danecki
> >Cc: "Gong, Zhipeng"
> >Cc: Jacek Danecki
> >Cc: "Ursulin, Tvrtko"
> >Cc: sta...@vger.kernel.org
> >---
> > drivers/gpu/drm/i915/i915_gem_userptr.c | 24
> > 1 file changed, 16 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
> >b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >index 903a875..ecd5c84 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >@@ -293,15 +293,23 @@ i915_gem_userptr_release__mmu_notifier(struct
> >drm_i915_gem_object *obj)
> > static struct i915_mmu_notifier *
> > i915_mmu_notifier_find(struct i915_mm_struct *mm)
> > {
> >-if (mm->mn == NULL) {
> >-down_write(&mm->mm->mmap_sem);
> >-mutex_lock(&to_i915(mm->dev)->mm_lock);
> >-if (mm->mn == NULL)
> >-mm->mn = i915_mmu_notifier_create(mm->mm);
> >-mutex_unlock(&to_i915(mm->dev)->mm_lock);
> >-up_write(&mm->mm->mmap_sem);
> >+struct i915_mmu_notifier *mn = mm->mn;
> >+
> >+mn = mm->mn;
> >+if (mn)
> >+return mn;
> >+
> >+down_write(&mm->mm->mmap_sem);
> >+mutex_lock(&to_i915(mm->dev)->mm_lock);
> >+if ((mn = mm->mn) == NULL) {
> >+mn = i915_mmu_notifier_create(mm->mm);
> >+if (!IS_ERR(mn))
> >+mm->mn = mn;
> > }
> >-return mm->mn;
>
Re: [Intel-gfx] [PATCH] drm/i915: Do not store the error pointer for a failed userptr registration
On Fri, 26 Sep 2014, Tvrtko Ursulin wrote:
> On 09/26/2014 10:31 AM, Chris Wilson wrote:
>> If we fail to create our mmu notification, we report the error back and
>> currently store the error inside the i915_mm_struct. This not only causes
>> subsequent registerations of the same mm to fail (an issue if the first
>> was interrupted by a signal and needed to be restarted) but also causes
>> us to eventually try and free the error pointer.
>>
>> [ 73.419599] BUG: unable to handle kernel NULL pointer dereference at
>> 004c
>> [ 73.419831] IP: [] mmu_notifier_unregister+0x23/0x130
>> [ 73.420065] PGD 8650c067 PUD 870bb067 PMD 0
>> [ 73.420319] Oops: [#1] SMP DEBUG_PAGEALLOC
>> [ 73.420580] CPU: 0 PID: 42 Comm: kworker/0:1 Tainted: GW
>> 3.17.0-rc6+ #1561
>> [ 73.420837] Hardware name: Intel Corporation SandyBridge
>> Platform/LosLunas CRB, BIOS ASNBCPT1.86C.0075.P00.1106281639 06/28/2011
>> [ 73.421405] Workqueue: events __i915_mm_struct_free__worker
>> [ 73.421724] task: 880088a81220 ti: 880088168000 task.ti:
>> 880088168000
>> [ 73.422051] RIP: 0010:[] []
>> mmu_notifier_unregister+0x23/0x130
>> [ 73.422410] RSP: 0018:88008816bd50 EFLAGS: 00010286
>> [ 73.422765] RAX: 0003 RBX: 880086485400 RCX:
>>
>> [ 73.423137] RDX: 88016d80ee90 RSI: 880086485400 RDI:
>> 0044
>> [ 73.423513] RBP: 88008816bd70 R08: 0001 R09:
>>
>> [ 73.423895] R10: 0320 R11: 0001 R12:
>> 0044
>> [ 73.424282] R13: 880166e5f008 R14: 88016d815200 R15:
>> 880166e5f040
>> [ 73.424682] FS: () GS:88016d80()
>> knlGS:
>> [ 73.425099] CS: 0010 DS: ES: CR0: 80050033
>> [ 73.425537] CR2: 004c CR3: 87f5f000 CR4:
>> 000407f0
>> [ 73.426157] Stack:
>> [ 73.426597] 880088a81248 880166e5f038 fffc
>> 880166e5f008
>> [ 73.427096] 88008816bd98 814a75f2 880166e5f038
>> 8800880f8a28
>> [ 73.427603] 88016d812ac0 88008816be00 8106321a
>> 810631af
>> [ 73.428119] Call Trace:
>> [ 73.428606] [] __i915_mm_struct_free__worker+0x42/0x80
>> [ 73.429116] [] process_one_work+0x1ba/0x610
>> [ 73.429632] [] ? process_one_work+0x14f/0x610
>> [ 73.430153] [] worker_thread+0x6b/0x4a0
>> [ 73.430671] [] ? trace_hardirqs_on+0xd/0x10
>> [ 73.431501] [] ? process_one_work+0x610/0x610
>> [ 73.432030] [] kthread+0xf6/0x110
>> [ 73.432561] [] ? __kthread_parkme+0x80/0x80
>> [ 73.433100] [] ret_from_fork+0x7c/0xb0
>> [ 73.433644] [] ? __kthread_parkme+0x80/0x80
>> [ 73.434194] Code: 0f 1f 84 00 00 00 00 00 66 66 66 66 90 8b 46 4c 85 c0
>> 0f 8e 10 01 00 00 55 48 89 e5 41 55 41 54 53 48 89 f3 49 89 fc 48 83 ec 08
>> <48> 83 7f 08 00 0f 84 b1 00 00 00 48 c7 c7 40 e6 ac 82 e8 26 65
>> [ 73.435942] RIP [] mmu_notifier_unregister+0x23/0x130
>> [ 73.437017] RSP
>> [ 73.437704] CR2: 004c
>>
>> Fixes regression from commit ad46cb533d586fdb256855437af876617c6cf609
>> Author: Chris Wilson
>> Date: Thu Aug 7 14:20:40 2014 +0100
>>
>> drm/i915: Prevent recursive deadlock on releasing a busy userptr
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84207
>> Testcase: igt/gem_render_copy_redux
>> Testcase: igt/gem_userptr_blits/create-destroy-sync
>> Signed-off-by: Chris Wilson
>> Cc: Jacek Danecki
>> Cc: "Gong, Zhipeng"
>> Cc: Jacek Danecki
>> Cc: "Ursulin, Tvrtko"
>> Cc: sta...@vger.kernel.org
>> ---
>> drivers/gpu/drm/i915/i915_gem_userptr.c | 24
>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
>> b/drivers/gpu/drm/i915/i915_gem_userptr.c
>> index 903a875..ecd5c84 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
>> @@ -293,15 +293,23 @@ i915_gem_userptr_release__mmu_notifier(struct
>> drm_i915_gem_object *obj)
>> static struct i915_mmu_notifier *
>> i915_mmu_notifier_find(struct i915_mm_struct *mm)
>> {
>> -if (mm->mn == NULL) {
>> -down_write(&mm->mm->mmap_sem);
>> -mutex_lock(&to_i915(mm->dev)->mm_lock);
>> -if (mm->mn == NULL)
>> -mm->mn = i915_mmu_notifier_create(mm->mm);
>> -mutex_unlock(&to_i915(mm->dev)->mm_lock);
>> -up_write(&mm->mm->mmap_sem);
>> +struct i915_mmu_notifier *mn = mm->mn;
>> +
>> +mn = mm->mn;
>> +if (mn)
>> +return mn;
>> +
>> +down_write(&mm->mm->mmap_sem);
>> +mutex_lock(&to_i915(mm->dev)->mm_lock);
>> +if ((mn = mm->mn) == NULL) {
>> +mn = i915_mmu_notifier_create(mm->mm);
>> +if (!IS_ERR(mn))
>> +mm->mn = mn;
>> }
>> -return mm->mn;
>> +mutex_unlock(&to_i915
Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer
On Fri, Sep 26, 2014 at 12:14:49PM +0100, Chris Wilson wrote: > On Fri, Sep 26, 2014 at 10:46:48AM +, Gore, Tim wrote: > > > > > > > -Original Message- > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > Sent: Friday, September 26, 2014 11:30 AM > > > To: Gore, Tim > > > Cc: intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer > > > > > > On Fri, Sep 26, 2014 at 10:08:54AM +, Gore, Tim wrote: > > > > I don't think so. This is really just about the Android low memory > > > > killer having Different goals to kswapd. Kswapd tries to keep a > > > > certain amount of free memory so that the kernel can run smoothly. On > > > > Android the lowmemorykiller attempts to maintain somewhat higher > > > > levels of free memory by killing off processes, because the user is > > > > not expected to ever close anything and expects new applications to > > > > open quickly. So if you put the memory under pressure the Android low > > > > memory killer will inevitably look for something to kill, and if your > > > > test is the only thing running its toast. The linux oom killer is still > > > > there, but > > > is never needed on Android because the lowmemorykiller gets there first. > > > > > > Though I think the interaction between lowmemkiller and i915 is broken, I > > > do > > > agree that we need to run our swap tests and that to do so we need to > > > disable lowmemkiller. > > > > > > I would prefer it if only the swap-thrash tests disabled the lowmemkiller > > > though, as I think we still need to test integration behaviour and if > > > lowmemkiller starts killing tests that we think should be well within the > > > limits, > > > that is likely to be our bug. > > > -Chris > > > > > We could do it on a test by test basis if people prefer. It just puts the > > responsibility > > on test writers to know when they might trigger the lowmemorykiller. The > > trouble > > is that the kernel is pretty lazy when comes to freeing up memory. You may > > think > > there should be plenty "free", but that doesn't mean those pages are on the > > free list. > > Once kswapd has achieved its high water mark its done. But the > > lowmemorykiller > > always looks for more than the high water mark (its threshold is based on > > the > > high water mark, so is always higher). If you have enough file backed pages > > you're ok, > > but igt tests don't tend to do much file reading. > > The principle is that mempressure tests call intel_check_memory() first > to see if it valid to run. One of its side-effects is kick the kernel > into freeing up all of its caches. It seems reasonable that we could > disable lowmemkiller here if the test declares that it wants to use > swap. The trick I leave up to you is how to reenable lowmemkiller > automatically when the test completes... > > Or since swap tests are already special, there is no problem in having > igt_swapping_start(); > ... > igt_swapping_end(); > bracketing them. Iiui corrrectly then this isn't good enough, and the lowmemory killer will go beserk even for the tests that exercise purgeable behaviour. Imo this stuff is just fundamentally misdesigned, but if our Android gfx people are ok with the state of affairs and don't want to fix the lowmemory killer for i915 then I'm ok with merging this hack. And then it also makes sense to have it unconditionally for all tests. We just need to add a big comment explaining that if someone ever fixes up the lowmemory killer we need to drop this again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not leak pages when freeing userptr objects
On Fri, 26 Sep 2014, Chris Wilson wrote: > On Fri, Sep 26, 2014 at 03:05:22PM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin >> >> sg_alloc_table_from_pages() can build us a table with coalesced ranges which >> means we need to iterate over pages and not sg table entries when releasing >> page references. >> >> Signed-off-by: Tvrtko Ursulin >> Cc: Chris Wilson >> Cc: "Barbalho, Rafael" > > Oh that's fun. I blame Imre for the recent invention of for_each_sg_page()! > Reviewed-by: Chris Wilson > Cc: sta...@vger.kernel.org Pushed to drm-intel-fixes, with the unused struct scatterlist *sg variable removed. Thanks for the patch, review, and testing. BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit
On Fri, Sep 26, 2014 at 10:27:23AM +0100, tim.g...@intel.com wrote:
> From: Tim Gore
>
> Currently tests that use igt_simple_main will simply call
> "exit()" if they pass, making it difficult to ensure that
> any required cleanup is done. At present this is not an
> issue, but it will be when I submit a patch to turn off the
> lowmemorykiller for all tests.
>
> Signed-off-by: Tim Gore
igt_install_exit_handler and you're done. Sorry if I was a bit unclear
with what I've meant exactly. Btw there's some cute docs for all this
stuff:
http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html
Worth reading once in a while, at least the overview sections ;-)
Cheers, Daniel
> ---
> lib/igt_core.c | 9 +
> lib/igt_core.h | 2 +-
> tests/igt_simulation.c | 2 +-
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 5d41468..9344815 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -641,7 +641,7 @@ bool igt_only_list_subtests(void)
> static bool skipped_one = false;
> static bool succeeded_one = false;
> static bool failed_one = false;
> -static int igt_exitcode;
> +static int igt_exitcode = IGT_EXIT_SUCCESS;
>
> static void exit_subtest(const char *) __attribute__((noreturn));
> static void exit_subtest(const char *result)
> @@ -692,7 +692,8 @@ void igt_skip(const char *f, ...)
> assert(in_fixture);
> __igt_fixture_end();
> } else {
> - exit(IGT_EXIT_SKIP);
> + igt_exitcode = IGT_EXIT_SKIP;
> + igt_exit();
> }
> }
>
> @@ -786,7 +787,7 @@ void igt_fail(int exitcode)
> __igt_fixture_end();
> }
>
> - exit(exitcode);
> + igt_exit();
> }
> }
>
> @@ -857,7 +858,7 @@ void igt_exit(void)
> exit(IGT_EXIT_SUCCESS);
>
> if (!test_with_subtests)
> - exit(IGT_EXIT_SUCCESS);
> + exit(igt_exitcode);
>
> /* Calling this without calling one of the above is a failure */
> assert(skipped_one || succeeded_one || failed_one);
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index d74cbf9..974a2fb 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char **argv,
> int main(int argc, char **argv) { \
> igt_simple_init(argc, argv); \
> igt_tokencat(__real_main, __LINE__)(); \
> - exit(0); \
> + igt_exit(); \
> } \
> static void igt_tokencat(__real_main, __LINE__)(void) \
>
> diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
> index 13b2da9..e588959 100644
> --- a/tests/igt_simulation.c
> +++ b/tests/igt_simulation.c
> @@ -65,7 +65,7 @@ static int do_fork(void)
>
> igt_skip_on_simulation();
>
> - exit(0);
> + igt_exit();
> } else {
> if (list_subtests)
> igt_subtest_init(2, argv_list);
> --
> 2.0.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Revert "drm/i915/bdw: BDW Software Turbo"
This reverts commit c76bb61a71083b2d90504cc6d0dda2047c5d63ca.
It's apparently too broken so that Rodrigo submitted a patch to add a
config option for it. Given that the design is also ... suboptimal and
that I've only merged this to get lead engineers and managers off my
back for one second let's just revert this.
/me puts on combat gear again
It was worth a shot ...
References:
1411686380-1953-1-git-send-email-rodrigo.vivi@intel.com">http://mid.mail-archive.com/1411686380-1953-1-git-send-email-rodrigo.vivi@intel.com
Cc: Jesse Barnes
Cc: Daisy Sun
Cc: Jesse Barnes
Cc: Rodrigo Vivi
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.h | 22
drivers/gpu/drm/i915/i915_irq.c | 21
drivers/gpu/drm/i915/i915_reg.h | 4 -
drivers/gpu/drm/i915/intel_display.c | 3 -
drivers/gpu/drm/i915/intel_pm.c | 230 ++-
5 files changed, 39 insertions(+), 241 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17dfce0f4e68..32180ac92770 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -945,23 +945,6 @@ struct intel_rps_ei {
u32 media_c0;
};
-struct intel_rps_bdw_cal {
- u32 it_threshold_pct; /* interrupt, in percentage */
- u32 eval_interval; /* evaluation interval, in us */
- u32 last_ts;
- u32 last_c0;
- bool is_up;
-};
-
-struct intel_rps_bdw_turbo {
- struct intel_rps_bdw_cal up;
- struct intel_rps_bdw_cal down;
- struct timer_list flip_timer;
- u32 timeout;
- atomic_t flip_received;
- struct work_struct work_max_freq;
-};
-
struct intel_gen6_power_mgmt {
/* work and pm_iir are protected by dev_priv->irq_lock */
struct work_struct work;
@@ -995,9 +978,6 @@ struct intel_gen6_power_mgmt {
bool enabled;
struct delayed_work delayed_resume_work;
- bool is_bdw_sw_turbo; /* Switch of BDW software turbo */
- struct intel_rps_bdw_turbo sw_turbo; /* Calculate RP interrupt timing */
-
/* manual wa residency calculations */
struct intel_rps_ei up_ei, down_ei;
@@ -2828,8 +2808,6 @@ extern void intel_disable_fbc(struct drm_device *dev);
extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
extern void intel_init_pch_refclk(struct drm_device *dev);
extern void gen6_set_rps(struct drm_device *dev, u8 val);
-extern void bdw_software_turbo(struct drm_device *dev);
-extern void gen8_flip_interrupt(struct drm_device *dev);
extern void valleyview_set_rps(struct drm_device *dev, u8 val);
extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
bool enable);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c96ddc953531..3201986bf25e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1979,27 +1979,6 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device
*dev, enum pipe pipe)
res1, res2);
}
-void gen8_flip_interrupt(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!dev_priv->rps.is_bdw_sw_turbo)
- return;
-
- if(atomic_read(&dev_priv->rps.sw_turbo.flip_received)) {
- mod_timer(&dev_priv->rps.sw_turbo.flip_timer,
-
usecs_to_jiffies(dev_priv->rps.sw_turbo.timeout) + jiffies);
- }
- else {
- dev_priv->rps.sw_turbo.flip_timer.expires =
-
usecs_to_jiffies(dev_priv->rps.sw_turbo.timeout) + jiffies;
- add_timer(&dev_priv->rps.sw_turbo.flip_timer);
- atomic_set(&dev_priv->rps.sw_turbo.flip_received, true);
- }
-
- bdw_software_turbo(dev);
-}
-
/* The RPS events need forcewake, so we add them to a work queue and mask their
* IMR bits until the work is done. Other interrupts can be processed without
* the work queue. */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b40d19..e887d4c13ca1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5585,10 +5585,6 @@ enum punit_power_well {
#define GEN8_UCGCTL6 0x9430
#define GEN8_SDEUNIT_CLOCK_GATE_DISABLE (1<<14)
-#define TIMESTAMP_CTR 0x44070
-#define FREQ_1_28_US(us) (((us) * 100) >> 7)
-#define MCHBAR_PCU_C0 (MCHBAR_MIRROR_BASE_SNB + 0x5960)
-
#define GEN6_GFXPAUSE 0xA000
#define GEN6_RPNSWREQ 0xA008
#define GEN6_TURBO_DISABLE (1<<31)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c5079f2c49f3..2d4258038ef2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9926,9 +9926,6 @@ static int intel_crtc_page_flip(st
Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2
On Wed, 24 Sep 2014, Joe Konno wrote: > From: Joe Konno > > Improper truncated integer division in the scale() function causes > actual_brightness != brightness. This (partial) work-around should be > sufficient for a majority of use-cases, but it is by no means a complete > solution. > > TODO: Determine how best to scale "user" values to "hw" values, and > vice-versa, when the ranges are of different sizes. That would be a > buggy scenario even with this work-around. > > The issue was introduced in the following (v3.17-rc1) commit: > > 6dda730 drm/i915: respect the VBT minimum backlight brightness > > v2: (thanks to Chris Wilson) clarify commit message, use rounded division > macro > > Signed-off-by: Joe Konno > --- > drivers/gpu/drm/i915/intel_panel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > b/drivers/gpu/drm/i915/intel_panel.c > index f17ada3..dcdfbb3 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val, > /* avoid overflows */ > target_val = (uint64_t)(source_val - source_min) * > (target_max - target_min); > - do_div(target_val, source_max - source_min); > + target_val = DIV_ROUND_CLOSEST(target_val, source_max - source_min); This fails to build with CONFIG_X86_32=y: drivers/built-in.o: In function `scale': intel_panel.c:(.text+0x12ab38): undefined reference to `__udivdi3' make: *** [vmlinux] Error 1 BR, Jani. > target_val += target_min; > > return target_val; > -- > 2.1.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Introduce sw_turbo parameter.
On Thu, Sep 25, 2014 at 07:06:20PM -0400, Rodrigo Vivi wrote:
> bdw_sw_turbo is been enabled unconditionally and it is causing gpu to be
> busted.
> GT freq stays on max value even when it is on idle or with screen off.
>
> And if this isn't actually the case it is at least breaking the current rps
> API.
> So let's let it disabled by default for now until it is properly adressing
> the tests.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=77869
> Cc: Daisy Sun
> Signed-off-by: Rodrigo Vivi
Given Chris' feedback on that patch I guess I'll just revert it.
Aside: When fixing up patches _always_ cite the offending commit.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h| 1 +
> drivers/gpu/drm/i915/i915_params.c | 6 ++
> drivers/gpu/drm/i915/intel_pm.c| 2 +-
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e845a81..159ddd8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2215,6 +2215,7 @@ struct i915_params {
> int lvds_channel_mode;
> int panel_use_ssc;
> int vbt_sdvo_panel_type;
> + int sw_turbo;
> int enable_rc6;
> int enable_fbc;
> int enable_ppgtt;
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 139f490..1cd587c 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -33,6 +33,7 @@ struct i915_params i915 __read_mostly = {
> .lvds_channel_mode = 0,
> .panel_use_ssc = -1,
> .vbt_sdvo_panel_type = -1,
> + .sw_turbo = 0,
> .enable_rc6 = -1,
> .enable_fbc = -1,
> .enable_execlists = 0,
> @@ -72,6 +73,11 @@ MODULE_PARM_DESC(semaphores,
> "Use semaphores for inter-ring sync "
> "(default: -1 (use per-chip defaults))");
>
> +module_param_named(sw_turbo, i915.sw_turbo, int, 0400);
> +MODULE_PARM_DESC(sw_turbo,
> + "Use SW Turbo. Currently available only on Broadwell"
> + "(default: 0 (disabled))");
> +
> module_param_named(enable_rc6, i915.enable_rc6, int, 0400);
> MODULE_PARM_DESC(enable_rc6,
> "Enable power-saving render C-state 6. "
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 49af81f..d51b17d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3789,7 +3789,7 @@ static void gen8_enable_rps(struct drm_device *dev)
> int unused;
>
> /* Use software Turbo for BDW */
> - dev_priv->rps.is_bdw_sw_turbo = IS_BROADWELL(dev);
> + dev_priv->rps.is_bdw_sw_turbo = IS_BROADWELL(dev) && i915.sw_turbo;
>
> /* 1a: Software RC state - RC0 */
> I915_WRITE(GEN6_RC_STATE, 0);
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Add ppgtt create/release trace points
On Thu, Sep 25, 2014 at 05:10:57PM +0100, daniele.ceraolospu...@intel.com wrote:
> From: Daniele Ceraolo Spurio
>
> These tracepoints are useful for observing the creation and
> destruction of Full PPGTTs.
>
> Signed-off-by: Daniele Ceraolo Spurio
Ok, I think this is an excellent opportunity to properly document the
tracepoints we already have. Since kerneldocs sucks I guess we need to do
it all with DOC: comments that get manually pulled in, with an overview
comment at the top of i915_trace.h.
That then also leaves you with a nice place to explain a bit what this
should be used for.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 4
> drivers/gpu/drm/i915/i915_trace.h | 40
> +
> 2 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index be0aa29..5577e86 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct
> drm_i915_file_private *fpriv)
>
> ppgtt->file_priv = fpriv;
>
> + trace_i915_ppgtt_create(ppgtt);
> +
> return ppgtt;
> }
>
> @@ -1182,6 +1184,8 @@ void i915_ppgtt_release(struct kref *kref)
> struct i915_hw_ppgtt *ppgtt =
> container_of(kref, struct i915_hw_ppgtt, ref);
>
> + trace_i915_ppgtt_release(ppgtt);
> +
> /* vmas should already be unbound */
> WARN_ON(!list_empty(&ppgtt->base.active_list));
> WARN_ON(!list_empty(&ppgtt->base.inactive_list));
> diff --git a/drivers/gpu/drm/i915/i915_trace.h
> b/drivers/gpu/drm/i915/i915_trace.h
> index f5aa006..ca84c49 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -587,6 +587,46 @@ TRACE_EVENT(intel_gpu_freq_change,
> TP_printk("new_freq=%u", __entry->freq)
> );
>
> +TRACE_EVENT(i915_ppgtt_create,
> + TP_PROTO(struct i915_hw_ppgtt *ppgtt),
> +
> + TP_ARGS(ppgtt),
> +
> + TP_STRUCT__entry(
> + __field(struct i915_address_space *, vm)
> + __field(u32, dev)
> + __field(u32, pid)
> + ),
> +
> + TP_fast_assign(
> + __entry->vm = &ppgtt->base;
> + __entry->dev = ppgtt->base.dev->primary->index;
> + __entry->pid = (unsigned int)task_pid_nr(current);
> + ),
> +
> + TP_printk("dev=%u, task_pid=%u, vm=%p",
> + __entry->dev, __entry->pid, __entry->vm)
> +);
> +
> +TRACE_EVENT(i915_ppgtt_release,
> +
> + TP_PROTO(struct i915_hw_ppgtt *ppgtt),
> +
> + TP_ARGS(ppgtt),
> +
> + TP_STRUCT__entry(
> + __field(struct i915_address_space *, vm)
> + __field(u32, dev)
> + ),
> +
> + TP_fast_assign(
> + __entry->vm = &ppgtt->base;
> + __entry->dev = ppgtt->base.dev->primary->index;
> + ),
> +
> + TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
> +);
> +
> #endif /* _I915_TRACE_H_ */
>
> /* This part must be outside protection */
> --
> 1.8.5.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Broadwell DDI Buffer translation - more tuning
On Thu, Sep 25, 2014 at 04:34:42PM +, Runyan, Arthur J wrote:
> That was a fast fix. Looks good now.
> Reviewed-by: Arthur Runyan
Both merged, thanks for patches&review.
-Daniel
>
>
> v2: Arthur noticed I was changing the wrong bit.
>
> Cc: Arthur Runyan
> Cc: Paulo Zanoni
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 66231f0..c9f4b3c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -96,7 +96,7 @@ static const struct ddi_buf_trans bdw_ddi_translations_dp[]
> = {
> { 0x80B2CFFF, 0x001B0002 },
> { 0x00FF, 0x000E000A },
> { 0x00DB6FFF, 0x00160005 },
> - { 0x00C71FFF, 0x001A0002 },
> + { 0x80C71FFF, 0x001A0002 },
> { 0x00F7DFFF, 0x00180004 },
> { 0x80D75FFF, 0x001B0002 },
> };
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/11] drm/i915: Clarify event_lock locking, irq&mixed context
On 09/29/2014 08:20 PM, Daniel Vetter wrote: On Mon, Sep 29, 2014 at 02:20:27PM +0800, Jike Song wrote: On 09/15/2014 08:55 PM, Daniel Vetter wrote: Now we tackle the functions also called from interrupt handlers. - intel_check_page_flip is exclusively called from irq handlers, so a plain spin_lock is all we need. In i915_irq.c we have the convention to give all such functions an _irq_handler postfix, but that would look strange and als be a bit a misleading name. I've opted for a WARN_ON(!in_irq()) instead. Hi Daniel, Is it possible to use in_interrupt() instead? Sorry to tell that, in our iGVT-g implementation, the host i915 irq handler needs to be called in a non hardirq driven context. i.e. a tasklet or workqueue. Hm, why that? Depending upon how you do this you might break a lot of the interrupt related locking we have ... This is a crucial integration issue, which patch does that change? -Daniel The RFC patch set is not sent out yet, hopefully in 1~2 days :) Yes I know it's not a good implementation ... I also wish there would be a better way to go :) -- Thanks, Jike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915/bdw: WaDisableFenceDestinationToSLM
On Mon, Sep 29, 2014 at 2:32 PM, Daniel Vetter wrote: >> > Signed-off-by: Rodrigo Vivi >> >> Reviewed-by: Mika Kuoppala > > Queued for -next, thanks for the patch. Well doesn't compile too well without the previous patch to introduce IS_GT3, so dropped again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.
On 09/29/2014 08:16 PM, Chris Wilson wrote:
On Mon, Sep 29, 2014 at 07:44:56PM +0800, Jike Song wrote:
On 09/19/2014 03:25 PM, Chris Wilson wrote:
Now, given that these are simply trapped memory access, wouldn't it be
simply to have:
struct i915_virtual_gpu {
struct vgt_if *if;
} vgu;
static inline bool intel_vgpu_active(struct drm_i915_private *i915) { return
i915->vgpu.if; }
then you have constructs like
void i915_check_vgpu(struct drm_i915_private *i915)
{
struct vgt_if *if;
if = i915->regs + VGT_PVINFO_PAGE;
if (if->magic != VGT_MAGIC)
return;
if (INTEL_VGT_IF_VERSION !=
INTEL_VGT_IF_VERSION_ENCODE(if->version_major,
if->version_minor))
return;
i915->vgpu.if = if;
}
And later, for example:
if (intel_vgpu_active(dev_priv))
dev_priv->num_fence_regs = dev_priv->vgpu.if->fence_num;
Hi Chris, sorry that I didn't understand you correctly. After discussion
with Yu today, I realized that unfortunately, the vgt_if can't be
dereferenced directly.
There are several reasons:
- dereferencing a MMIO address will be complained by sparse(1)
- for Guest VM, such accesses will be trapped by hypervisor, and
hence emulated correctly; However this doesn't work for Host(e.g.
Domain 0 of Xen, the Linux host KVM resides in). For host, we used
a proactive mechanism to redirect i915 MMIO accesses to vgt,
the GPU device-model, for the sake of central management & sharing
among VMs, including host.
You only need to be careful during vgpu detection. After that you know
everything is safe. If you do the detection during intel_uncore_init(),
or similar, you can use raw mmio access and explict sparse annotations
to do the right thing.
-Chris
Hi Chris,
Thanks for your suggestion, however, it addressed issue #1 but not issue #2.
I'd like to give a detailed explanation :)
For Guest i915 driver, when accessing a MMIO reg, it goes:
whatever I915_READ/readl or direct dereferencing(addr)
addr is translated to gpa(guest physical address) by guest
paging structure
hypervisor trapped the gpa access and forward it to vgt
vgt_emulate_read32(vgt instance of guest, gpa)
The real problem is, when running as the host gpu driver, i915 MMIO/GTT
accesses are:
1) every difficult to trap, say the domain 0 of Xen hypervisor; or
2) impossible to trap, say KVM(In KVM, the host i915 is running in the
very same
privilege level and root mode as KVM hypervisor.)
So, for Host i915 driver, when accessing a MMIO reg, it goes:
I915_READ(addr)
gen6_read32(addr)
offset = addr - dev_priv->regs
vgt_mmio_readl(the vgt instance of host, offset)
.. some processing ..
if (passed-throuth)
readl(offset + dev_priv->regs)
else
pa = BAR0 + offset
vgt_emulate_read32(vgt instance of
host, pa)
The vgt_if corresponds to the PVINFO page, not passed-through(actually it
doesn't physically exist),
should fall into the "else" above.
Given that ... Yes we can dereference pointers for guest here, but consider
that we'll
deal with host in the future ...
--
Thanks,
Jike
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
On Thu, Sep 25, 2014 at 07:11:11AM +0300, Ville Syrjälä wrote:
> On Wed, Sep 24, 2014 at 09:42:18PM +0200, Daniel Vetter wrote:
> > On Wed, Sep 24, 2014 at 08:44:42AM -0700, Clint Taylor wrote:
> > > On 09/24/2014 01:51 AM, Daniel Vetter wrote:
> > > >On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.tay...@intel.com
> > > >wrote:
> > > >>From: Clint Taylor
> > > >>
> > > >>Haswell and later silicon has added a new pixel replication register
> > > >>to the pipe timings for each transcoder. Now in addition to the
> > > >>DPLL_A_MD register for the pixel clock double, we also need to write to
> > > >>the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> > > >>to the DPLL only double the pixel clock.
> > > >>
> > > >>Signed-off-by: Clint Taylor
> > > >>---
> > > >> drivers/gpu/drm/i915/i915_reg.h |3 +++
> > > >> drivers/gpu/drm/i915/intel_display.c |6 +-
> > > >> 2 files changed, 8 insertions(+), 1 deletion(-)
> > > >>
> > > >>diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > >>b/drivers/gpu/drm/i915/i915_reg.h
> > > >>index 15c0eaa..7c078d9 100644
> > > >>--- a/drivers/gpu/drm/i915/i915_reg.h
> > > >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> > > >>@@ -2431,6 +2431,7 @@ enum punit_power_well {
> > > >> #define _PIPEASRC 0x6001c
> > > >> #define _BCLRPAT_A0x60020
> > > >> #define _VSYNCSHIFT_A 0x60028
> > > >>+#define _MULTIPLY_A0x6002c
> > > >>
> > > >> /* Pipe B timing regs */
> > > >> #define _HTOTAL_B 0x61000
> > > >>@@ -2442,6 +2443,7 @@ enum punit_power_well {
> > > >> #define _PIPEBSRC 0x6101c
> > > >> #define _BCLRPAT_B0x61020
> > > >> #define _VSYNCSHIFT_B 0x61028
> > > >>+#define _MULTIPLY_B0x6102c
> > > >>
> > > >> #define TRANSCODER_A_OFFSET 0x6
> > > >> #define TRANSCODER_B_OFFSET 0x61000
> > > >>@@ -2462,6 +2464,7 @@ enum punit_power_well {
> > > >> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> > > >> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> > > >> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> > > >>+#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> > > >
> > > >MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
> > > >with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
> > > >>
> > > >> /* HSW+ eDP PSR registers */
> > > >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ?
> > > >> 0x64800 : 0x6f800)
> > > >>diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > >>b/drivers/gpu/drm/i915/intel_display.c
> > > >>index c092ff4..e58fcde 100644
> > > >>--- a/drivers/gpu/drm/i915/intel_display.c
> > > >>+++ b/drivers/gpu/drm/i915/intel_display.c
> > > >>@@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc
> > > >>*crtc)
> > > >>
> > > >>intel_set_pipe_timings(intel_crtc);
> > > >>
> > > >>+ I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> > > >
> > > >This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
> > > >Same below.
> > > >
> > > The MULTIPLY Macro calls the _TRANSCODER2 MACRO which already indexes the
> > > register based on intel_crtc->pipe. This should be all that's required.
> >
> > I don't see where it indexes with intel_crtc->pipe ...
> >
> > But it doesn't matter since the register is clearly in the transcoder
> > block, and the reason why Bspec says is per-pipe is that the edp
> > transcoder doesn't have it. So on second consideration I guess we can keep
> > this part as-is then.
>
> ? If it doesn't exist for EDP we can't just go passing cpu_transcoder
> to it.
>
> BDW BSpec seems to claim that it really is a transcoder register and not
> a pipe register (just looking at the offset isn't enoguh to tell that
> as PIPESRC shows). So in that sense using cpu_transcoder is more
> appropriate, but if we do that we must not write the register when
> cpu_transcoder == EDP. I suppose that even makes sense since it's only
> valid for HDMI/DVI and that's not supported on the EDP transcoder.
> But someone really must verify that it really is a transcoder and not a
> pipe register and that it has no effect on transcoder EDP.
I guess we could use the cpu_transcoder and add a WARN_ON(cpu_transcoder
== EDP). Makes stuff consistent and if we ever botch this up we'll know.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/2] Disable Android low memory killer
From: Tim Gore For some tests that put pressure on memory, the Android lowmemorykiller needs to be disabled for the test to run to completion. The first patch is a simple bit of preparation to ensure that all (well written) "simple" tests exit via a call to igt_exit, in the same way as tests with subtests do. This is to make sure we can clean up by re-enabling the lowmemorykiller. The second patch is to disable the Android lowmemorykiller during the common initialisation code (in oom_adjust_for_doom to be exact) and to re-enstate it in igt_exit. v1: As above v2: Remove the call to disable the lowmemorykiller from oom_adjust_for_doom. lowmemorykiller is not disabled by default now; it is up to each individual test to call low_mem_killer_disable() if it needs to. Tim Gore (2): lib/igt_core: make single/simple tests use igt_exit lib/igt_core.c: add function to disable lowmemorykiller lib/igt_core.c | 79 +++--- lib/igt_core.h | 2 +- tests/igt_simulation.c | 2 +- 3 files changed, 77 insertions(+), 6 deletions(-) -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] lib/igt_core: make single/simple tests use igt_exit
From: Tim Gore
Currently tests that use igt_simple_main will simply call
"exit()" if they pass, making it difficult to ensure that
any required cleanup is done. At present this is not an
issue, but it will be when I submit a patch to turn off the
lowmemorykiller for all tests.
Signed-off-by: Tim Gore
---
lib/igt_core.c | 9 +
lib/igt_core.h | 2 +-
tests/igt_simulation.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 5d41468..9344815 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -641,7 +641,7 @@ bool igt_only_list_subtests(void)
static bool skipped_one = false;
static bool succeeded_one = false;
static bool failed_one = false;
-static int igt_exitcode;
+static int igt_exitcode = IGT_EXIT_SUCCESS;
static void exit_subtest(const char *) __attribute__((noreturn));
static void exit_subtest(const char *result)
@@ -692,7 +692,8 @@ void igt_skip(const char *f, ...)
assert(in_fixture);
__igt_fixture_end();
} else {
- exit(IGT_EXIT_SKIP);
+ igt_exitcode = IGT_EXIT_SKIP;
+ igt_exit();
}
}
@@ -786,7 +787,7 @@ void igt_fail(int exitcode)
__igt_fixture_end();
}
- exit(exitcode);
+ igt_exit();
}
}
@@ -857,7 +858,7 @@ void igt_exit(void)
exit(IGT_EXIT_SUCCESS);
if (!test_with_subtests)
- exit(IGT_EXIT_SUCCESS);
+ exit(igt_exitcode);
/* Calling this without calling one of the above is a failure */
assert(skipped_one || succeeded_one || failed_one);
diff --git a/lib/igt_core.h b/lib/igt_core.h
index d74cbf9..974a2fb 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char **argv,
int main(int argc, char **argv) { \
igt_simple_init(argc, argv); \
igt_tokencat(__real_main, __LINE__)(); \
- exit(0); \
+ igt_exit(); \
} \
static void igt_tokencat(__real_main, __LINE__)(void) \
diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
index 13b2da9..e588959 100644
--- a/tests/igt_simulation.c
+++ b/tests/igt_simulation.c
@@ -65,7 +65,7 @@ static int do_fork(void)
igt_skip_on_simulation();
- exit(0);
+ igt_exit();
} else {
if (list_subtests)
igt_subtest_init(2, argv_list);
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] lib/igt_core.c: add function to disable lowmemorykiller
From: Tim Gore
Several IGT tests put a lot of pressure on memory and
when running these tests on Android they tend to get
killed by the lowmemorykiller. The lowmemorykiller really
is not usefull in this context and is just preventing the
test from doing its job. This commit adds a function to
disable the lowmemorykiller by writing "" to its
oom adj parameter, which means it will never "select"
any process to kill. The normal linux oom killer is still
there to protect the kernel.
This function is not called by default; it is up to each
individual test to call this function if it might trigger
the lowmemorykiller.
The igt_exit() now includes a call to this function to
re-enable the lowmemorykiller if it has been disabled.
A test may also re-enable the lowmemorykiller once it is
finished since this is benign if called more than once.
Signed-off-by: Tim Gore
---
lib/igt_core.c | 70 ++
1 file changed, 70 insertions(+)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 9344815..172a204 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -326,6 +326,72 @@ static void print_usage(const char *help_str, bool
output_on_stderr)
fprintf(f, "%s\n", help_str);
}
+
+/* Some of the IGT tests put quite a lot of pressure on memory and when
+ * running on Android they are sometimes killed by the Android low memory
killer.
+ * The low memory killer really isn't usefull in this context and has no
+ * interaction with the gpu driver that we are testing, so the following
+ * function is used to disable it by modifying one of its module parameters.
+ * We still have the normal linux oom killer to protect the kernel.
+ * Apparently it is also possible for the lowmemorykiller to get included
+ * in some linux distributions; so rather than check for Android we directly
+ * check for the existence of the module parameter we want to adjust.
+ */
+void low_mem_killer_disable(bool disable)
+{
+ static const char*
adj_fname="/sys/module/lowmemorykiller/parameters/adj";
+ static const char no_lowmem_killer[] = "";
+ int fd;
+ struct stat buf;
+ /* The following must persist across invocations */
+ static char prev_adj_scores[256];
+ static int adj_scores_len = 0;
+ static bool is_disabled = false;
+
+ /* check to see if there is something to do */
+ if (!(disable ^ is_disabled))
+ return;
+
+ /* capture the permissions bits for the lowmemkiller adj pseudo-file.
+ Bail out if the stat fails; it probably means that there is no
+ lowmemorykiller, but in any case we're doomed. */
+ if (stat (adj_fname, &buf))
+ {
+ igt_assert(errno == ENOENT);
+ return;
+ }
+ /* make sure the file can be read/written - by default it is write-only
*/
+ chmod (adj_fname, S_IRUSR | S_IWUSR);
+
+ if (disable)
+ {
+ /* read the current oom adj parameters for lowmemorykiller */
+ fd = open(adj_fname, O_RDWR);
+ igt_assert(fd != -1);
+ adj_scores_len = read(fd, (void*)prev_adj_scores, 255);
+ igt_assert(adj_scores_len > 0);
+
+ /* writing to this module parameter effectively diables the
+* low memory killer. This is not a real file, so we dont need
to
+* seek to the start or truncate it */
+ write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
+ close(fd);
+ is_disabled = true;
+ }
+ else
+ {
+ /* just re-enstate the original settings */
+ fd = open(adj_fname, O_WRONLY);
+ igt_assert(fd != -1);
+ write(fd, prev_adj_scores, adj_scores_len);
+ close(fd);
+ is_disabled = false;
+ }
+
+ /* re-enstate the file permissions */
+ chmod (adj_fname, buf.st_mode);
+}
+
static void oom_adjust_for_doom(void)
{
int fd;
@@ -334,6 +400,7 @@ static void oom_adjust_for_doom(void)
fd = open("/proc/self/oom_score_adj", O_WRONLY);
igt_assert(fd != -1);
igt_assert(write(fd, always_kill, sizeof(always_kill)) ==
sizeof(always_kill));
+ close(fd);
}
static int common_init(int argc, char **argv,
@@ -848,6 +915,9 @@ void igt_exit(void)
{
igt_exit_called = true;
+ /* re-enstate the low mem killer in case we disabled it */
+ low_mem_killer_disable(false);
+
if (run_single_subtest && !run_single_subtest_found) {
igt_warn("Unknown subtest: %s\n", run_single_subtest);
exit(IGT_EXIT_INVALID);
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915/bdw: WaDisableFenceDestinationToSLM
On Thu, Sep 25, 2014 at 03:37:37PM +0300, Mika Kuoppala wrote:
> Rodrigo Vivi writes:
>
> > This WA affect BDW GT3 E and F steppings.
Thou shalt not mention steppings in public. Fixed here and in the comment
below while applying.
> >
> > Signed-off-by: Rodrigo Vivi
>
> Reviewed-by: Mika Kuoppala
Queued for -next, thanks for the patch.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index ad8179b..124ea60 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4836,6 +4836,7 @@ enum punit_power_well {
> > /* GEN8 chicken */
> > #define HDC_CHICKEN0 0x7300
> > #define HDC_FORCE_NON_COHERENT(1<<4)
> > +#define HDC_FENCE_DEST_SLM_DISABLE(1<<14)
> >
> > /* WaCatErrorRejectionIssue */
> > #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 681ea86..7c3d17a 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -740,8 +740,12 @@ static int bdw_init_workarounds(struct intel_engine_cs
> > *ring)
> > * workaround for for a possible hang in the unlikely event a TLB
> > * invalidation occurs during a PSD flush.
> > */
> > + /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production E/F) */
> > intel_ring_emit_wa(ring, HDC_CHICKEN0,
> > - _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT));
> > + _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT |
> > + (IS_BDW_GT3(dev) ?
> > + HDC_FENCE_DEST_SLM_DISABLE : 0)
> > + ));
> >
> > /* Wa4x4STCOptimizationDisable:bdw */
> > intel_ring_emit_wa(ring, CACHE_MODE_1,
> > --
> > 1.9.3
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/8] drm/i915: Add the display switch logic for vgpu in i915 driver
On Mon, Sep 29, 2014 at 02:31:17PM +0800, Zhiyuan Lv wrote:
> Hi Daniel,
>
> On Fri, Sep 19, 2014 at 06:09:37PM +0200, Daniel Vetter wrote:
> > On Sat, Sep 20, 2014 at 02:47:05AM +0800, Jike Song wrote:
> > > From: Yu Zhang
> > >
> > > Display switch logic is added to notify the vgt mediator that
> > > current vgpu have a valid surface to show. It does so by writing
> > > the display_ready field in PV INFO page, and then will be handled
> > > in vgt mediator. This is useful to avoid trickiness when the VM's
> > > framebuffer is being accessed in the middle of VM modesetting,
> > > e.g. compositing the framebuffer in the host side
> > >
> > > Signed-off-by: Yu Zhang
> > > Signed-off-by: Jike Song
> > > Signed-off-by: Zhiyuan Lv
> > > ---
> > > drivers/gpu/drm/i915/i915_dma.c | 8
> > > drivers/gpu/drm/i915/i915_reg.h | 7 +++
> > > drivers/gpu/drm/i915/intel_display.c | 10 ++
> > > 3 files changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > > b/drivers/gpu/drm/i915/i915_dma.c
> > > index bb4ad52..d9462e1 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -1790,6 +1790,14 @@ int i915_driver_load(struct drm_device *dev,
> > > unsigned long flags)
> > > } else {
> > > /* Start out suspended in ums mode. */
> > > dev_priv->ums.mm_suspended = 1;
> > > + if (intel_vgpu_active(dev)) {
> > > + /*
> > > + * Notify a valid surface after modesetting,
> > > + * when running inside a VM.
> > > + */
> > > + I915_WRITE(vgt_info_off(display_ready),
> > > + VGT_DRV_DISPLAY_READY);
> > > + }
> > > }
> > >
> > > i915_setup_sysfs(dev);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index a70f12e..38d606c 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6673,5 +6673,12 @@ enum punit_power_well {
> > > #define vgt_info_off(x) \
> > > (VGT_PVINFO_PAGE + (long)&((struct vgt_if *) NULL)->x)
> > >
> > > +/*
> > > + * The information set by the guest gfx driver, through the
> > > display_ready field
> > > + */
> > > +enum vgt_display_status {
> > > + VGT_DRV_DISPLAY_NOT_READY = 0,
> > > + VGT_DRV_DISPLAY_READY, /* ready for display switch */
> > > +};
> > >
> > > #endif /* _I915_REG_H_ */
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index b78f00a..3af9259 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11642,6 +11642,16 @@ static int intel_crtc_set_config(struct
> > > drm_mode_set *set)
> > > intel_modeset_check_state(set->crtc->dev);
> > > }
> > >
> > > + if (intel_vgpu_active(dev)) {
> > > + /*
> > > + * Notify a valid surface after modesetting,
> > > + * when running inside a VM.
> > > + */
> > > + struct drm_i915_private *dev_priv = dev->dev_private;
> > > +
> > > + I915_WRITE(vgt_info_off(display_ready), VGT_DRV_DISPLAY_READY);
> > > + }
> >
> > Since very shortly we now have the frontbuffer tracking support code.
> > Should we move it there? See intel_frontbuffer_flush&flip.
>
> Thank you very much for the comments and sorry for the delayed reply!
>
> For virtual machines we only need such notification once for guest system
> booting, would that be too heavy to add code into the flush&flip function,
> consider that they are to be called many times?
>
> Going forward, we want to use the same "display_ready" for i915 running in
> host
> machines so that we can capture display status changes. Would that be good to
> keep the code in "intel_crtc_set_config"? Appreciate your inputs. Thanks!
I guess the question is what exactly you want to signal to the hyporvisor
with this. I guess I need to dig a bit into the sourcecode for the
hyperviros part, and you need to make a really clear specification of when
guests should call this and what the hyporvisor must to in reaction of
this.
I don't think we'll have any issues with a bit of overhead in the
frontbuffer flip/flush functions, they're not called too often. Aside:
There's no nice kerneldoc for this stuff:
http://people.freedesktop.org/~danvet/drm/drmI915.html#idp54709056
Cheers, Daniel
>
> Regards,
> -Zhiyuan
>
> > -Daniel
> >
> > > +
> > > if (ret) {
> > > DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
> > > set->crtc->base.id, ret);
> > > --
> > > 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
> > +41 (0) 79 3
Re: [Intel-gfx] [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
On Fri, Sep 26, 2014 at 03:04:22PM -0700, Clint Taylor wrote:
> On 09/26/2014 09:38 AM, Ville Syrjälä wrote:
> > On Thu, Sep 25, 2014 at 10:03:53AM -0700, clinton.a.tay...@intel.com wrote:
> >> From: Clint Taylor
> >>
> >> Haswell and later silicon has added a new pixel replication register
> >> to the pipe timings for each transcoder. Now in addition to the
> >> DPLL_A_MD register for the pixel clock double, we also need to write
> >> to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> >> to the DPLL only double the pixel clock.
> >>
> >> ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> >> ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
> >>
> >> Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?=
> >> Cc: Daniel Vetter
> >> Cc: Jani Nikula
> >>
> >> Signed-off-by: Clint Taylor
> >> ---
> >> drivers/gpu/drm/i915/i915_reg.h |3 +++
> >> drivers/gpu/drm/i915/intel_display.c | 10 +-
> >> 2 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h
> >> index ad8179b..035d58c 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -2443,6 +2443,7 @@ enum punit_power_well {
> >> #define _PIPEASRC0x6001c
> >> #define _BCLRPAT_A 0x60020
> >> #define _VSYNCSHIFT_A0x60028
> >> +#define _MULTIPLY_A 0x6002c
> >>
> >> /* Pipe B timing regs */
> >> #define _HTOTAL_B0x61000
> >> @@ -2454,6 +2455,7 @@ enum punit_power_well {
> >> #define _PIPEBSRC0x6101c
> >> #define _BCLRPAT_B 0x61020
> >> #define _VSYNCSHIFT_B0x61028
> >> +#define _MULTIPLY_B 0x6102c
> >>
> >> #define TRANSCODER_A_OFFSET 0x6
> >> #define TRANSCODER_B_OFFSET 0x61000
> >> @@ -2474,6 +2476,7 @@ enum punit_power_well {
> >> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> >> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> >> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> >> +#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> >>
> >> /* HSW+ eDP PSR registers */
> >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ?
> >> 0x64800 : 0x6f800)
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 858011d..f8c1f11 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc
> >> *crtc)
> >>
> >>intel_set_pipe_timings(intel_crtc);
> >>
> >> + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> >> + I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
> >> + intel_crtc->config.pixel_multiplier - 1);
> >> + }
> >
> > So did you verify that the register really is a transcoder register?
> > Eg. set PIPE_MULT(A) to >1x and use pipe A to drive the EDP transcoder.
>
> I did not verify. This change was done based on the fact that the
> register does not exist in the VPG HTML version of the BPEC for
> Transcoder_EDP, only TRANS_MULT_A, _B, and _C are defined.
>
> Do we have an SI contact that can confirm?
Cc:ing Art.
Art, the confusion here is whether PIPE_MULT is a transcoder register
or a pipe register. BSpec seems to be telling us that it's a transcoder
register but the confusion comes from the fact that the EDP transcoder
doesn't have this register. My theory is that it is a transcoder register,
but since pixel repeat isn't needed for eDP the register isn't present
(or relevant) in the EDP transcoder. Can you clarify this?
Although in this case it would be very easy to test this theory on
actual hardware as I previously suggested.
>
> -Clint
>
>
> >
> >> +
> >>if (intel_crtc->config.has_pch_encoder) {
> >>intel_cpu_transcoder_set_m_n(intel_crtc,
> >> &intel_crtc->config.fdi_m_n, NULL);
> >> @@ -7853,7 +7858,10 @@ static bool haswell_get_pipe_config(struct
> >> intel_crtc *crtc,
> >>pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> >>(I915_READ(IPS_CTL) & IPS_ENABLE);
> >>
> >> - pipe_config->pixel_multiplier = 1;
> >> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> >> + pipe_config->pixel_multiplier =
> >> + I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
> >> + }
> >>
> >>return true;
> >> }
> >> --
> >> 1.7.9.5
> >
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the PTEs after updating them before suspend
On Thu, Sep 25, 2014 at 10:13:12AM +0100, Chris Wilson wrote:
> As we use WC updates of the PTE, we are responsible for notifying the
> hardware when to flush its TLBs. Do so after we zap all the PTEs before
> suspend (and the BIOS tries to read our GTT).
>
> Fixes a regression from
>
> commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
> Author: Ben Widawsky
> Date: Wed Oct 16 09:21:30 2013 -0700
>
> drm/i915: Disable GGTT PTEs on GEN6+ suspend
>
> that survived and continue to cause harm even after
>
> commit e568af1c626031925465a5caaab7cca1303d55c7
> Author: Daniel Vetter
> Date: Wed Mar 26 20:08:20 2014 +0100
>
> drm/i915: Undo gtt scratch pte unmapping again
>
> v2: Trivial rebase.
> v3: Fixes requires pointer dances.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
> Tested-by: ming@intel.com
> Signed-off-by: Chris Wilson
> Cc: sta...@vger.kernel.org
> Cc: Takashi Iwai
> Cc: Paulo Zanoni
> Cc: Todd Previte
> Cc: Daniel Vetter
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 1411613f2174..e42925f76b4b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device
> *dev)
> POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
> }
>
> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
> +{
> + if (INTEL_INFO(dev_priv->dev)->gen < 6) {
> + intel_gtt_chipset_flush();
> + } else {
> + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> + POSTING_READ(GFX_FLSH_CNTL_GEN6);
> + }
> +}
> +
> void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device
> *dev)
> dev_priv->gtt.base.start,
> dev_priv->gtt.base.total,
> true);
> +
> + i915_ggtt_flush(dev_priv);
> }
>
> void i915_gem_restore_gtt_mappings(struct drm_device *dev)
> @@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device
> *dev)
> gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
> }
>
> - i915_gem_chipset_flush(dev);
> + i915_ggtt_flush(dev_priv);
> }
>
> int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make sure PSR is ready for been re-enabled.
On Thu, Sep 25, 2014 at 10:50:51AM -0700, Rodrigo Vivi wrote:
> On Thu, Sep 25, 2014 at 10:36 AM, Paulo Zanoni wrote:
>
> > 2014-09-24 19:16 GMT-03:00 Rodrigo Vivi :
> > > Let's make sure PSR is propperly disabled before to re-enabled it.
> > >
> > > According to Spec, after disabled PSR CTL, the Idle state might occur
> > > up to 24ms, that is one full frame time (1/refresh rate),
> > > plus SRD exit training time (max of 6ms),
> > > plus SRD aux channel handshake (max of 1.5ms).
> > >
> > > So if something went wrong PSR will be disabled until next full
> > > enable/disable setup.
> > >
> > > v2: The 24ms above takes in account 16ms for refresh rate on 60Hz mode.
> > However
> > > on low frequency modes this can take longer. So let's use 50ms for
> > safeness.
> > >
> > > v3: Move wait out of psr.lock critical area.
> > >
> > > Cc: Daniel Vetter
> > > Cc: Paulo Zanoni
> > > Signed-off-by: Rodrigo Vivi
> >
> > Again, since we already waited for 100ms while queueing
> > intel_edp_psr_work, an additional 50ms is basically useless.
>
>
> Agree. But it doesn't hurt it is just a timeout to give more time in case
> psr is still on transition.
> what is unlike I know...
Yeah, looks good enough for now imo, patch merged.
> > I'd
> > really like this function to just have an I915_READ instead of yet
> > another wait, so any necessary wait-time-tuning would be part of the
> > schedule_delayed_work(dev_priv->psr.work) call.
> >
>
> Uhm. that was my first idea actually. I was willing to kill the delayed
> work at all and use just this read scheme.
> However this didn't worked It seems hardware doesn't like when we have
> to much sequential on-off psr switches.
>
> So 100ms is enough to avoid this high frequency on-off and avoid sloweness
> and other issues.
>
> The 50ms extra is just to be on the safe side checking if we can reaable it
> or give a bit more time for it instead of just wait until next full
> enable/disable sequence.
Is there a way to have a less massive psr entry/exit knob? Maybe one that
just does a one-shot upload?
If that doesn't work then I think the timeout is totally ok - if we need
to upload a few frames anyway to keep hw happy then a short delay won't
make things worse really. Hopfully single-shot upload for pageflips still
work.
Cheers, Daniel
>
>
> >
> > That said, the current patch is already an improvement, so:
> > Reviewed-by: Paulo Zanoni
> >
>
> Thank you very much.
>
>
> >
> > But I'd prefer the solution I proposed :)
> >
>
> me too. :)
>
>
> >
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 11 +++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > > index a119b9b..b8699b0 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -2208,6 +2208,17 @@ static void intel_edp_psr_work(struct work_struct
> > *work)
> > > container_of(work, typeof(*dev_priv), psr.work.work);
> > > struct intel_dp *intel_dp = dev_priv->psr.enabled;
> > >
> > > + /* We have to make sure PSR is ready for re-enable
> > > +* otherwise it keeps disabled until next full enable/disable
> > cycle.
> > > +* PSR might take some time to get fully disabled
> > > +* and be ready for re-enable.
> > > +*/
> > > + if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev_priv->dev)) &
> > > + EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
> > > + DRM_ERROR("Timed out waiting for PSR Idle for
> > re-enable\n");
> > > + return;
> > > + }
> > > +
> > > mutex_lock(&dev_priv->psr.lock);
> > > intel_dp = dev_priv->psr.enabled;
> > >
> > > --
> > > 1.9.3
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
> > --
> > Paulo Zanoni
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/11] drm/i915: Clarify event_lock locking, irq&mixed context
On Mon, Sep 29, 2014 at 02:20:27PM +0800, Jike Song wrote: > On 09/15/2014 08:55 PM, Daniel Vetter wrote: > >Now we tackle the functions also called from interrupt handlers. > > > >- intel_check_page_flip is exclusively called from irq handlers, so a > > plain spin_lock is all we need. In i915_irq.c we have the convention > > to give all such functions an _irq_handler postfix, but that would > > look strange and als be a bit a misleading name. I've opted for a > > WARN_ON(!in_irq()) instead. > > Hi Daniel, > > Is it possible to use in_interrupt() instead? Sorry to tell that, in > our iGVT-g implementation, the host i915 irq handler needs to be called > in a non hardirq driven context. i.e. a tasklet or workqueue. Hm, why that? Depending upon how you do this you might break a lot of the interrupt related locking we have ... This is a crucial integration issue, which patch does that change? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Minimize the huge amount of unecessary fbc sw cache clean.
On Wed, Sep 24, 2014 at 07:50:59PM -0400, Rodrigo Vivi wrote:
> The sw cache clean on BDW is a tempoorary workaround because we cannot
> set cache clean on blt ring with risk of hungs. So we are doing the cache
> clean on sw.
> However we are doing much more than needed. Not only when using blt ring.
> So, with this extra w/a we minimize the ammount of cache cleans and call it
> only
> on same cases that it was being called on gen7.
>
> The traditional FBC Cache clean happens over LRI on BLT ring when there is a
> frontbuffer touch happening. frontbuffer tracking set fbc_dirty variable
> to let BLT flush that it must clean FBC cache.
>
> fbc.need_sw_cache_clean works in the opposite information direction
> of ring->fbc_dirty telling software on frontbuffer tracking to perform
> the cache clean on sw side.
>
> v2: Clean it a little bit and fully check for Broadwell instead of gen8.
>
> v3: Rebase after frontbuffer organization.
>
> v4: Wiggle confused me. So fixing v3!
>
> Cc: Daniel Vetter
> Cc: Paulo Zanoni
> Reviewed-by: Paulo Zanoni
> Signed-off-by: Rodrigo Vivi
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 10 +-
> drivers/gpu/drm/i915/intel_frontbuffer.c | 6 --
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++--
> 4 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index da607ab..4cd2aa3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -676,6 +676,14 @@ struct i915_fbc {
>* possible. */
> bool enabled;
>
> + /* On gen8 some rings cannont perform fbc clean operation so for now
> + * we are doing this on SW with mmio.
> + * This variable works in the opposite information direction
> + * of ring->fbc_dirty telling software on frontbuffer tracking
> + * to perform the cache clean on sw side.
> + */
> + bool need_sw_cache_clean;
> +
> struct intel_fbc_work {
> struct delayed_work work;
> struct drm_crtc *crtc;
> @@ -2843,7 +2851,7 @@ extern void intel_modeset_setup_hw_state(struct
> drm_device *dev,
> extern void i915_redisable_vga(struct drm_device *dev);
> extern void i915_redisable_vga_power_on(struct drm_device *dev);
> extern bool intel_fbc_enabled(struct drm_device *dev);
> -extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);
> +extern void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
> extern void intel_disable_fbc(struct drm_device *dev);
> extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
> extern void intel_init_pch_refclk(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
> b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index f74744c..7eb74a6 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -189,8 +189,10 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>* needs to be reworked into a proper frontbuffer tracking scheme like
>* psr employs.
>*/
> - if (IS_BROADWELL(dev))
> - gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
> + if (dev_priv->fbc.need_sw_cache_clean) {
> + dev_priv->fbc.need_sw_cache_clean = false;
> + bdw_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
> + }
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index aaab056..36842c4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -380,7 +380,7 @@ bool intel_fbc_enabled(struct drm_device *dev)
> return dev_priv->fbc.enabled;
> }
>
> -void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
> +void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 896f564..b56e031 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2237,6 +2237,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
> u32 invalidate, u32 flush)
> {
> struct drm_device *dev = ring->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> uint32_t cmd;
> int ret;
>
> @@ -2267,8 +2268,12 @@ static int gen6_ring_flush(struct intel_engine_cs
> *ring,
> }
> intel_ring_advance(ring);
>
> - if (IS_GEN7(dev) && !invalidate && flush)
> - return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
> + if (!invalidate && flush) {
> + if (IS_GEN7(dev))
> + return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
> + else if (IS_BROADWELL(dev))
> + dev_
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.
On Mon, Sep 29, 2014 at 07:44:56PM +0800, Jike Song wrote:
> On 09/19/2014 03:25 PM, Chris Wilson wrote:
> >Now, given that these are simply trapped memory access, wouldn't it be
> >simply to have:
> >
> >struct i915_virtual_gpu {
> > struct vgt_if *if;
> >} vgu;
> >
> >static inline bool intel_vgpu_active(struct drm_i915_private *i915) { return
> >i915->vgpu.if; }
> >
> >then you have constructs like
> >void i915_check_vgpu(struct drm_i915_private *i915)
> >{
> > struct vgt_if *if;
> >
> > if = i915->regs + VGT_PVINFO_PAGE;
> > if (if->magic != VGT_MAGIC)
> > return;
> >
> > if (INTEL_VGT_IF_VERSION !=
> > INTEL_VGT_IF_VERSION_ENCODE(if->version_major,
> > if->version_minor))
> > return;
> >
> >
> > i915->vgpu.if = if;
> >}
> >
> >And later, for example:
> >
> >if (intel_vgpu_active(dev_priv))
> > dev_priv->num_fence_regs = dev_priv->vgpu.if->fence_num;
> >
>
> Hi Chris, sorry that I didn't understand you correctly. After discussion
> with Yu today, I realized that unfortunately, the vgt_if can't be
> dereferenced directly.
>
> There are several reasons:
>
> - dereferencing a MMIO address will be complained by sparse(1)
>
> - for Guest VM, such accesses will be trapped by hypervisor, and
> hence emulated correctly; However this doesn't work for Host(e.g.
> Domain 0 of Xen, the Linux host KVM resides in). For host, we used
> a proactive mechanism to redirect i915 MMIO accesses to vgt,
> the GPU device-model, for the sake of central management & sharing
> among VMs, including host.
You only need to be careful during vgpu detection. After that you know
everything is safe. If you do the detection during intel_uncore_init(),
or similar, you can use raw mmio access and explict sparse annotations
to do the right thing.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.
On 9/29/2014 7:44 PM, Jike Song wrote:
On 09/19/2014 03:25 PM, Chris Wilson wrote:
Now, given that these are simply trapped memory access, wouldn't it be
simply to have:
struct i915_virtual_gpu {
struct vgt_if *if;
} vgu;
static inline bool intel_vgpu_active(struct drm_i915_private *i915) {
return i915->vgpu.if; }
then you have constructs like
void i915_check_vgpu(struct drm_i915_private *i915)
{
struct vgt_if *if;
if = i915->regs + VGT_PVINFO_PAGE;
if (if->magic != VGT_MAGIC)
return;
if (INTEL_VGT_IF_VERSION !=
INTEL_VGT_IF_VERSION_ENCODE(if->version_major,
if->version_minor))
return;
i915->vgpu.if = if;
}
And later, for example:
if (intel_vgpu_active(dev_priv))
dev_priv->num_fence_regs = dev_priv->vgpu.if->fence_num;
Hi Chris, sorry that I didn't understand you correctly. After discussion
with Yu today, I realized that unfortunately, the vgt_if can't be
dereferenced directly.
There are several reasons:
- dereferencing a MMIO address will be complained by sparse(1)
- for Guest VM, such accesses will be trapped by hypervisor, and
hence emulated correctly; However this doesn't work for Host(e.g.
Domain 0 of Xen, the Linux host KVM resides in). For host, we used
a proactive mechanism to redirect i915 MMIO accesses to vgt,
the GPU device-model, for the sake of central management & sharing
among VMs, including host.
Given that, though technically your code works for Guest, but after the
integration of host support of iGVT, we still need to use
I915_READ/I915_WRITE
then. The host patches is soon to posted for your review :)
I should have realized that earlier, sorry!
Hi Chris,
Sorry, I also should have noticed this issue earlier.
To my understanding, the reason you proposed to use the "struct
vgt_if *if" in struct vgpu, to replace the previous vgpu_active, is to
simplify the mmio accesses in our patches. This suggestion works fine
from the guest & native point of view. However, just like Jike's mail
said, this change may not work for the host side, which also need to
visit the PVINFO page from time to time. So, could we still keep the
vgpu_active flag when detecting virtual gpu, and read the mmio registers
in PVINFO structure by I915_READ?
Thanks
Yu
-Chris
--
Thanks,
Jike
___
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 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.
On 09/19/2014 03:25 PM, Chris Wilson wrote:
Now, given that these are simply trapped memory access, wouldn't it be
simply to have:
struct i915_virtual_gpu {
struct vgt_if *if;
} vgu;
static inline bool intel_vgpu_active(struct drm_i915_private *i915) { return
i915->vgpu.if; }
then you have constructs like
void i915_check_vgpu(struct drm_i915_private *i915)
{
struct vgt_if *if;
if = i915->regs + VGT_PVINFO_PAGE;
if (if->magic != VGT_MAGIC)
return;
if (INTEL_VGT_IF_VERSION !=
INTEL_VGT_IF_VERSION_ENCODE(if->version_major,
if->version_minor))
return;
i915->vgpu.if = if;
}
And later, for example:
if (intel_vgpu_active(dev_priv))
dev_priv->num_fence_regs = dev_priv->vgpu.if->fence_num;
Hi Chris, sorry that I didn't understand you correctly. After discussion
with Yu today, I realized that unfortunately, the vgt_if can't be
dereferenced directly.
There are several reasons:
- dereferencing a MMIO address will be complained by sparse(1)
- for Guest VM, such accesses will be trapped by hypervisor, and
hence emulated correctly; However this doesn't work for Host(e.g.
Domain 0 of Xen, the Linux host KVM resides in). For host, we used
a proactive mechanism to redirect i915 MMIO accesses to vgt,
the GPU device-model, for the sake of central management & sharing
among VMs, including host.
Given that, though technically your code works for Guest, but after the
integration of host support of iGVT, we still need to use I915_READ/I915_WRITE
then. The host patches is soon to posted for your review :)
I should have realized that earlier, sorry!
-Chris
--
Thanks,
Jike
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] topic/core-stuff
Hi Dave, Ok, here's the update core-stuff pull request with the locking fixup patch fixed up with another patch. Cheers, Daniel The following changes since commit d743ecf360637d489a3ba81a268f574359149601: drm/doc: Fixup drm_irq kerneldoc includes. (2014-09-24 11:43:47 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-09-29 for you to fetch changes up to 1b11629737ca5414b0310d35e01a125cfde1ba4d: drm: Drop grab fpriv->fbs_lock in drm_fb_release (2014-09-25 17:13:40 +0200) Daniel Vetter (3): drm: Fixup locking for universal cursor planes drm: Improve debug output for drm_wait_one_vblank drm: Drop grab fpriv->fbs_lock in drm_fb_release Fabian Frederick (9): drm/cirrus: use container_of to resolve cirrus_fbdev from drm_fb_helper drm/mgag200: use container_of to resolve mga_fbdev from drm_fb_helper drm/radeon: use container_of to resolve radeon_fbdev from drm_fb_helper drm/nouveau: use container_of to resolve nouveau_fbdev from drm_fb_helper drm/nouveau: use container_of to resolve nouveau_plane from drm_plane drm/qxl: use container_of to resolve qxl_fbdev from drm_fb_helper drm/gma500: use container_of to resolve psb_fbdev from drm_fb_helper drm/ast: use container_of to resolve ast_fbdev from drm_fb_helper drm/udl: use container_of to resolve udl_fbdev from drm_fb_helper Mario Kleiner (1): drm: Don't update vblank timestamp when the counter didn't change drivers/gpu/drm/ast/ast_fb.c | 3 +- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 3 +- drivers/gpu/drm/drm_crtc.c | 63 +- drivers/gpu/drm/drm_irq.c | 7 +++- drivers/gpu/drm/gma500/framebuffer.c | 3 +- drivers/gpu/drm/mgag200/mgag200_fb.c | 3 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 15 --- drivers/gpu/drm/nouveau/nouveau_fbcon.c| 3 +- drivers/gpu/drm/qxl/qxl_fb.c | 3 +- drivers/gpu/drm/radeon/radeon_fb.c | 3 +- drivers/gpu/drm/udl/udl_fb.c | 3 +- 11 files changed, 75 insertions(+), 34 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request
On Fri, Sep 26, 2014 at 11:52:09PM +0200, Rafael J. Wysocki wrote:
> On Friday, September 26, 2014 10:30:08 AM Aaron Lu wrote:
> > Some of the Thinkpads' firmware will issue a backlight change request
> > through i915 operation region unconditionally on AC plug/unplug, the
> > backlight level used is arbitrary and thus should be ignored. This is
> > handled by commit 0b9f7d93ca61 (ACPI / i915: ignore firmware requests
> > for backlight change). Then there is a Dell laptop whose vendor backlight
> > interface also makes use of operation region to change backlight level
> > and with the above commit, that interface no long works. The condition
> > used to ignore the backlight change request from firmware is thus
> > changed to: if the vendor backlight interface is not in use and the ACPI
> > backlight interface is broken, we ignore the requests; oterwise, we keep
> > processing them.
> >
> > Reference: https://lkml.org/lkml/2014/9/23/854
> > Reported-and-tested-by: Pali Rohár
> > Cc: # v3.16 and later
> > Signed-off-by: Aaron Lu
>
> Daniel, any objections?
Nope, ack from my side.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_opregion.c | 16 +++-
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> > b/drivers/gpu/drm/i915/intel_opregion.c
> > index ca52ad2ae7d1..d8de1d5140a7 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -396,6 +396,16 @@ int intel_opregion_notify_adapter(struct drm_device
> > *dev, pci_power_t state)
> > return -EINVAL;
> > }
> >
> > +/*
> > + * If the vendor backlight interface is not in use and ACPI backlight
> > interface
> > + * is broken, do not bother processing backlight change requests from
> > firmware.
> > + */
> > +static bool should_ignore_backlight_request(void)
> > +{
> > + return acpi_video_backlight_support() &&
> > + !acpi_video_verify_backlight_support();
> > +}
> > +
> > static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -404,11 +414,7 @@ static u32 asle_set_backlight(struct drm_device *dev,
> > u32 bclp)
> >
> > DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
> >
> > - /*
> > -* If the acpi_video interface is not supposed to be used, don't
> > -* bother processing backlight level change requests from firmware.
> > -*/
> > - if (!acpi_video_verify_backlight_support()) {
> > + if (should_ignore_backlight_request()) {
> > DRM_DEBUG_KMS("opregion backlight request ignored\n");
> > return 0;
> > }
> >
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
