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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

-- 
Ville Syrjäl 
Intel OTC