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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to