| Message ID | 1442254803-20043-9-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 14, 2015 at 03:20:03PM -0300, Paulo Zanoni wrote: > I only tested this on BDW and SKL, but since the register description > is the same ever since gen4, let's assume that all gens take the same > register format. If that's not true, then hopefully someone will > bisect a bug to this patch and we'll fix it. > > Notice that the wrong fence offset register just means that the > hardware tracking will be wrong. > > Testcases: > - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt > - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt > > v2: > - Add intel_crtc->adjusted_{x,y} so this code can work independently > of intel_gen4_compute_page_offset(). (Ville). > - This version also works on SKL. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 9 +++++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_fbc.c | 25 ++++++++++++++++++++----- > 3 files changed, 31 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index fc00867..c27a636 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > (intel_crtc->config->pipe_src_w - 1) * pixel_size; > } > > + intel_crtc->adjusted_x = x; > + intel_crtc->adjusted_y = y; > + > I915_WRITE(reg, dspcntr); > > I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); > @@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, > } > } > > + intel_crtc->adjusted_x = x; > + intel_crtc->adjusted_y = y; > + > I915_WRITE(reg, dspcntr); > > I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); > @@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > } > plane_offset = y_offset << 16 | x_offset; > > + intel_crtc->adjusted_x = x_offset; > + intel_crtc->adjusted_y = y_offset; > + > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset); > I915_WRITE(PLANE_SIZE(pipe, 0), plane_size); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 02a755a..6054bcb 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -537,6 +537,8 @@ struct intel_crtc { > * gen4+ this only adjusts up to a tile, offsets within a tile are > * handled in the hw itself (with the TILEOFF register). */ > unsigned long dspaddr_offset; > + int adjusted_x; > + int adjusted_y; > > struct drm_i915_gem_object *cursor_bo; > uint32_t cursor_addr; > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index a3a97f2..2319dc5 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -41,6 +41,19 @@ > #include "intel_drv.h" > #include "i915_drv.h" > > +/* > + * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the > + * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's > + * origin so the x and y offsets can actually fit the registers. As a > + * consequence, the fence doesn't really start exactly at the display plane > + * address we program because it starts at the real start of the buffer, so we > + * have to take this into consideration here. > + */ > +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc) > +{ > + return crtc->base.y - crtc->adjusted_y; > +} We may want to move over to using the plane state here in the future. But for now it looks OK, so: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > static void i8xx_fbc_disable(struct drm_i915_private *dev_priv) > { > u32 fbc_ctl; > @@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc) > fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE; > fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane); > I915_WRITE(FBC_CONTROL2, fbc_ctl2); > - I915_WRITE(FBC_FENCE_OFF, crtc->base.y); > + I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc)); > } > > /* enable it... */ > @@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc) > dpfc_ctl |= DPFC_CTL_LIMIT_1X; > dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg; > > - I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y); > + I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc)); > > /* enable it... */ > I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); > @@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > u32 dpfc_ctl; > int threshold = dev_priv->fbc.threshold; > + unsigned int y_offset; > > dev_priv->fbc.enabled = true; > > @@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) > if (IS_GEN5(dev_priv)) > dpfc_ctl |= obj->fence_reg; > > - I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y); > + y_offset = get_crtc_fence_y_offset(crtc); > + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); > I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); > /* enable it... */ > I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); > @@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) > if (IS_GEN6(dev_priv)) { > I915_WRITE(SNB_DPFC_CTL_SA, > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > - I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y); > + I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset); > } > > intel_fbc_nuke(dev_priv); > @@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc) > > I915_WRITE(SNB_DPFC_CTL_SA, > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > - I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y); > + I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc)); > > intel_fbc_nuke(dev_priv); > > -- > 2.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Pulled in the entire series to dinq, thanks. -Daniel On Mon, Sep 21, 2015 at 05:04:49PM +0300, Ville Syrjälä wrote: > On Mon, Sep 14, 2015 at 03:20:03PM -0300, Paulo Zanoni wrote: > > I only tested this on BDW and SKL, but since the register description > > is the same ever since gen4, let's assume that all gens take the same > > register format. If that's not true, then hopefully someone will > > bisect a bug to this patch and we'll fix it. > > > > Notice that the wrong fence offset register just means that the > > hardware tracking will be wrong. > > > > Testcases: > > - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt > > - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt > > > > v2: > > - Add intel_crtc->adjusted_{x,y} so this code can work independently > > of intel_gen4_compute_page_offset(). (Ville). > > - This version also works on SKL. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 9 +++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_fbc.c | 25 ++++++++++++++++++++----- > > 3 files changed, 31 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index fc00867..c27a636 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > > (intel_crtc->config->pipe_src_w - 1) * pixel_size; > > } > > > > + intel_crtc->adjusted_x = x; > > + intel_crtc->adjusted_y = y; > > + > > I915_WRITE(reg, dspcntr); > > > > I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); > > @@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, > > } > > } > > > > + intel_crtc->adjusted_x = x; > > + intel_crtc->adjusted_y = y; > > + > > I915_WRITE(reg, dspcntr); > > > > I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); > > @@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > > } > > plane_offset = y_offset << 16 | x_offset; > > > > + intel_crtc->adjusted_x = x_offset; > > + intel_crtc->adjusted_y = y_offset; > > + > > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > > I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset); > > I915_WRITE(PLANE_SIZE(pipe, 0), plane_size); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 02a755a..6054bcb 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -537,6 +537,8 @@ struct intel_crtc { > > * gen4+ this only adjusts up to a tile, offsets within a tile are > > * handled in the hw itself (with the TILEOFF register). */ > > unsigned long dspaddr_offset; > > + int adjusted_x; > > + int adjusted_y; > > > > struct drm_i915_gem_object *cursor_bo; > > uint32_t cursor_addr; > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > > index a3a97f2..2319dc5 100644 > > --- a/drivers/gpu/drm/i915/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/intel_fbc.c > > @@ -41,6 +41,19 @@ > > #include "intel_drv.h" > > #include "i915_drv.h" > > > > +/* > > + * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the > > + * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's > > + * origin so the x and y offsets can actually fit the registers. As a > > + * consequence, the fence doesn't really start exactly at the display plane > > + * address we program because it starts at the real start of the buffer, so we > > + * have to take this into consideration here. > > + */ > > +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc) > > +{ > > + return crtc->base.y - crtc->adjusted_y; > > +} > > We may want to move over to using the plane state here in the future. > But for now it looks OK, so: > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > + > > static void i8xx_fbc_disable(struct drm_i915_private *dev_priv) > > { > > u32 fbc_ctl; > > @@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc) > > fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE; > > fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane); > > I915_WRITE(FBC_CONTROL2, fbc_ctl2); > > - I915_WRITE(FBC_FENCE_OFF, crtc->base.y); > > + I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc)); > > } > > > > /* enable it... */ > > @@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc) > > dpfc_ctl |= DPFC_CTL_LIMIT_1X; > > dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg; > > > > - I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y); > > + I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc)); > > > > /* enable it... */ > > I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); > > @@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) > > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > > u32 dpfc_ctl; > > int threshold = dev_priv->fbc.threshold; > > + unsigned int y_offset; > > > > dev_priv->fbc.enabled = true; > > > > @@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) > > if (IS_GEN5(dev_priv)) > > dpfc_ctl |= obj->fence_reg; > > > > - I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y); > > + y_offset = get_crtc_fence_y_offset(crtc); > > + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); > > I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); > > /* enable it... */ > > I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); > > @@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) > > if (IS_GEN6(dev_priv)) { > > I915_WRITE(SNB_DPFC_CTL_SA, > > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > > - I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y); > > + I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset); > > } > > > > intel_fbc_nuke(dev_priv); > > @@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc) > > > > I915_WRITE(SNB_DPFC_CTL_SA, > > SNB_CPU_FENCE_ENABLE | obj->fence_reg); > > - I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y); > > + I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc)); > > > > intel_fbc_nuke(dev_priv); > > > > -- > > 2.5.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > 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_display.c b/drivers/gpu/drm/i915/intel_display.c index fc00867..c27a636 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, (intel_crtc->config->pipe_src_w - 1) * pixel_size; } + intel_crtc->adjusted_x = x; + intel_crtc->adjusted_y = y; + I915_WRITE(reg, dspcntr); I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); @@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, } } + intel_crtc->adjusted_x = x; + intel_crtc->adjusted_y = y; + I915_WRITE(reg, dspcntr); I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); @@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, } plane_offset = y_offset << 16 | x_offset; + intel_crtc->adjusted_x = x_offset; + intel_crtc->adjusted_y = y_offset; + I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset); I915_WRITE(PLANE_SIZE(pipe, 0), plane_size); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 02a755a..6054bcb 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -537,6 +537,8 @@ struct intel_crtc { * gen4+ this only adjusts up to a tile, offsets within a tile are * handled in the hw itself (with the TILEOFF register). */ unsigned long dspaddr_offset; + int adjusted_x; + int adjusted_y; struct drm_i915_gem_object *cursor_bo; uint32_t cursor_addr; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index a3a97f2..2319dc5 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -41,6 +41,19 @@ #include "intel_drv.h" #include "i915_drv.h" +/* + * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the + * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's + * origin so the x and y offsets can actually fit the registers. As a + * consequence, the fence doesn't really start exactly at the display plane + * address we program because it starts at the real start of the buffer, so we + * have to take this into consideration here. + */ +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc) +{ + return crtc->base.y - crtc->adjusted_y; +} + static void i8xx_fbc_disable(struct drm_i915_private *dev_priv) { u32 fbc_ctl; @@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc) fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE; fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane); I915_WRITE(FBC_CONTROL2, fbc_ctl2); - I915_WRITE(FBC_FENCE_OFF, crtc->base.y); + I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc)); } /* enable it... */ @@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= DPFC_CTL_LIMIT_1X; dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg; - I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y); + I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc)); /* enable it... */ I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); @@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) struct drm_i915_gem_object *obj = intel_fb_obj(fb); u32 dpfc_ctl; int threshold = dev_priv->fbc.threshold; + unsigned int y_offset; dev_priv->fbc.enabled = true; @@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) if (IS_GEN5(dev_priv)) dpfc_ctl |= obj->fence_reg; - I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y); + y_offset = get_crtc_fence_y_offset(crtc); + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); @@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) if (IS_GEN6(dev_priv)) { I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj->fence_reg); - I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y); + I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset); } intel_fbc_nuke(dev_priv); @@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc) I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj->fence_reg); - I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y); + I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc)); intel_fbc_nuke(dev_priv);
I only tested this on BDW and SKL, but since the register description is the same ever since gen4, let's assume that all gens take the same register format. If that's not true, then hopefully someone will bisect a bug to this patch and we'll fix it. Notice that the wrong fence offset register just means that the hardware tracking will be wrong. Testcases: - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt v2: - Add intel_crtc->adjusted_{x,y} so this code can work independently of intel_gen4_compute_page_offset(). (Ville). - This version also works on SKL. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 9 +++++++++ drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_fbc.c | 25 ++++++++++++++++++++----- 3 files changed, 31 insertions(+), 5 deletions(-)