From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH 3/4] drm/i915: Try harder to get FBC
Date: Fri, 20 Jun 2014 09:55:34 -0700
Message-ID: <20140620165533.GC23411@bwidawsk.net>
References: <1403204773-7112-1-git-send-email-benjamin.widawsky@intel.com>
 <1403204773-7112-3-git-send-email-benjamin.widawsky@intel.com>
 <C7E999358BBE9E45938BA940F5F511087490D710@fmsmsx116.amr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112])
 by gabe.freedesktop.org (Postfix) with ESMTP id A26E46E207
 for <intel-gfx@lists.freedesktop.org>; Fri, 20 Jun 2014 09:55:40 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <C7E999358BBE9E45938BA940F5F511087490D710@fmsmsx116.amr.corp.intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
To: "Runyan, Arthur J" <arthur.j.runyan@intel.com>, Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>, "Widawsky,
 Benjamin" <benjamin.widawsky@intel.com>
List-Id: intel-gfx@lists.freedesktop.org

Rodrigo, when you're ready, can you pull in Art's requests?

On Fri, Jun 20, 2014 at 03:56:08PM +0000, Runyan, Arthur J wrote:
> You give me too much credit.  I just gave you an explanation of what the hardware does, then you ran with it.
> 
> On Thu, Jun 19, 2014 at 12:06:13PM -0700, Ben Widawsky wrote:
> +		DRM_INFO("Reducing the compressed framebuffer size. This may lead to increased power. Try to increase stolen memory size if available in BIOS.\n");
> 
> I prefer "This may lead to less power savings than a non-reduced size." since FBC is still going to save power.

Got it.

> 
>  	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane);
>  	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> +		dev_priv->fbc.threshold++;
> +
> +	switch (dev_priv->fbc.threshold) {
> +	case 4:
> +		dpfc_ctl |= DPFC_CTL_LIMIT_4X;
> +		break;
> +	case 2:
>  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> -	else
> +		break;
> +	case 1:
>  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> +		break;
> +	}
> 
> I Am Not A Coder, but at a glance it looks like the ++ could lead to undefined case 3 when you want case 4.
> 
Thanks for your feedback. 3 is an invalid value, but a default case
should be added here. I'd do a BUG_ON, but that's strictly forbidden.

-- 
Ben Widawsky, Intel Open Source Technology Center