* [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
@ 2015-02-05 10:04 Jani Nikula
2015-02-05 10:06 ` Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2015-02-05 10:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Ben Widawsky
The check for previously reserved stolen space size for FBC in
i915_gem_stolen_setup_compression() did not take the compression
threshold into account. Fix this by storing and comparing to
uncompressed size instead.
The bug has been introduced in
commit 5e59f7175f96550ede91f58d267d2b551cb6fbba
Author: Ben Widawsky <benjamin.widawsky@intel.com>
Date: Mon Jun 30 10:41:24 2014 -0700
drm/i915: Try harder to get FBC
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88975
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: stable@vger.kernel.org # 3.17+
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_stolen.c | 8 ++++----
drivers/gpu/drm/i915/intel_fbc.c | 3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca64b999403f..97ce50a176a7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -772,7 +772,7 @@ struct intel_context {
};
struct i915_fbc {
- unsigned long size;
+ unsigned long uncompressed_size;
unsigned threshold;
unsigned int fb_id;
enum plane plane;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index a2045848bd1a..59401f3b902c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -231,7 +231,7 @@ static int i915_setup_compression(struct drm_device *dev, int size, int fb_cpp)
dev_priv->mm.stolen_base + compressed_llb->start);
}
- dev_priv->fbc.size = size / dev_priv->fbc.threshold;
+ dev_priv->fbc.uncompressed_size = size;
DRM_DEBUG_KMS("reserved %d bytes of contiguous stolen space for FBC\n",
size);
@@ -253,7 +253,7 @@ int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_c
if (!drm_mm_initialized(&dev_priv->mm.stolen))
return -ENODEV;
- if (size < dev_priv->fbc.size)
+ if (size < dev_priv->fbc.uncompressed_size)
return 0;
/* Release any current block */
@@ -266,7 +266,7 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- if (dev_priv->fbc.size == 0)
+ if (dev_priv->fbc.uncompressed_size == 0)
return;
drm_mm_remove_node(&dev_priv->fbc.compressed_fb);
@@ -276,7 +276,7 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
kfree(dev_priv->fbc.compressed_llb);
}
- dev_priv->fbc.size = 0;
+ dev_priv->fbc.uncompressed_size = 0;
}
void i915_gem_cleanup_stolen(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 624d1d92d284..b572bb6ebcff 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -78,7 +78,8 @@ static void i8xx_fbc_enable(struct drm_crtc *crtc)
dev_priv->fbc.enabled = true;
- cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
+ /* Note: fbc.threshold == 1 for i8xx */
+ cfb_pitch = dev_priv->fbc.uncompressed_size / FBC_LL_SIZE;
if (fb->pitches[0] < cfb_pitch)
cfb_pitch = fb->pitches[0];
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
2015-02-05 10:04 [PATCH] drm/i915/fbc: fix the check for already reserved fbc size Jani Nikula
@ 2015-02-05 10:06 ` Chris Wilson
2015-02-05 14:27 ` Daniel Vetter
2015-02-05 13:27 ` shuang.he
2015-02-05 17:08 ` Ben Widawsky
2 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2015-02-05 10:06 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Ben Widawsky
On Thu, Feb 05, 2015 at 12:04:27PM +0200, Jani Nikula wrote:
> The check for previously reserved stolen space size for FBC in
> i915_gem_stolen_setup_compression() did not take the compression
> threshold into account. Fix this by storing and comparing to
> uncompressed size instead.
>
> The bug has been introduced in
>
> commit 5e59f7175f96550ede91f58d267d2b551cb6fbba
> Author: Ben Widawsky <benjamin.widawsky@intel.com>
> Date: Mon Jun 30 10:41:24 2014 -0700
>
> drm/i915: Try harder to get FBC
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88975
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: stable@vger.kernel.org # 3.17+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Thanks, I think uncompressed_size clarifies exactly what we want to
compare against (is this allocation request any bigger or smaller than
the last request). Then if we need the final size for anything we can
add it without further confusion.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
2015-02-05 10:04 [PATCH] drm/i915/fbc: fix the check for already reserved fbc size Jani Nikula
2015-02-05 10:06 ` Chris Wilson
@ 2015-02-05 13:27 ` shuang.he
2015-02-05 17:08 ` Ben Widawsky
2 siblings, 0 replies; 6+ messages in thread
From: shuang.he @ 2015-02-05 13:27 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, jani.nikula
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5714
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 282/283 282/283
ILK -1 316/319 315/319
SNB +22-1 322/346 343/346
IVB -1 382/384 381/384
BYT 296/296 296/296
HSW -1 425/428 424/428
BDW 318/333 318/333
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt_kms_flip_vblank-vs-hang PASS(3, M26M37) TIMEOUT(1, M26)
SNB igt_kms_cursor_crc_cursor-size-change NSPT(1, M22)PASS(1, M35)DMESG_FAIL(1, M35) PASS(1, M35)
SNB igt_kms_flip_dpms-vs-vblank-race-interruptible DMESG_WARN(1, M35)PASS(2, M22M35) DMESG_WARN(1, M35)
SNB igt_kms_flip_event_leak NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_kms_rotation_crc_primary-rotation NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_kms_rotation_crc_sprite-rotation NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_cursor NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_cursor-dpms NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_dpms-non-lpsp NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_drm-resources-equal NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_fences NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_fences-dpms NSPT(1, M22)DMESG_WARN(1, M35)PASS(1, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-execbuf NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-mmap-cpu NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-mmap-gtt NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-pread NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_i2c NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_modeset-non-lpsp NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_pci-d3-state NSPT(1, M22)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_rte NSPT(1, M22)PASS(2, M35) PASS(1, M35)
IVB igt_gem_pwrite_pread_snooped-copy-performance DMESG_WARN(1, M34)PASS(1, M21) DMESG_WARN(1, M34)
*HSW igt_gem_pwrite_pread_snooped-copy-performance PASS(2, M20M40) DMESG_WARN(1, M40)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
2015-02-05 10:06 ` Chris Wilson
@ 2015-02-05 14:27 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-02-05 14:27 UTC (permalink / raw)
To: Chris Wilson, Jani Nikula, intel-gfx, Ben Widawsky
On Thu, Feb 05, 2015 at 10:06:05AM +0000, Chris Wilson wrote:
> On Thu, Feb 05, 2015 at 12:04:27PM +0200, Jani Nikula wrote:
> > The check for previously reserved stolen space size for FBC in
> > i915_gem_stolen_setup_compression() did not take the compression
> > threshold into account. Fix this by storing and comparing to
> > uncompressed size instead.
> >
> > The bug has been introduced in
> >
> > commit 5e59f7175f96550ede91f58d267d2b551cb6fbba
> > Author: Ben Widawsky <benjamin.widawsky@intel.com>
> > Date: Mon Jun 30 10:41:24 2014 -0700
> >
> > drm/i915: Try harder to get FBC
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88975
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > Cc: stable@vger.kernel.org # 3.17+
Dropped since fbc isn't enabled by default.
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Thanks, I think uncompressed_size clarifies exactly what we want to
> compare against (is this allocation request any bigger or smaller than
> the last request). Then if we need the final size for anything we can
> add it without further confusion.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
2015-02-05 10:04 [PATCH] drm/i915/fbc: fix the check for already reserved fbc size Jani Nikula
2015-02-05 10:06 ` Chris Wilson
2015-02-05 13:27 ` shuang.he
@ 2015-02-05 17:08 ` Ben Widawsky
2015-02-06 8:09 ` Jani Nikula
2 siblings, 1 reply; 6+ messages in thread
From: Ben Widawsky @ 2015-02-05 17:08 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Thu, Feb 05, 2015 at 12:04:27PM +0200, Jani Nikula wrote:
> The check for previously reserved stolen space size for FBC in
> i915_gem_stolen_setup_compression() did not take the compression
> threshold into account. Fix this by storing and comparing to
> uncompressed size instead.
>
> The bug has been introduced in
>
> commit 5e59f7175f96550ede91f58d267d2b551cb6fbba
> Author: Ben Widawsky <benjamin.widawsky@intel.com>
> Date: Mon Jun 30 10:41:24 2014 -0700
>
> drm/i915: Try harder to get FBC
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88975
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: stable@vger.kernel.org # 3.17+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
I guess we can argue semantically about whether or not it was a bug. If I didn't
put the DRM_INFO there, everything would have continued to work and nobody would
have complained. I personally prefer keeping the size as the compressed size
like you did here: https://bugs.freedesktop.org/show_bug.cgi?id=88975#c1
but Chris already reviewed this so nuts to me.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
2015-02-05 17:08 ` Ben Widawsky
@ 2015-02-06 8:09 ` Jani Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2015-02-06 8:09 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, 05 Feb 2015, Ben Widawsky <benjamin.widawsky@intel.com> wrote:
> On Thu, Feb 05, 2015 at 12:04:27PM +0200, Jani Nikula wrote:
>> The check for previously reserved stolen space size for FBC in
>> i915_gem_stolen_setup_compression() did not take the compression
>> threshold into account. Fix this by storing and comparing to
>> uncompressed size instead.
>>
>> The bug has been introduced in
>>
>> commit 5e59f7175f96550ede91f58d267d2b551cb6fbba
>> Author: Ben Widawsky <benjamin.widawsky@intel.com>
>> Date: Mon Jun 30 10:41:24 2014 -0700
>>
>> drm/i915: Try harder to get FBC
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88975
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
>> Cc: stable@vger.kernel.org # 3.17+
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> I guess we can argue semantically about whether or not it was a bug. If I didn't
> put the DRM_INFO there, everything would have continued to work and nobody would
> have complained.
Okay, bug or not, I felt it was unnecessary to be doing the reservation
over and over at every update.
I also contemplated making the logging DRM_INFO_ONCE as a follow-up, but
then thought it's a good canary, at least for now. We occasionally have
these moments where we discover we do some stuff way too often.
> I personally prefer keeping the size as the compressed size
> like you did here: https://bugs.freedesktop.org/show_bug.cgi?id=88975#c1
> but Chris already reviewed this so nuts to me.
As you can tell, I did not really have an opinion one way or the other,
so someone else having an opinion won...
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-06 8:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-05 10:04 [PATCH] drm/i915/fbc: fix the check for already reserved fbc size Jani Nikula
2015-02-05 10:06 ` Chris Wilson
2015-02-05 14:27 ` Daniel Vetter
2015-02-05 13:27 ` shuang.he
2015-02-05 17:08 ` Ben Widawsky
2015-02-06 8:09 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox