[Nouveau] [PATCH 4/9] nvkm/fb/ramnv50: Ressurect timing code, use proper timing/rammap handlers

[Roy Spliet]

Hello Tobias,

Reply inline.

--- original message ---
From: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de>
Date: 01:00:17 23-05-2015
To: Roy Spliet <rspliet at eclipso.eu>, nouveau at lists.freedesktop.org
Subject: Re: [Nouveau] [PATCH 4/9] nvkm/fb/ramnv50: Ressurect timing code, use  proper timing/rammap handlers

> On 23.05.2015 00:33, Roy Spliet wrote:
> > Might need some generalisation to < GT200. For those: use at your
> own risk!
> >
> > Signed-off-by: Roy Spliet <rspliet at eclipso.eu>
> > ---
> >   .../drm/nouveau/include/nvkm/subdev/bios/ramcfg.h  |  16 ++
> >   .../drm/nouveau/include/nvkm/subdev/bios/rammap.h  |   2 +
> >   drivers/gpu/drm/nouveau/nvkm/subdev/bios/rammap.c  |  29 ++++
> >   drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c   | 167 +++++++++++++++++----
> 
> >   4 files changed, 181 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/ramcfg.h
> b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/ramcfg.h
> > index c6fb6aa..f09b6bf 100644
[...]
> > +	timing[0] = (T(RP) << 24 | T(RAS) << 16 | T(RFC) <<
> 8 | T(RC));
> > +	timing[1] = (T(WR) + 1 + T(CWL)) << 24 |
> > +		    max_t(u8, T(18), 1) << 16 |
> > +		    (T(WTR) + 1 + T(CWL)) << 8 |
> > +		    (3 + T(CL) - T(CWL));
> > +	timing[2] = (T(CWL) - 1) << 24 |
> > +		    (T(RRD) << 16) |
> > +		    (T(RCDWR) << 8) |
> > +		    T(RCDRD);
> > +	timing[3] = (unkt3b - 2 + T(CL)) << 24 |
> > +		    unkt3b << 16 |
> > +		    (T(CL) - 1) << 8 |
> > +		    (T(CL) - 1);
> > +	timing[4] = (cur4 & 0xffff0000) |
> > +		    T(13) << 8 |
> > +		    T(13);
> > +	timing[5] = T(RFC) << 24 |
> > +		    max_t(u8, T(RCDRD), T(RCDWR)) << 16 |
> > +		    T(RP);
> > +	/* Timing 6 is already done above */
> > +	timing[7] = (cur7 & 0xff00ffff) | (T(CL) - 1) << 16;
> > +	timing[8] = (cur8 & 0xffffff00);
> > +
> > +	/* XXX: P.version == 1 only has DDR2 and GDDR3? */
> 
> mh can you state both (DDR2/GDDR3) here and bail out on default as well?

This type of error checking is way earlier, in nv50_ram_ctor(). Hence, this code will
currently only be executed with GDDR3 cards, I just didn't feel it's justified to remove
DDR2 conditional code that I investigated years ago, otherwise it would have been even
slightly cleaner.

Roy