[Intel-gfx] [PATCH] drm/i915: Calculate self-refresh watermark by using one unique algorithm on g4x/965/9xx platform

[ykzhao]

On Sat, 2009-11-21 at 00:45 +0800, Jesse Barnes wrote:
> On Fri, 20 Nov 2009 16:39:07 +0800
> yakui.zhao at intel.com wrote:
> 
> > From: Zhao Yakui <yakui.zhao at intel.com>
> > 
> > Use one unique algorithm to calucate the self-refresh watermark on
> > g4x/965/9xx platform, which is done by calling the function of
> > intel_calculate_wm. In this function it will calculate the requried
> > minimum fifo size firstly and then get the expected watermark.
> > 
> > We will configure the cursor self-refresh watermark on 965 platform.
> > Otherwise it will be zero.
> > 
> > Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
> > ---
> 
> G45:
> 
> > -		line_time_us = ((sr_hdisplay * 1000) / sr_clock);
> >  
> > -		/* Use ns/us then divide to preserve precision */
> > -		sr_entries = (((sr_latency_ns / line_time_us) + 1) *
> > -			      pixel_size * sr_hdisplay) / 1000;
> > -		sr_entries = roundup(sr_entries / cacheline_size, 1);
> > -		DRM_DEBUG("self-refresh entries: %d\n", sr_entries);
> > +		srwm = intel_calculate_wm(sr_clock, &g4x_wm_info,
> > +						pixel_size,
> > sr_latency_ns); +
> > +		DRM_DEBUG_KMS("self-refresh watermark: %d\n", srwm);
> 
> 965:
> 
> > planeb_clock;
> > -		line_time_us = ((sr_hdisplay * 1000) / sr_clock);
> > -
> > -		/* Use ns/us then divide to preserve precision */
> > -		sr_entries = (((sr_latency_ns / line_time_us) + 1) *
> > -			      pixel_size * sr_hdisplay) / 1000;
> > -		sr_entries = roundup(sr_entries /
> > I915_FIFO_LINE_SIZE, 1);
> > -		DRM_DEBUG("self-refresh entries: %d\n", sr_entries);
> > -		srwm = I945_FIFO_SIZE - sr_entries;
> > -		if (srwm < 0)
> > -			srwm = 1;
> > -		srwm &= 0x3f;
> > +
> > +		srwm = intel_calculate_wm(sr_clock, &i945_wm_info,
> > +						pixel_size,
> > sr_latency_ns);
> > +		DRM_DEBUG_KMS("self-refresh watermark: %d\n", srwm);
> > +
> 
> 945:
> 
> > -		line_time_us = ((sr_hdisplay * 1000) / sr_clock);
> > -
> > -		/* Use ns/us then divide to preserve precision */
> > -		sr_entries = (((sr_latency_ns / line_time_us) + 1) *
> > -			      pixel_size * sr_hdisplay) / 1000;
> > -		sr_entries = roundup(sr_entries / cacheline_size, 1);
> > -		DRM_DEBUG_KMS("self-refresh entries: %d\n",
> > sr_entries);
> > -		srwm = total_size - sr_entries;
> > -		if (srwm < 0)
> > -			srwm = 1;
> > +
> > +		srwm = intel_calculate_wm(sr_clock, &planea_params,
> > +						pixel_size,
> > sr_latency_ns);
> > +		DRM_DEBUG_KMS("self-refresh watermark: %d\n", srwm);
> >  		I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN | (srwm &
> 
> I don't think a straight unification like this will work unless you fix
> up intel_calculate_wm.  Note that 945 and 965 are "fetch when this many
> entries is free in the FIFO" and GM45+ is "always keep this many
> entries in the FIFO", so for the second two you need to know the total
> FIFO size and subtract the amount of coverage needed.  On GM45 you just
> need to program the coverage (thankfully since FIFO sizing is managed
> by hardware).

Only the self-refresh watermark is touched. This is effective only when
cpu is in C2/C3/C4 and the memory enters the self-refresh mode. The
condition to enter self-refresh mode is that only 1 display pipe is
used.

>From the spec it seems that the entire FIFO is allocated to the single
active display plane if only one single display plane is active. And
from the spec it seems that we can use the total fifo size to calculate
the self-refresh watermark.

If so, is it appropriate to calculate the self-refresh watermark by
using the function of intel_calculate_wm?

And the spec describes the watermark as the following:(965gm/g4x):
   Number in 64bytes of FIFO above which the dispplay streamer will
generate the request to memory.

In such case IMO we can get the self-refresh watermark by subtract the
required entry from the total fifo size.
Is it correct?

thanks.
> 
> What I've been doing with this code is keeping a userland program that
> duplicates the calculations.  That way I can test various inputs easily
> and track down reported bugs.  It would be good if you could do
> something similar and confirm that the outputs are the same before and
> after the change.
> 
> Thanks,