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

Ville Syrjälä
2013-11-22T23:08:21Z

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.

--
Ville Syrjälä
Intel OTC