Re: [Intel-gfx] [PATCH 3/7] drm/exynos: Renaming DP training vswing pre emph defines
On Thursday, August 28, 2014 1:33 PM, Sonika Sonika wrote: On 8/28/2014 6:25 AM, Jingoo Han wrote: On Friday, August 08, 2014 7:54 PM, Sonika Jindal wrote: From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_400 (0 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) ... Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/exynos/exynos_dp_core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index 4f3c7eb..02602a8 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -329,8 +329,8 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp) return retval; for (lane = 0; lane lane_count; lane++) - buf[lane] = DP_TRAIN_PRE_EMPHASIS_0 | - DP_TRAIN_VOLTAGE_SWING_400; + buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 | + DP_TRAIN_VOLTAGE_SWING_LEVEL_0; NAK! It makes build error. Please build your patch, before sending the patch. It is a rule when submitting patches. Please, fix it as follows. + buf[lane] = DP_TRAIN_PRE_EMPHASIS_LEVEL_0| + DP_TRAIN_VOLTAGE_SWING_LEVEL_0; I think the first patch which you have taken (which adds new defines) is the one from the previous series for the same change. In the second version, I have named them as DP_TRAIN_PRE_EMPH_LEVEL_* which was done using cocci. Following is from that patch: Oh, I see. Sorry for annoying you. However, how about tagging V2, V3.. into patches? For instance, '[PATCH V2 3/7] drm/exynos: Renaming DP training vswing pre emph defines' It will be helpful, in order to prevent the same mistakes happening again. Also, the patch looks good. Acked-by: Jingoo Han jg1@samsung.com Best regards, Jingoo Han # define DP_TRAIN_PRE_EMPHASIS_0(0 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_0 (0 3) # define DP_TRAIN_PRE_EMPHASIS_3_5 (1 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_1 (1 3) # define DP_TRAIN_PRE_EMPHASIS_6(2 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_2 (2 3) # define DP_TRAIN_PRE_EMPHASIS_9_5 (3 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_3 (3 3) Best regards, Jingoo Han retval = exynos_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET, lane_count, buf); -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/exynos: Renaming DP training vswing pre emph defines
On 8/28/2014 11:36 AM, Jingoo Han wrote: On Thursday, August 28, 2014 1:33 PM, Sonika Sonika wrote: On 8/28/2014 6:25 AM, Jingoo Han wrote: On Friday, August 08, 2014 7:54 PM, Sonika Jindal wrote: From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_400 (0 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) ... Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/exynos/exynos_dp_core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index 4f3c7eb..02602a8 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -329,8 +329,8 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp) return retval; for (lane = 0; lane lane_count; lane++) - buf[lane] = DP_TRAIN_PRE_EMPHASIS_0 | - DP_TRAIN_VOLTAGE_SWING_400; + buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 | + DP_TRAIN_VOLTAGE_SWING_LEVEL_0; NAK! It makes build error. Please build your patch, before sending the patch. It is a rule when submitting patches. Please, fix it as follows. + buf[lane] = DP_TRAIN_PRE_EMPHASIS_LEVEL_0| + DP_TRAIN_VOLTAGE_SWING_LEVEL_0; I think the first patch which you have taken (which adds new defines) is the one from the previous series for the same change. In the second version, I have named them as DP_TRAIN_PRE_EMPH_LEVEL_* which was done using cocci. Following is from that patch: Oh, I see. Sorry for annoying you. However, how about tagging V2, V3.. into patches? For instance, '[PATCH V2 3/7] drm/exynos: Renaming DP training vswing pre emph defines' It will be helpful, in order to prevent the same mistakes happening again. Actually I had bumped the version in the cover letter. Because last time I had changed the version of all the patches (for some other feature), somebody asked me to just change the version at the top when it is a series. Also, this was a different patch altogether done using cocci, so thought it should be fine. Will take care next time :) Thanks, Sonika Also, the patch looks good. Acked-by: Jingoo Han jg1@samsung.com Best regards, Jingoo Han # define DP_TRAIN_PRE_EMPHASIS_0 (0 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_0 (0 3) # define DP_TRAIN_PRE_EMPHASIS_3_5 (1 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_1 (1 3) # define DP_TRAIN_PRE_EMPHASIS_6 (2 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_2 (2 3) # define DP_TRAIN_PRE_EMPHASIS_9_5 (3 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_3 (3 3) Best regards, Jingoo Han retval = exynos_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET, lane_count, buf); -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/gem_workarounds: igt to test workaround registers
On Thu, Aug 28, 2014 at 06:55:37AM +0100, Chris Wilson wrote: On Wed, Aug 27, 2014 at 11:30:35PM +0100, Damien Lespiau wrote: On Wed, Aug 27, 2014 at 06:52:57PM +0100, Chris Wilson wrote: Just to clarify, he was not ok because the list we maintain in the test can get out of sync with the workarounds we apply in the driver which can be avoided if it is generated by the kernel itself. Test driven development would suggest that the test itself maintains its list. Something I heard Daniel say himself before ;-) It may be ok to maintain the list in the test in this case considering the list is fairly small but it is not my call. The best thing about independent testing is that it is independent... Well also depends on what you're testing I suppose. It's hard enough to have a complete list of W/As, so two of them is bound to end up in tears. If we are testing that the list of W/As the kernel knows about is indeed applied correctly at init/reset/suspend resume, that's already a good step. Also, that second list in i-g-t is not going to be implemented in complete independence from the kernel tree, it's likely to be the same person doing both sides, ending up copy/pasting a file anyway, no value in doing that. The two lists argument works well if 2 different engineers/teams implement the 2 sides, effective cross-checking the list of W/As as a result, but we don't quite have the people to do that. The point of the independent test is more that you can ask people to run and see if it reports strange things on unknown kernels that might explain bugs. There has to be an external list anyway just so that you can check off the appropriate w/a. Putting that second list in the kernel just leads to bugs in the kernel as aptly demonstrated by the patch and doesn't lead to those warm fuzzy feelings. Ah, fair, for those points it'd be ok to just isolate the W/As in a file, decide that the master copy is in the kernel and sync it from the kernel to i-g-t when it changes. Not the full independent testing I was thinking about with separate people writing code and validation. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm: Renaming DP training vswing pre emph defines
On Wed, Aug 27, 2014 at 03:11:08PM +0200, Thierry Reding wrote: So we're left with #define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) Vs #define DP_TRAIN_VOLTAGE_SWING_LEVEL(x) ((x) 0) The second variant doesn't really bring much more clarity? Can we just go with the first? I think the parameterized version is more convenient, especially if you want to use that during training sequences and iterate over the levels. That's a fair point, but today's code manages to do without that nicety. I think these kind of refinements could go in series with code actually using them on top. But I don't feel too strongly about it, so either way is fine with me. Thanks, taking some of your time to provide feedback is always appreciated! -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks
On Wed, 27 Aug 2014, Mathias Krause mini...@googlemail.com wrote:
The __init annotations for the DMI callback functions are wrong as this
code can be called even after the module has been initialized, e.g. like
this:
# echo 1 /sys/bus/pci/devices/:00:02.0/remove
# modprobe i915
# echo 1 /sys/bus/pci/rescan
The first command will remove the PCI device from the kernel's device
list so the second command won't see it right away. But as it registers
a PCI driver it'll see it on the third command. If the system happens to
match one of the DMI table entries we'll try to call a function in long
released memory and generate an Oops, at best.
Fix this by removing the bogus annotation.
Modpost should have caught that one but it ignores section reference
mismatches from the .rodata section. :/
Fixes: 25e341cfc33d (drm/i915: quirk away broken OpRegion VBT)
Fixes: 8ca4013d702d (CHROMIUM: i915: Add DMI override to skip CRT...)
Fixes: 425d244c8670 (drm/i915: ignore LVDS on intel graphics systems...)
Signed-off-by: Mathias Krause mini...@googlemail.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Duncan Laurie dlau...@chromium.org
Cc: Jarod Wilson ja...@redhat.com
Cc: Rusty Russell ru...@rustcorp.com.au # Can modpost be fixed?
Cc: sta...@vger.kernel.org
Nice catch! Thanks for the patch, pushed to drm-intel-fixes.
BR,
Jani.
---
In the long run me might want to move the DMI tests to some __init code
to be able to mark the DMI tables as __initconst, thereby allowing to
release this memory after module initialization. That would safe us some
~11 kB of memory, as the DMI data shouldn't change at run-time.
drivers/gpu/drm/i915/intel_bios.c |2 +-
drivers/gpu/drm/i915/intel_crt.c |2 +-
drivers/gpu/drm/i915/intel_lvds.c |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index a66955037e4e..eee79e1c3222 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1123,7 +1123,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
}
}
-static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id
*id)
+static int intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
{
DRM_DEBUG_KMS(Falling back to manually reading VBT from
VBIOS ROM for %s\n,
diff --git a/drivers/gpu/drm/i915/intel_crt.c
b/drivers/gpu/drm/i915/intel_crt.c
index e8abfce40976..9212e6504e0f 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -804,7 +804,7 @@ static const struct drm_encoder_funcs intel_crt_enc_funcs
= {
.destroy = intel_encoder_destroy,
};
-static int __init intel_no_crt_dmi_callback(const struct dmi_system_id *id)
+static int intel_no_crt_dmi_callback(const struct dmi_system_id *id)
{
DRM_INFO(Skipping CRT initialization for %s\n, id-ident);
return 1;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index 881361c0f27e..fdf40267249c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -538,7 +538,7 @@ static const struct drm_encoder_funcs
intel_lvds_enc_funcs = {
.destroy = intel_encoder_destroy,
};
-static int __init intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
+static int intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
{
DRM_INFO(Skipping LVDS initialization for %s\n, id-ident);
return 1;
--
1.7.10.4
--
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] [QA 08/28 ww35] Testing report for `drm-intel-testing` (was: Updated -next)
Summary We covered the platform: Broadwell, Baytrail-M, Haswell (mobile, desktop and ULT), Ivybridge, SandyBridge, IronLake. In this circle, 1 new bugs is filed. Bug 83094https://bugs.freedesktop.org/show_bug.cgi?id=83094 - [BDW Bisected]System dmesg contains ERROR when machine resumes from s3 Finding 4 Top bugs Bug 82340https://bugs.freedesktop.org/show_bug.cgi?id=82340 - [HSW Bisected] Fail to resume from S4, causing system hang (once out of 5 test circles) Bug 81948https://bugs.freedesktop.org/show_bug.cgi?id=81948 - [BYT-M Regression]System unable resume from S3/S4 Bug 82842https://bugs.freedesktop.org/show_bug.cgi?id=82842 - [BYT-M]System unable go to S4 sporadically Bug 82593https://bugs.freedesktop.org/show_bug.cgi?id=82593 - [BYT]udevadm unable to monitor HDMI plug/unplug event sporadically Test Environment Kernel version: commit 26b521a9d5dd5d31eeea20884062788ba19e5e1c Merge: e1791f6 80b36f4 Author: Jani Nikula jani.nik...@intel.com Date: Thu Aug 21 10:04:12 2014 +0300 Merge remote-tracking branch 'origin/topic/vblank-rework' into drm-intel-nightly Kernel Bugs Status: [cid:image001.png@01CFC2E0.3A64C860] Power consumption : HSWU36 drm-intel-testing-08-23 save consumption i915.enable_fbc=1 0.44% i915.enable_fbc=1=-1 (default)disable i915.disable_power_well=1 0.47% i915.disable_power_well=0 BDW21 drm-intel-testing-08-23 i915.enable_fbc=1 0.54% i915.enable_fbc=1=-1 (default)disable i915.disable_power_well=1 0.51% i915.disable_power_well=0 Thanks --Sun, Yi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/bdw: BDW Software Turbo
On Wed, Aug 27, 2014 at 08:57:56PM +0200, Daniel Vetter wrote: On Thu, Aug 14, 2014 at 12:37:53PM -0700, Jesse Barnes wrote: On Mon, 11 Aug 2014 23:33:57 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Mon, Aug 11, 2014 at 11:08:38AM -0700, Daisy Sun wrote: BDW supports GT C0 residency reporting in constant time unit. Driver calculates GT utilization based on C0 residency and adjusts RP frequency up/down accordingly. For offscreen workload specificly, set frequency to RP0. Offscreen task is not restricted by frame rate, it can be executed as soon as possible. Transcoding and serilized workload between CPU and GPU both need high GT performance, RP0 is a good option in this case. RC6 will kick in to compensate power consumption when GT is not active. v2: Rebase on recent drm-intel-nightly v3: Add flip timerout monitor, when no flip is deteced within 100ms, set frequency to RP0. Ok, let's make this really clear: If you wire this into the flip handling in any way, I will not merge your patch. The timer should be fully independant and tie into the gpu busy/idle handling we already have. Sounds like Daisy won't be able to spend any more time on this either. So we're left with this patch, which does improve things for most cases, or no patch, which leaves things universally bad. Unless someone wants to pick up the additional work and testing of using a timer scheme, making sure we don't have needless wakeups, and generally improve power/perf across even more cases than this patch. I'm taking this as an ack from you and pulled the patch into dinq. Maybe also a nak from me for the bad design and poor integration with the existing RPS infrastructue? -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/bdw: Render state init for Execlists
On Thu, Aug 21, 2014 at 11:40:54AM +0100, Thomas Daniel wrote:
From: Oscar Mateo oscar.ma...@intel.com
The batchbuffer that sets the render context state is submitted
in a different way, and from different places.
We needed to make both the render state preparation and free functions
outside accesible, and namespace accordingly. This mess is so that all
LR, LRC and Execlists functionality can go together in intel_lrc.c: we
can fix all of this later on, once the interfaces are clear.
v2: Create a separate ctx-rcs_initialized for the Execlists case, as
suggested by Chris Wilson.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
v3: Setup ring status page in lr_context_deferred_create when the
default context is being created. This means that the render state
init for the default context is no longer a special case. Execute
deferred creation of the default context at the end of
logical_ring_init to allow the render state commands to be submitted.
Fix style errors reported by checkpatch. Rebased.
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.h |4 +-
drivers/gpu/drm/i915/i915_gem_render_state.c | 40 --
drivers/gpu/drm/i915/i915_gem_render_state.h | 47 +
drivers/gpu/drm/i915/intel_lrc.c | 73
--
drivers/gpu/drm/i915/intel_lrc.h |2 +
drivers/gpu/drm/i915/intel_renderstate.h |8 +--
6 files changed, 135 insertions(+), 39 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.h
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e449f81..f416e341 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -37,6 +37,7 @@
#include intel_ringbuffer.h
#include intel_lrc.h
#include i915_gem_gtt.h
+#include i915_gem_render_state.h
#include linux/io-mapping.h
#include linux/i2c.h
#include linux/i2c-algo-bit.h
@@ -635,6 +636,7 @@ struct intel_context {
} legacy_hw_ctx;
/* Execlists */
+ bool rcs_initialized;
struct {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
@@ -2596,8 +2598,6 @@ int i915_gem_context_create_ioctl(struct drm_device
*dev, void *data,
int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
-/* i915_gem_render_state.c */
-int i915_gem_render_state_init(struct intel_engine_cs *ring);
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev,
struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index e60be3f..a9a62d7 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -28,13 +28,6 @@
#include i915_drv.h
#include intel_renderstate.h
-struct render_state {
- const struct intel_renderstate_rodata *rodata;
- struct drm_i915_gem_object *obj;
- u64 ggtt_offset;
- int gen;
-};
-
static const struct intel_renderstate_rodata *
render_state_get_rodata(struct drm_device *dev, const int gen)
{
@@ -127,30 +120,47 @@ static int render_state_setup(struct render_state *so)
return 0;
}
-static void render_state_fini(struct render_state *so)
+void i915_gem_render_state_fini(struct render_state *so)
{
i915_gem_object_ggtt_unpin(so-obj);
drm_gem_object_unreference(so-obj-base);
}
-int i915_gem_render_state_init(struct intel_engine_cs *ring)
+int i915_gem_render_state_prepare(struct intel_engine_cs *ring,
+ struct render_state *so)
{
- struct render_state so;
int ret;
if (WARN_ON(ring-id != RCS))
return -ENOENT;
- ret = render_state_init(so, ring-dev);
+ ret = render_state_init(so, ring-dev);
if (ret)
return ret;
- if (so.rodata == NULL)
+ if (so-rodata == NULL)
return 0;
- ret = render_state_setup(so);
+ ret = render_state_setup(so);
+ if (ret) {
+ i915_gem_render_state_fini(so);
+ return ret;
+ }
+
+ return 0;
+}
+
+int i915_gem_render_state_init(struct intel_engine_cs *ring)
+{
+ struct render_state so;
+ int ret;
+
+ ret = i915_gem_render_state_prepare(ring, so);
if (ret)
- goto out;
+ return ret;
+
+ if (so.rodata == NULL)
+ return 0;
ret = ring-dispatch_execbuffer(ring,
so.ggtt_offset,
@@ -164,6 +174,6 @@ int i915_gem_render_state_init(struct intel_engine_cs
*ring)
ret = __i915_add_request(ring, NULL, so.obj,
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave - Some more fixes for 3.17, mostly stable material. BR, Jani. The following changes since commit 52addcf9d6669fa439387610bc65c92fa0980cef: Linux 3.17-rc2 (2014-08-25 15:36:20 -0700) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-08-28 for you to fetch changes up to bbe1c2740d3a25aa1dbe5d842d2ff09cddcdde0a: drm/i915: Remove bogus __init annotation from DMI callbacks (2014-08-28 09:54:27 +0300) Mathias Krause (1): drm/i915: Remove bogus __init annotation from DMI callbacks Paulo Zanoni (1): drm/i915: fix plane/cursor handling when runtime suspended Scot Doyle (2): drm/i915: Ignore VBT backlight presence check on Acer C720 (4005U) drm/i915: don't warn if backlight unexpectedly enabled Ville Syrjälä (1): drm/i915: Move intel_ddi_set_vc_payload_alloc(false) to haswell_crtc_disable() drivers/gpu/drm/i915/intel_bios.c| 2 +- drivers/gpu/drm/i915/intel_crt.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 34 ++ drivers/gpu/drm/i915/intel_lvds.c| 2 +- drivers/gpu/drm/i915/intel_panel.c | 8 5 files changed, 37 insertions(+), 11 deletions(-) -- 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] i915: Regression: +4W in idle power use on Macbook Pro 15 (late 2013)
On Mon, Aug 25, 2014 at 3:22 AM, Jani Nikula jani.nik...@intel.com wrote: [just moving from lkml to intel-gfx for a better fitting audience] On Mon, 25 Aug 2014, Jani Nikula jani.nik...@intel.com wrote: On Fri, 22 Aug 2014, Eric Rannaud eric.rann...@gmail.com wrote: Hi, Between 3.15.4 and 3.15.8, there was an increase in idle power consumption on Apple Macbook Pro 15 (late 2013) on a freshly booted system (no wifi driver loaded; brightness set to 4/100; X running; no desktop environment, except Awesome), from 6.5W to about 10.5W, as reported by powertop. In the stable tree, it bisects to: commit f4db98240ac2c6d9d2118c6f82d483ff5293f1ed Author: Chris Wilson ch...@chris-wilson.co.uk Date: Fri Jun 6 10:37:11 2014 +0100 drm/i915: Disable FBC by default also on Haswell and later commit 0368920e51ae0cded0eb518c340a4dd17764d461 upstream. It causes black screen on bootup and is approximately 100x slower than running with FBC disabled, so the GPU runs at a high frequency for much longer - completely contrary to the power saving claims. It also still has mutex deadlocks in multi-head scenarios, which can lead to a system/X lockup. These bugs were known before FBC was enabled by default on Haswell and still have not been fixed. The issue is still present in Linus' tree (v3.17-rc1-22-g480cadc2b7e0). With a 75Wh battery, that's a significant loss in battery life in normal use. I'll be happy to help test any potential fix. The earlier regression trumps, and in this case it was enabling FBC by default on Haswell. Sorry. You can enable FBC with i915.enable_fbc=1 module parameter, but all bets are off. See the commit message you quoted above. I don't recommend. For what it's worth, I have a Mid-2014, Macbook Pro Retina (13inch display), running Archlinux with 3.16. Definitely, enable_fbc is a win for me and I do manually enable it. But I am still seeing what I believe to be a regression overall of about +4W even with fbc enabled. Still digging for clues. Sean -- Sean V. Kelley sean.v.kel...@intel.com Open Source Technology Center / SSG Intel Corp. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: Regression: +4W in idle power use on Macbook Pro 15 (late 2013)
On Thu, Aug 28, 2014 at 9:41 AM, Sean V Kelley sean.v.kel...@intel.com wrote: For what it's worth, I have a Mid-2014, Macbook Pro Retina (13inch display), running Archlinux with 3.16. Definitely, enable_fbc is a win for me and I do manually enable it. But I am still seeing what I believe to be a regression overall of about +4W even with fbc enabled. Still digging for clues. To be clear, because I don't think I listed that case, with 3.17.0-rc2-ARCH-00040-gff0c57ac7043 and i915.enable_fbc=1 the idle power consumption goes back down to under 7W, which is about back to normal. There does seem to be a roughly +500mW regression since 3.13, but it's a little hard to tell because of the sampling noise in the power information as reported by powertop. Is there a systematic, large-scale effort to obtain baseline power data over time on various configs? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/9] drm/i915: split intel_update_plane into check() and commit()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Due to the upcoming atomic modesetting feature we need to separate
some update functions into a check step that can fail and a commit
step that should, ideally, never fail.
This commit splits intel_update_plane() and its commit part can still
fail due to the fb pinning procedure.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_sprite.c | 128 ++--
1 file changed, 93 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 4cbe286..b1cb606 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane
*intel_plane)
return key.flags != I915_SET_COLORKEY_NONE;
}
+static bool get_visible(struct drm_rect *src, struct drm_rect *dst,
+ const struct drm_rect *clip,
+ int min_scale, int max_scale)
+{
+ int hscale, vscale;
+
+ hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
+ BUG_ON(hscale 0);
+
+ vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
+ BUG_ON(vscale 0);
+
+ return drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+}
+
static int
-intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+intel_check_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
unsigned int crtc_w, unsigned int crtc_h,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
- struct drm_device *dev = plane-dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
- enum pipe pipe = intel_crtc-pipe;
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
struct drm_i915_gem_object *obj = intel_fb-obj;
- struct drm_i915_gem_object *old_obj = intel_plane-obj;
- int ret;
- bool primary_enabled;
bool visible;
int hscale, vscale;
int max_scale, min_scale;
@@ -882,20 +892,6 @@ intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
.x2 = intel_crtc-active ? intel_crtc-config.pipe_src_w : 0,
.y2 = intel_crtc-active ? intel_crtc-config.pipe_src_h : 0,
};
- const struct {
- int crtc_x, crtc_y;
- unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y, src_w, src_h;
- } orig = {
- .crtc_x = crtc_x,
- .crtc_y = crtc_y,
- .crtc_w = crtc_w,
- .crtc_h = crtc_h,
- .src_x = src_x,
- .src_y = src_y,
- .src_w = src_w,
- .src_h = src_h,
- };
/* Don't modify another pipe's plane */
if (intel_plane-pipe != intel_crtc-pipe) {
@@ -930,13 +926,7 @@ intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
drm_rect_rotate(src, fb-width 16, fb-height 16,
intel_plane-rotation);
- hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
- BUG_ON(hscale 0);
-
- vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
- BUG_ON(vscale 0);
-
- visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+ visible = get_visible(src, dst, clip, max_scale, min_scale);
crtc_x = dst.x1;
crtc_y = dst.y1;
@@ -1027,6 +1017,54 @@ intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
}
}
+ return 0;
+}
+
+static int
+intel_commit_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = plane-dev;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ enum pipe pipe = intel_crtc-pipe;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+ struct drm_i915_gem_object *old_obj = intel_plane-obj;
+ int ret;
+ bool primary_enabled;
+ bool visible;
+ int max_scale, min_scale;
+ struct drm_rect src = {
+ /* sample coordinates in 16.16 fixed point */
+ .x1 = src_x,
+ .x2 = src_x + src_w,
+ .y1 = src_y,
+ .y2 = src_y + src_h,
+ };
+ struct drm_rect dst = {
+ /* integer pixels */
+
[Intel-gfx] [PATCH 8/9] drm/i915: return error if fb is NULL
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
intel_pipe_check_base() should return an error is the fb received is
set to NULL.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 42bd6c6..eb6febf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2681,7 +2681,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
/* no fb bound */
if (!fb) {
DRM_ERROR(No FB bound\n);
- return 0;
+ return -EINVAL;
}
if (intel_crtc-plane INTEL_INFO(dev)-num_pipes) {
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 9/9] drm/i915: split intel_pipe_set_base() into check() and commit()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Take out some parts of code that can fail from it and move them to
intel_pipe_check_base(), the only failure point in intel_pipe_set_base()
now is the fb pinning procudure.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 39
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index eb6febf..ead2f24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2661,17 +2661,11 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
*crtc)
}
static int
-intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
+intel_pipe_check_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc-dev;
- struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum pipe pipe = intel_crtc-pipe;
- struct drm_framebuffer *old_fb = crtc-primary-fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
- int ret;
if (intel_crtc_has_pending_flip(crtc)) {
DRM_ERROR(pipe is still busy with an old pageflip\n);
@@ -2691,6 +2685,22 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return -EINVAL;
}
+ return 0;
+}
+
+static int
+intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
+ struct drm_framebuffer *fb)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum pipe pipe = intel_crtc-pipe;
+ struct drm_framebuffer *old_fb = crtc-primary-fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
+ int ret;
+
mutex_lock(dev-struct_mutex);
ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
if (ret == 0)
@@ -9868,6 +9878,10 @@ free_work:
if (ret == -EIO) {
out_hang:
intel_crtc_wait_for_pending_flips(crtc);
+ ret = intel_pipe_check_base(crtc, crtc-x, crtc-y, fb);
+ if (ret)
+ return ret;
+
ret = intel_pipe_set_base(crtc, crtc-x, crtc-y, fb);
if (ret == 0 event)
drm_send_vblank_event(dev, pipe, event);
@@ -11396,6 +11410,10 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
intel_crtc_wait_for_pending_flips(set-crtc);
+ ret = intel_pipe_check_base(set-crtc, set-x, set-y, set-fb);
+ if (ret)
+ goto fail;
+
ret = intel_pipe_set_base(set-crtc,
set-x, set-y, set-fb);
@@ -11620,12 +11638,17 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_crtc *crtc,
.x2 = intel_crtc-active ? intel_crtc-config.pipe_src_w : 0,
.y2 = intel_crtc-active ? intel_crtc-config.pipe_src_h : 0,
};
+ int ret;
- return drm_plane_helper_check_update(plane, crtc, fb,
+ ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true, visible);
+ if (ret)
+ return ret;
+
+ return intel_pipe_check_base(crtc, src.x1, src.y1, fb);
}
static int
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: split intel_crtc_cursor_set_obj() into check() and commit()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Create intel_crtc_cursor_create_obj() to check any need setting
before we call intel_crtc_cursor_set_obj() to apply the cursor updates.
intel_crtc_cursor_check_obj() must always be called before
intel_crtc_cursor_set_obj().
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 115 ---
1 file changed, 80 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0173c53..86c8fa3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8232,30 +8232,25 @@ static bool cursor_size_ok(struct drm_device *dev,
}
/*
- * intel_crtc_cursor_set_obj - Set cursor to specified GEM object
+ * intel_crtc_cursor_check_obj - Check settings for a specified GEM object
*
- * Note that the object's reference will be consumed if the update fails. If
- * the update succeeds, the reference of the old object (if any) will be
- * consumed.
+ * Note that the object's reference will be consumed if the check fails. If
+ * the check succeeds, the reference of the old object (if any) will be
+ * consumed in intel_crtc_cursor_set_obj(). It must be called before
+ * intel_crtc_cursor_set_obj()
*/
-static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
+static int intel_crtc_cursor_check_obj(struct drm_crtc *crtc,
struct drm_i915_gem_object *obj,
uint32_t width, uint32_t height)
+
{
struct drm_device *dev = crtc-dev;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum pipe pipe = intel_crtc-pipe;
- unsigned old_width, stride;
- uint32_t addr;
+ unsigned stride;
int ret;
/* if we want to turn off the cursor ignore width and height */
- if (!obj) {
- DRM_DEBUG_KMS(cursor off\n);
- addr = 0;
- mutex_lock(dev-struct_mutex);
- goto finish;
- }
+ if (!obj)
+ return 0;
/* Check for which cursor types we support */
if (!cursor_size_ok(dev, width, height)) {
@@ -8302,7 +8297,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
goto fail_unpin;
}
- addr = i915_gem_obj_ggtt_offset(obj);
} else {
int align = IS_I830(dev) ? 16 * 1024 : 256;
ret = i915_gem_object_attach_phys(obj, align);
@@ -8310,8 +8304,50 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
DRM_DEBUG_KMS(failed to attach phys object\n);
goto fail_locked;
}
- addr = obj-phys_handle-busaddr;
}
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+
+fail_unpin:
+ i915_gem_object_unpin_from_display_plane(obj);
+fail_locked:
+ mutex_unlock(dev-struct_mutex);
+fail:
+ drm_gem_object_unreference_unlocked(obj-base);
+ return ret;
+}
+
+/*
+ * intel_crtc_cursor_set_obj - Set cursor to specified GEM object
+ *
+ * intel_crtc_cursor_check_obj() must be called before this function
+ * so check for failures can be done before apply the update.
+ */
+static void intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
+struct drm_i915_gem_object *obj,
+uint32_t width, uint32_t height)
+{
+ struct drm_device *dev = crtc-dev;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum pipe pipe = intel_crtc-pipe;
+ unsigned old_width;
+ uint32_t addr;
+
+ /* if we want to turn off the cursor ignore width and height */
+ if (!obj) {
+ DRM_DEBUG_KMS(cursor off\n);
+ addr = 0;
+ mutex_lock(dev-struct_mutex);
+ goto finish;
+ }
+
+ /* we only need to pin inside GTT if cursor is non-phy */
+ mutex_lock(dev-struct_mutex);
+ if (!INTEL_INFO(dev)-cursor_needs_physical)
+ addr = i915_gem_obj_ggtt_offset(obj);
+ else
+ addr = obj-phys_handle-busaddr;
finish:
if (intel_crtc-cursor_bo) {
@@ -8337,15 +8373,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
}
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
-
- return 0;
-fail_unpin:
- i915_gem_object_unpin_from_display_plane(obj);
-fail_locked:
- mutex_unlock(dev-struct_mutex);
-fail:
- drm_gem_object_unreference_unlocked(obj-base);
- return ret;
}
static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
@@ -11733,12 +11760,20 @@ static struct drm_plane
*intel_primary_plane_create(struct drm_device *dev,
static int
intel_cursor_plane_disable(struct drm_plane *plane)
{
+ int ret;
+
[Intel-gfx] [PATCH 3/9] drm/i915: fix memleak in intel_set_config_save_state()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk If the save_encoder_crtcs or save_connector_encoders allocation fail we need to free everything we have already allocated. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 05937fe..a8a8abe 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11018,13 +11018,13 @@ static int intel_set_config_save_state(struct drm_device *dev, kcalloc(dev-mode_config.num_encoder, sizeof(struct drm_crtc *), GFP_KERNEL); if (!config-save_encoder_crtcs) - return -ENOMEM; + goto free_crtc; config-save_connector_encoders = kcalloc(dev-mode_config.num_connector, sizeof(struct drm_encoder *), GFP_KERNEL); if (!config-save_connector_encoders) - return -ENOMEM; + goto free_encoder; /* Copy data. Note that driver private data is not affected. * Should anything bad happen only the expected state is @@ -11046,6 +11046,12 @@ static int intel_set_config_save_state(struct drm_device *dev, } return 0; + +free_encoder: + kfree(config-save_encoder_crtcs); +free_crtc: + kfree(config-save_crtc_enabled); + return -ENOMEM; } static void intel_set_config_restore_state(struct drm_device *dev, -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/9] drm/i915: init sprites with univeral plane init function
From: Derek Foreman derek.fore...@collabora.co.uk
Really just for completeness - old init function ends up making the plane
exactly the same way due to the way the enums are set up.
Signed-off-by: Derek Foreman derek.fore...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_sprite.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 0bdb00b..4cbe286 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe
pipe, int plane)
intel_plane-plane = plane;
intel_plane-rotation = BIT(DRM_ROTATE_0);
possible_crtcs = (1 pipe);
- ret = drm_plane_init(dev, intel_plane-base, possible_crtcs,
-intel_plane_funcs,
-plane_formats, num_plane_formats,
-false);
+ ret = drm_universal_plane_init(dev, intel_plane-base, possible_crtcs,
+ intel_plane_funcs,
+ plane_formats, num_plane_formats,
+ DRM_PLANE_TYPE_OVERLAY);
if (ret) {
kfree(intel_plane);
goto out;
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915: split intel_primary_plane_setplane() into check() and commit()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
As a preparation for atomic updates we need to split the code to check
everything we are going to commit first. This patch starts the work to
split intel_primary_plane_setplane() into check() and commit() parts.
More work is expected on this to get a better split of the two steps.
Ideally the commit() step should never fail.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 63
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 86c8fa3..42bd6c6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11594,16 +11594,13 @@ disable_unpin:
}
static int
-intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
-struct drm_framebuffer *fb, int crtc_x, int crtc_y,
-unsigned int crtc_w, unsigned int crtc_h,
-uint32_t src_x, uint32_t src_y,
-uint32_t src_w, uint32_t src_h)
+intel_check_primary_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y, uint32_t src_w,
+ uint32_t src_h, bool *visible)
{
- struct drm_device *dev = crtc-dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(plane-fb);
struct drm_rect dest = {
/* integer pixels */
.x1 = crtc_x,
@@ -11623,17 +11620,33 @@ intel_primary_plane_setplane(struct drm_plane *plane,
struct drm_crtc *crtc,
.x2 = intel_crtc-active ? intel_crtc-config.pipe_src_w : 0,
.y2 = intel_crtc-active ? intel_crtc-config.pipe_src_h : 0,
};
- bool visible;
- int ret;
- ret = drm_plane_helper_check_update(plane, crtc, fb,
+ return drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
- false, true, visible);
+ false, true, visible);
+}
- if (ret)
- return ret;
+static int
+intel_commit_primary_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y, uint32_t src_w,
+ uint32_t src_h, bool visible)
+{
+ struct drm_device *dev = crtc-dev;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(plane-fb);
+ struct drm_rect src = {
+ /* 16.16 fixed point */
+ .x1 = src_x,
+ .y1 = src_y,
+ .x2 = src_x + src_w,
+ .y2 = src_y + src_h,
+ };
+ int ret;
/*
* If the CRTC isn't enabled, we're just pinning the framebuffer,
@@ -11710,6 +11723,28 @@ intel_primary_plane_setplane(struct drm_plane *plane,
struct drm_crtc *crtc,
return 0;
}
+static int
+intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
+struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+unsigned int crtc_w, unsigned int crtc_h,
+uint32_t src_x, uint32_t src_y,
+uint32_t src_w, uint32_t src_h)
+{
+ bool visible;
+ int ret;
+
+ ret = intel_check_primary_plane(plane, crtc, fb, crtc_x, crtc_y,
+ crtc_w, crtc_h, src_x, src_y,
+ src_w, src_h, visible);
+ if (ret)
+ return ret;
+
+ intel_commit_primary_plane(plane, crtc, fb, crtc_x, crtc_y, crtc_w,
+ crtc_h, src_x, src_y, src_w, src_h, visible);
+
+ return 0;
+}
+
/* Common destruction function for both primary and cursor planes */
static void intel_plane_destroy(struct drm_plane *plane)
{
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/9] drm/i915: split intel_cursor_plane_update() into check() and commit()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Due to the upcoming atomic modesetting feature we need to separate
some update functions into a check step that can fail and a commit
step that should, ideally, never fail.
The commit part can still fail, but that should be solved in another
upcoming patch.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 54 ++--
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a8a8abe..0173c53 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11742,15 +11742,13 @@ intel_cursor_plane_disable(struct drm_plane *plane)
}
static int
-intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
+intel_check_cursor_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h)
+ uint32_t src_x, uint32_t src_y, uint32_t src_w,
+ uint32_t src_h, bool *visible)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
- struct drm_i915_gem_object *obj = intel_fb-obj;
struct drm_rect dest = {
/* integer pixels */
.x1 = crtc_x,
@@ -11770,16 +11768,23 @@ intel_cursor_plane_update(struct drm_plane *plane,
struct drm_crtc *crtc,
.x2 = intel_crtc-active ? intel_crtc-config.pipe_src_w : 0,
.y2 = intel_crtc-active ? intel_crtc-config.pipe_src_h : 0,
};
- bool visible;
- int ret;
- ret = drm_plane_helper_check_update(plane, crtc, fb,
- src, dest, clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true, visible);
- if (ret)
- return ret;
+ return drm_plane_helper_check_update(plane, crtc, fb,
+src, dest, clip,
+DRM_PLANE_HELPER_NO_SCALING,
+DRM_PLANE_HELPER_NO_SCALING,
+true, true, visible);
+}
+
+static int
+intel_commit_cursor_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ bool visible)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
crtc-cursor_x = crtc_x;
crtc-cursor_y = crtc_y;
@@ -11794,6 +11799,27 @@ intel_cursor_plane_update(struct drm_plane *plane,
struct drm_crtc *crtc,
return 0;
}
}
+
+static int
+intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ bool visible;
+ int ret;
+
+ ret = intel_check_cursor_plane(plane, crtc, fb, crtc_x, crtc_y, crtc_w,
+ crtc_h, src_x, src_y, src_w, src_h,
+ visible);
+ if (ret)
+ return ret;
+
+ return intel_commit_cursor_plane(plane, crtc, fb, crtc_x, crtc_y,
+crtc_w, crtc_h, visible);
+}
+
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
.update_plane = intel_cursor_plane_update,
.disable_plane = intel_cursor_plane_disable,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915: trivial: remove unneed set to NULL
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
At this point of the code the obj var is already NULL, so we don't
need to set it again to NULL.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b2e4eac..05937fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8253,7 +8253,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
if (!obj) {
DRM_DEBUG_KMS(cursor off\n);
addr = 0;
- obj = NULL;
mutex_lock(dev-struct_mutex);
goto finish;
}
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: Regression: +4W in idle power use on Macbook Pro 15 (late 2013)
Hi, On Tuesday 26 August 2014 16:00:51, Eric Rannaud wrote： On Tue, Aug 26, 2014 at 1:59 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote: Forcing FBC with i915.enable_fbc=1 brings the idle power consumption back to under 7W, however. This is all on 3.15.4-ARCH-00041-gf4db98240ac2. Any significant changes in package C state as reported in powertop? Indeed fairly impressive how much fbc saves here ... Not that I can tell. Powertop report with FBC: http://pastebin.com/5qfJKpTQ Without FBC: http://pastebin.com/NaYkR4n0 Some highly uneducated guesses on what could explain a +4W jump with no FBC: #1- The higher DRAM and bus duty cycle during scanout is enough to prevent some DRAM subsystems from sleeping, by crossing some tight threshold (maybe Apple has FBC enabled, so parameters somewhere in firmware are tuned for the lower level of background activity they expect with FBC on?). Not much we can do about that, unless such parameters can be tweaked by us. #2- Without FBC, the FB doesn't fit in L3 (i7-4750HQ has 6MB, 2880x1800 compressed at least 1:4 fits), keeping the DRAM awake more. #3- Disabling FBC somehow affects the layout of the framebuffer in DRAM, keeping more of the DRAM active and awake during scanout. Different tiling, swizzling, etc. parameters? Is it worth looking at the code for that kind of thing? To be clear, I'm (blindly) suggesting that it might be possible to increase the locality of the framebuffer in physical DRAM, even without compression enabled. I notices similar behavior. From the output of turborstat, with FBC MBP can reach PC6 more than 90% of the time when idling, after FBC was disable in 3.14 or 3.15 the chip stays in PC2 and the estimated GPU power consumption is a few watts higher. -- Best Regards, LR ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
