On Wed, 20 Nov 2013 15:10:39 +0200
Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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.
> > +struct intel_plane_config {
> > + int pixel_format; /* DRM fourcc code */
>
> The comment doesn't match how you do things in the code.
Yeah, Bob mentioned that too, fixed.
Does this look useful to you otherwise?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
