xf86-video-intel: src/i810_reg.h src/i830_driver.c

[Jesse Barnes]

On Thursday, May 29, 2008 1:44 am Alan Hourihane wrote:
> On Wed, 2008-05-28 at 15:49 -0700, Jesse Barnes wrote:
> > src/i810_reg.h    |    2 ++
> >  src/i830_driver.c |   11 ++++++++++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > New commits:
> > commit bd137a19dc29dd466eac030e040f729ed0807e3f
> > Author: Jesse Barnes <jbarnes at hobbes.lan>
> > Date:   Wed May 28 15:47:47 2008 -0700
> >
> >     Fixup DSPARB for 855 & 945
> >
> >     It turns out 855 has a different DSPARB layout than 915+.  And 945+
> > have more FIFO entries, so we have to allocate things differently.  So on
> > 855 split the FIFO evenly again between A & B planes, and do the same on
> > 945, where we have a larger FIFO.  Fixes an issue reported by Daniel
> > Stone with the previous default value.
> >
> > diff --git a/src/i810_reg.h b/src/i810_reg.h
> > index d97780f..9da258a 100644
> > --- a/src/i810_reg.h
> > +++ b/src/i810_reg.h
> > @@ -2105,6 +2105,8 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > SOFTWARE. #define DSPARB			0x70030
> >  #define   DSPARB_CSTART_SHIFT	7
> >  #define   DSPARB_BSTART_SHIFT	0
> > +#define   DSPARB_BEND_SHIFT	9 /* on 855 */
> > +#define   DSPARB_AEND_SHIFT	0
> >  #define DSPFW1			0x70034
> >  #define DSPFW2			0x70038
> >  #define DSPFW3			0x7003c
> > diff --git a/src/i830_driver.c b/src/i830_driver.c
> > index 1b07064..1041fe3 100644
> > --- a/src/i830_driver.c
> > +++ b/src/i830_driver.c
> > @@ -1963,7 +1963,16 @@ SetHWOperatingState(ScrnInfoPtr pScrn)
> >      * we don't use plane C at all so we can allocate all but one of the
> > 96 * FIFO RAM entries equally between planes A and B.
> >      */
> > -   OUTREG(DSPARB, (95 << DSPARB_CSTART_SHIFT) | (48 <<
> > DSPARB_BSTART_SHIFT)); +   if (IS_I9XX(pI830)) {
> > +       if (IS_I915GM(pI830) || IS_I915G(pI830))
> > +	   OUTREG(DSPARB, (95 << DSPARB_CSTART_SHIFT) |
> > +		  (48 << DSPARB_BSTART_SHIFT));
> > +       else
> > +	   OUTREG(DSPARB, (127 << DSPARB_CSTART_SHIFT) |
> > +		  (64 << DSPARB_BSTART_SHIFT));
> > +   } else {
> > +       OUTREG(DSPARB, 254 << DSPARB_BEND_SHIFT | 128 <<
> > DSPARB_AEND_SHIFT); +   }
> >  }
>
> Jesse,
>
> >From what I understand of the DSPARB register, these values shouldn't be
>
> hardcoded values but based on the HDISPLAY value that's configured for
> each pipe to ensure correct FIFO splits.
>
> Just a heads up.

You're right, we should be doing a bandwidth calculation based on the current 
mode for each plane.  But I still don't have access to the spreadsheet that 
documents how the FIFO should be split, so until then we'll have to hope that 
stealing all of display plane C's FIFO entries is enough. :)  And at least 
now we'll know when underruns happen, since I added some debug code.

Thanks,
Jesse