Hi Eric,

On Mon, Mar 12, 2018 at 12:02:26PM -0700, Eric Anholt wrote:
> > +static pixman_image_t *convert_frame_format(pixman_image_t *src,
> > +                                       int format)
> > +{
> > +   pixman_image_t *converted;
> > +   unsigned int w = pixman_image_get_width(src);
> > +   unsigned int h = pixman_image_get_height(src);
> > +   void *data = malloc(w * h * 4);
> > +
> > +   memset(data, 0, w * h * 4);
> > +   converted = pixman_image_create_bits(format, w, h, data,
> > +                                        PIXMAN_FORMAT_BPP(format) / 8 * w);
> > +   pixman_image_composite(PIXMAN_OP_ADD, src, NULL, converted,
> > +                          0, 0, 0, 0, 0, 0, w, h);
> > +   return converted;
> > +}
> 
> Instead of the memset, you could just use PIXMAN_OP_SRC.

I guess PIXMAN_OP_ADD will do a composition between the existing
buffer content and the new one, while PIXMAN_OP_SRC will just take
whatever comes from the buffer and overwrite the previous content?

> Also, instead of "* 4", probably want "* PIXMAN_FORMAT_BPP(format) / 8"
> there too.

Ah, yes, definitely.

> > +#define PIXEL_MASK 0x00f8f8f8
> 
> If we're going to have some tolerance, the tolerance should probably
> depend on the lowest depth of the channel involved.

Indeed, yes.

> However, I'm not sure this is needed -- we should be able to get
> bit-exact from VC4 by filling in the size-extension bits in the HVS.

I guess it really depends on what we're aiming for here. While it
would probably work with VC4, I think this part is rather generic, so
I guess we could end up in scenarios where the CRTC will not fill the
bits to our expected value and where it wouldn't be something we can
change.

I just looked at the Allwinner display engine for example, and it
didn't have such configuration (documented, at least).

That's why I went with this approach here (but I can definitely change
it if we want to deal with that theorical case later).

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to