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

Jesse Barnes
2013-11-22T22:01:01Z

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.

> > +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@lists.freedesktop.org