On Thu, Oct 01, 2015 at 07:55:57PM -0300, Paulo Zanoni wrote: > We were considering the whole framebuffer height, but the spec says we > should only consider the active display height size. There were still > some unclear questions based on the spec, but the hardware guys > clarified them for us. According to them: > > - CFB size = CFB stride * Number of lines FBC writes to CFB > - CFB stride = plane stride / compression limit > - Number of lines FBC writes to CFB = MIN(plane source height, maximum > number of lines FBC writes to CFB) > - Plane source height = > - pipe source height (PIPE_SRCSZ register) (before SKL) > - plane size register height (PLANE_SIZE register) (SKL+) > - Maximum number of lines FBC writes to CFB = > - plane source height (before HSW) > - 2048 (HSW+) > > For the plane source height, I could just have made our code do > I915_READ() in order to be more future proof, but since it's not cool > to do register reads I decided to just recalculate the values we use > when we actually write to those registers. > > With this patch, depending on your machine configuration, a lot of the > kms_frontbuffer_tracking subtests that used to result in a SKIP due to > not enough stolen memory still start resulting in a PASS. > > v2: Use the clipped src size instead of pipe_src_h (Ville). > v3: Use the appropriate information provided by the hardware guys. > v4: Bikesheds: s/sizes/size/, s/fb_cpp/cpp/ (Ville). > v5: - Don't use crtc->config->pipe_src_x for BDW- (Ville). > - Fix the register name written in the comment. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_fbc.c | 54 ++++++++++++++++++++++++++++++++++++---- > 1 file changed, 49 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index 1b2ebb2..18e228b 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -698,16 +698,61 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv) > mutex_unlock(&dev_priv->fbc.lock); > } > > -static int intel_fbc_setup_cfb(struct drm_i915_private *dev_priv, int size, > - int fb_cpp) > +/* > + * For SKL+, the plane source size used by the hardware is based on the value we > + * write to the PLANE_SIZE register. For BDW-, the hardware looks at the value > + * we wrote to PIPESRC. > + */ > +static void intel_fbc_get_plane_source_size(struct intel_crtc *crtc, > + int *width, int *height) > { > + struct intel_plane_state *plane_state = > + to_intel_plane_state(crtc->base.primary->state); > + int w, h; > + > + if (intel_rotation_90_or_270(plane_state->base.rotation)) { > + w = drm_rect_height(&plane_state->src) >> 16; > + h = drm_rect_width(&plane_state->src) >> 16; > + } else { > + w = drm_rect_width(&plane_state->src) >> 16; > + h = drm_rect_height(&plane_state->src) >> 16; > + } > + > + if (width) > + *width = w; > + if (height) > + *height = h; > +} Yep, I like this much better. Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > + > +static int intel_fbc_calculate_cfb_size(struct intel_crtc *crtc) > +{ > + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; > + struct drm_framebuffer *fb = crtc->base.primary->fb; > + int lines; > + > + intel_fbc_get_plane_source_size(crtc, NULL, &lines); > + if (INTEL_INFO(dev_priv)->gen >= 7) > + lines = min(lines, 2048); > + > + return lines * fb->pitches[0]; > +} > + > +static int intel_fbc_setup_cfb(struct intel_crtc *crtc) > +{ > + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; > + struct drm_framebuffer *fb = crtc->base.primary->fb; > + int size, cpp; > + > + size = intel_fbc_calculate_cfb_size(crtc); > + cpp = drm_format_plane_cpp(fb->pixel_format, 0); > + > if (size <= dev_priv->fbc.uncompressed_size) > return 0; > > /* Release any current block */ > __intel_fbc_cleanup_cfb(dev_priv); > > - return intel_fbc_alloc_cfb(dev_priv, size, fb_cpp); > + return intel_fbc_alloc_cfb(dev_priv, size, cpp); > } > > static bool stride_is_valid(struct drm_i915_private *dev_priv, > @@ -897,8 +942,7 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > goto out_disable; > } > > - if (intel_fbc_setup_cfb(dev_priv, obj->base.size, > - drm_format_plane_cpp(fb->pixel_format, 0))) { > + if (intel_fbc_setup_cfb(intel_crtc)) { > set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL); > goto out_disable; > } > -- > 2.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx