Re: [PATCH 2/5] drm/i915: retrieve current fb config into new plane_config structure at init

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Sat, 23 Nov 2013 01:26:34 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Fri, Nov 22, 2013 at 03:21:08PM -0800, Jesse Barnes wrote:
> > On Sat, 23 Nov 2013 01:08:17 +0200
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > 
> > > On Fri, Nov 22, 2013 at 01:55:35PM -0800, Jesse Barnes wrote:
> > > > On Wed, 20 Nov 2013 15:10:39 +0200
> > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > > > > +	case DISPPLANE_8BPP:
> > > > > > +		return DRM_FORMAT_C8;
> > > > > > +	case DISPPLANE_BGRX555:
> > > > > > +		return DRM_FORMAT_ARGB1555;
> > > > >                                   ^
> > > > > X
> > > > 
> > > > Oops fixed, thanks.  These formats are a lot of typing.
> > > > 
> > > > > > +	case DISPPLANE_RGBA888:
> > > > > > +	case DISPPLANE_RGBX101010:
> > > > > > +	case DISPPLANE_RGBA101010:
> > > > > > +	case DISPPLANE_BGRX101010:
> > > > > > +		plane_config->bpp = 32;
> > > > > > +		break;
> > > > > > +	}
> > > > > 
> > > > > Maybe just intel_format_to_fourcc()+drm_format_plane_cpp() or something.
> > > > > Just to avoid duplicating essentially the same code.
> > > > 
> > > > Ah yeah, good idea, fixed.
> > > > 
> > > > > > +	val = I915_READ(HTOTAL(pipe));
> > > > > > +	plane_config->fb_width = (val & 0xffff) + 1;
> > > > > > +	val = I915_READ(VTOTAL(pipe));
> > > > > > +	plane_config->fb_height = (val & 0xffff) + 1;
> > > > > 
> > > > > Why make the fb the size of htotal/vtotal? pipe src size should be all
> > > > > we need.
> > > > 
> > > > Ok fixed.  Are there no cases where they'll be different?  Maybe
> > > > centered modes would have a pipesrc that differs from the
> > > > htotal/vtotal?  But even in that case we just want the pipesrc.
> > > 
> > > htotal/vtotal should have nothing to do with the fb dimensions.
> > > pipesrc (or plane w/h if we had such registers for primary planes)
> > > are the only things we can really tell. The information whether
> > > the fb was orignally larger than that can't be recovered.
> > 
> > We couldn't get that from the scaling regs?  If you're up or
> > downscaling, don't you have to specify a factor somewhere?  Either in
> > the pfit regs or the sprite scaling regs?
> 
> Scaling doesn't matter. Pipesrc defines the size of the viewport
> into the fb. Scaling regs would just tell us whether that gets up
> or downscaled by some amount.

Oh yeah for viewing just a portion of the fb... yeah in that case we'd
be hosed.  But otoh, this code would just preserve the viewable region
so maybe no one would be the wiser...

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