Re: [PATCH] drm/i915: update FBC maximum fb sizes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jun 04, 2013 at 02:46:14PM -0300, Paulo Zanoni wrote:
> 2013/6/4 Daniel Vetter <daniel@ffwll.ch>:
> > On Mon, Jun 3, 2013 at 11:15 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but
> >> without proper front buffer invalidation on the last 2k lines, so
> >> don't enable FBC on these cases for now.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
> >>  1 file changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 49a1887..cf123c1 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
> >>   *   - no pixel mulitply/line duplication
> >>   *   - no alpha buffer discard
> >>   *   - no dual wide
> >> - *   - framebuffer <= 2048 in width, 1536 in height
> >> + *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
> >>   *
> >>   * We can't assume that any compression will take place (worst case),
> >>   * so the compressed buffer has to be the same size as the uncompressed
> >> @@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
> >>         struct intel_framebuffer *intel_fb;
> >>         struct drm_i915_gem_object *obj;
> >>         int enable_fbc;
> >> +       unsigned int max_hdisplay, max_vdisplay;
> >>
> >>         if (!i915_powersave)
> >>                 return;
> >> @@ -507,8 +508,16 @@ void intel_update_fbc(struct drm_device *dev)
> >>                 dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
> >>                 goto out_disable;
> >>         }
> >> -       if ((crtc->mode.hdisplay > 2048) ||
> >> -           (crtc->mode.vdisplay > 1536)) {
> >> +
> >> +       if (IS_G4X(dev) || INTEL_INFO(dev)->gen > 4) {
> >
> > Bikeshed-time: I prefer gen checks to be of the form gen >= x or gen <
> > y because:
> > - It looks more like C code should look like.
> > - We we talk about platform ranges we tend to use e.g. ilk+ and
> > pre-gen6, so would fit more naturally with the written word.
> >
> > Patch looks good otherwise, but for a bit I was thinking "wait, g4x is
> > gen4, what's going on here?" ...
> 
> Yeah, you're right. I was trying to find out "how do I check for
> Cantiga+?" and basically copied the line I found inside
> intel_crtc_compute_config. We could really have a map between these
> older codenames and our checks somewhere.

We should maybe also have some feature macro for ctg+ display. Much
of the north display changed already for ctg rather than ilk.

-- 
Ville Syrjälä
Intel OTC