On Tue, 26 Nov 2013 19:28:27 +0200
Ville Syrjälä <ville.syrj...@linux.intel.com> wrote:

> On Mon, Nov 25, 2013 at 03:51:15PM -0800, Jesse Barnes wrote:
> > And move it up in the file for earlier usage.
> > 
> > v2: add pre-gen4 support as well (Chris)
> > 
> > Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 53 
> > ++++++++++++++++++++++++++----------
> >  1 file changed, 38 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index e85d838..321d751 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5452,6 +5452,28 @@ static void vlv_crtc_clock_get(struct intel_crtc 
> > *crtc,
> >     pipe_config->port_clock = clock.dot / 5;
> >  }
> >  
> > +static u32
> > +intel_framebuffer_pitch_for_width(struct drm_i915_private *dev_priv, int 
> > width,
> > +                             int bpp, bool tiled)
> > +{
> > +   u32 pitch = DIV_ROUND_UP(width * bpp, 8);
> > +   int align;
> > +
> > +   if (tiled) {
> > +           if (INTEL_INFO(dev_priv->dev)->gen < 4) {
> > +                   /* Pre-965 needs power of two tile width */
> > +                   for (align = 512; align < pitch; align <<= 1)
> > +                           ;
> > +           } else {
> > +                   align = 512;
> > +           }
> 
> Gen2 tiles are 128 bytes wide, not 512 bytes.
> 
> So maybe something like this:
>  if (IS_GEN2()) {
>    return roundup_power_of_two(max(pitch, 128));
>  else if (IS_GEN3())
>    return roundup_power_of_two(max(pitch, 512));
>  else
>    return ALIGN(pitch, 512);
> 
> > +   } else {
> > +           align = 64;

Ah good old gen2.  Pretty sure we've just treated it as 512 wide in the
past?  But I'm not going to go digging around the DDX to see... :)

I was looking for that POT helper but didn't see it, that makes things
a little nicer.

> Also I just noticed in the docs that gen2/3 only seem to require 32byte
> alignment for linear buffers. But relaxing that would require a change
> to intel_framebuffer_init() as well, so it looks like material for
> another patch. Also would need to be tested on real hardware.

Yeah separate patch.  It'll save us gobs of memory. :)

> > -intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
> > +intel_framebuffer_size_for_mode(struct drm_i915_private *dev_priv,
> > +                           struct drm_display_mode *mode, int bpp)
> >  {
> > -   u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp);
> > +   u32 pitch = intel_framebuffer_pitch_for_width(dev_priv,
> > +                                                 mode->hdisplay, bpp,
> > +                                                 false);
> >     return ALIGN(pitch * mode->vdisplay, PAGE_SIZE);
> 
> This should also align the fb height to tile height, otherwise
> intel_framebuffer_init() might reject it.

Hm another existing bug.  Separate patch.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to