Hi Ville,
On Wednesday 15 March 2017 03:53 PM, Ville Syrjälä wrote:
On Wed, Mar 15, 2017 at 12:29:35PM +0530, Praveen Paneri wrote:
FBC is only enabled for X-tiled framebuffers but there are
quite a few cases where we tend to use Y-tiled framebuffers.
So enabling it for non X-tiled framebuffers.
This patch doesn't actuall enable anything. In fact to me it looks like
we're already allowing Y tiling + FBC, so the commit message/subject should
likely say that you're fixing something.
I agree! I can rename the patch as
Fix FBC Stride programming for Y-tiled FBs
Unless I'm missing some magic check somewhere that still prevents
Y tiled + FBC. But then this patch wouldn't be sufficient either.
We had a check to activate FBC only for X-tiled buffers but that has
been removed by this patch.
https://patchwork.kernel.org/patch/9546759/
With this change, we don't know what would happen in case of Y-tiled
buffer as it is suggested that Stride for Y-tiled buffers must be
programmed by SW (Probably a HW bug/limitation).
Signed-off-by: Praveen Paneri <praveen.paneri@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_fbc.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc843f9..9d7a376 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2506,6 +2506,8 @@ enum skl_disp_power_wells {
#define BDW_FBC_COMPRESSION_MASK 0xfff
#define FBC_LL_SIZE (1536)
+#define FBC_YSTRIDE _MMIO(0x4208c)
+#define FBC_STRIDE_OVERRIDE (1<<13)
These defines shouldn't be in the middle of the FBC1 register defines.
The register should also be called CHICKEN_MISC_1, so the defines
should live next to the other CHICKEN_MISC register defines.
Will fix this
#define FBC_LLC_READ_CTRL _MMIO(0x9044)
#define FBC_LLC_FULLY_OPEN (1<<30)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 17d418b..0ac9889 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -301,6 +301,14 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
u32 dpfc_ctl;
int threshold = dev_priv->fbc.threshold;
+ if (INTEL_GEN(dev_priv) >= 9 &&
AFAICS IS_GEN9() would be better.
Yes!
+ i915_gem_object_get_tiling(cache->vma->obj) != I915_TILING_X) {
+ struct intel_fbc_state_cache *cache = &dev_priv->fbc.state_cache;
+ int cfb_stride = DIV_ROUND_UP(cache->plane.src_w,
+ (32 * threshold)) * 8;
Are we taking this compressed stride alignment requirement into account
when we allocate the cfb?
cfb size is calculated as (line * cache->fb.stride) but cfb is allocated
as per the original FB size (threshold = 1) where stride would be higher
than what is calculated here. Would we still need to take care of the
alignment?
Thanks,
Praveen
+ I915_WRITE(FBC_YSTRIDE, FBC_STRIDE_OVERRIDE | cfb_stride);
+ }
+
dpfc_ctl = 0;
if (IS_IVYBRIDGE(dev_priv))
dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
