diff mbox

[04/16] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB

Message ID 1439588061-18064-5-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Aug. 14, 2015, 9:34 p.m. UTC
The doc is pretty clear that this register should be set to 0 on SNB.
We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Aug. 28, 2015, 2:46 p.m. UTC | #1
On Fri, Aug 14, 2015 at 06:34:09PM -0300, Paulo Zanoni wrote:
> The doc is pretty clear that this register should be set to 0 on SNB.
> We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below.

Bspec says:
"Restriction : The CPU fence is always programmed to match the Display
 Buffer base, so this offset must be programmed to 0 to match. 	DevSNB"

We definitely don't program the fence to match DSPSURF, so it's not very
clear that this is really the right thing to do. I suppose it depends on
how the Y offset in the SA register interacts with this one. I never got
around to fixing the Y offset stuff in my FBC efforts, so I've not tried
it on real hardware and so I have no sure answer here.

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 9ffa7dc..f7be9ab8 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
>  		dpfc_ctl |= obj->fence_reg;
>  
>  	y_offset = get_crtc_fence_y_offset(crtc);
> -	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> +
> +	if (IS_GEN5(dev_priv))
> +		I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> +	else
> +		I915_WRITE(ILK_DPFC_FENCE_YOFF, 0);
> +
>  	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);
> -- 
> 2.4.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R Oct. 8, 2015, 9:26 p.m. UTC | #2
Em Sex, 2015-08-28 às 17:46 +0300, Ville Syrjälä escreveu:
> On Fri, Aug 14, 2015 at 06:34:09PM -0300, Paulo Zanoni wrote:

> > The doc is pretty clear that this register should be set to 0 on

> > SNB.

> > We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines

> > below.

> 

> Bspec says:

> "Restriction : The CPU fence is always programmed to match the

> Display

>  Buffer base, so this offset must be programmed to 0 to match. 	

> DevSNB"

> 

> We definitely don't program the fence to match DSPSURF, so it's not

> very

> clear that this is really the right thing to do. I suppose it depends

> on

> how the Y offset in the SA register interacts with this one. I never

> got

> around to fixing the Y offset stuff in my FBC efforts, so I've not

> tried

> it on real hardware and so I have no sure answer here.


The BSpec page for DPFC Control on SNB says:

8<---------------------
Project :DEVSNB
iMPH will only send the host modify message when modifications are in
the fence selected in the DPFC_CONTROL_SA register CPUFNCNUM field. The
fence field in the FBC Host Modification message will always be 0 and
this field must be programmed to 0 to match.
8<---------------------

(and DPFC_CONTROL_SA is register 0x100100)

> 

> > 

> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_fbc.c | 7 ++++++-

> >  1 file changed, 6 insertions(+), 1 deletion(-)

> > 

> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c

> > b/drivers/gpu/drm/i915/intel_fbc.c

> > index 9ffa7dc..f7be9ab8 100644

> > --- a/drivers/gpu/drm/i915/intel_fbc.c

> > +++ b/drivers/gpu/drm/i915/intel_fbc.c

> > @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc

> > *crtc)

> >  		dpfc_ctl |= obj->fence_reg;

> >  

> >  	y_offset = get_crtc_fence_y_offset(crtc);

> > -	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);

> > +

> > +	if (IS_GEN5(dev_priv))

> > +		I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);

> > +	else

> > +		I915_WRITE(ILK_DPFC_FENCE_YOFF, 0);

> > +

> >  	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);

> > -- 

> > 2.4.6

> > 

> > _______________________________________________

> > Intel-gfx mailing list

> > Intel-gfx@lists.freedesktop.org

> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

>
Ville Syrjälä Oct. 8, 2015, 9:37 p.m. UTC | #3
On Thu, Oct 08, 2015 at 09:26:19PM +0000, Zanoni, Paulo R wrote:
> Em Sex, 2015-08-28 às 17:46 +0300, Ville Syrjälä escreveu:
> > On Fri, Aug 14, 2015 at 06:34:09PM -0300, Paulo Zanoni wrote:
> > > The doc is pretty clear that this register should be set to 0 on
> > > SNB.
> > > We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines
> > > below.
> > 
> > Bspec says:
> > "Restriction : The CPU fence is always programmed to match the
> > Display
> >  Buffer base, so this offset must be programmed to 0 to match. 	
> > DevSNB"
> > 
> > We definitely don't program the fence to match DSPSURF, so it's not
> > very
> > clear that this is really the right thing to do. I suppose it depends
> > on
> > how the Y offset in the SA register interacts with this one. I never
> > got
> > around to fixing the Y offset stuff in my FBC efforts, so I've not
> > tried
> > it on real hardware and so I have no sure answer here.
> 
> The BSpec page for DPFC Control on SNB says:
> 
> 8<---------------------
> Project :DEVSNB
> iMPH will only send the host modify message when modifications are in
> the fence selected in the DPFC_CONTROL_SA register CPUFNCNUM field. The
> fence field in the FBC Host Modification message will always be 0 and
> this field must be programmed to 0 to match.
> 8<---------------------

That's about the fence number, it doesn't say anything about the offset
IIRC.

I guess it could be tested by, say:
- set the SA fence offset to some high number and leave this one low
- do the opposite
and in each case see if GTT tracking still works.

And to make sure were testin the right thing, probably repeat with
both offsets set high and make sure the tracking really does not work.

> 
> (and DPFC_CONTROL_SA is register 0x100100)
> 
> > 
> > > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbc.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > > b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 9ffa7dc..f7be9ab8 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc
> > > *crtc)
> > >  		dpfc_ctl |= obj->fence_reg;
> > >  
> > >  	y_offset = get_crtc_fence_y_offset(crtc);
> > > -	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> > > +
> > > +	if (IS_GEN5(dev_priv))
> > > +		I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> > > +	else
> > > +		I915_WRITE(ILK_DPFC_FENCE_YOFF, 0);
> > > +
> > >  	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);
> > > -- 
> > > 2.4.6
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 9ffa7dc..f7be9ab8 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -216,7 +216,12 @@  static void ilk_fbc_enable(struct intel_crtc *crtc)
 		dpfc_ctl |= obj->fence_reg;
 
 	y_offset = get_crtc_fence_y_offset(crtc);
-	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
+
+	if (IS_GEN5(dev_priv))
+		I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
+	else
+		I915_WRITE(ILK_DPFC_FENCE_YOFF, 0);
+
 	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);