| Message ID | 1443740157-3803-1-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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@linux.intel.com> > + > +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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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; +} + +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; }
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@intel.com> --- drivers/gpu/drm/i915/intel_fbc.c | 54 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 5 deletions(-)