All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/fbc: fix the check for already reserved fbc size
Date: Fri, 06 Feb 2015 10:09:00 +0200	[thread overview]
Message-ID: <87zj8rwjfn.fsf@intel.com> (raw)
In-Reply-To: <20150205170816.GC27834@intel.com>

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

      reply	other threads:[~2015-02-06  8:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zj8rwjfn.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=benjamin.widawsky@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.