On Wed, 27 Apr 2022 21:44:34 -0300
Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
> On 4/27/22 04:43, Pekka Paalanen wrote:
> > On Tue, 26 Apr 2022 22:22:22 -0300
> > Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
> >
> >> On April 26, 2022 10:03:09 PM GMT-03:00, Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
> >>>
> >>>
> >>> On 4/25/22 22:54, Igor Torrente wrote:
> >>>> Hi Pekka,
> >>>>
> >>>> On 4/25/22 05:10, Pekka Paalanen wrote:
> >>>>> On Sat, 23 Apr 2022 15:53:20 -0300
> >>>>> Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
> >>>>>
> >
> > ...
> >
> >>>>>>>>> +static void argb_u16_to_XRGB8888(struct vkms_frame_info *frame_info,
> >>>>>>>>> + const struct line_buffer *src_buffer, int y)
> >>>>>>>>> +{
> >>>>>>>>> + int x, x_dst = frame_info->dst.x1;
> >>>>>>>>> + u8 *dst_pixels = packed_pixels_addr(frame_info, x_dst, y);
> >>>>>>>>> + struct pixel_argb_u16 *in_pixels = src_buffer->pixels;
> >>>>>>>>> + int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> >>>>>>>>> + src_buffer->n_pixels);
> >>>>>>>>> +
> >>>>>>>>> + for (x = 0; x < x_limit; x++, dst_pixels += 4) {
> >>>>>>>>> + dst_pixels[3] = (u8)0xff;
> >>>>>>>>
> >>>>>>>> When writing to XRGB, it's not necessary to ensure the X channel has
> >>>>>>>> any sensible value. Anyone reading from XRGB must ignore that value
> >>>>>>>> anyway. So why not write something wacky here, like 0xa1, that is far
> >>>>>>>> enough from both 0x00 or 0xff to not be confused with them even
> >>>>>>>> visually? Also not 0x7f or 0x80 which are close to half of 0xff.
> >>>>>>>>
> >>>>>>>> Or, you could save a whole function and just use argb_u16_to_ARGBxxxx()
> >>>>>>>> instead, even for XRGB destination.
> >>>>>>>
> >>>>>>>
> >>>>>>> Right. Maybe I could just leave the channel untouched.
> >>>>>
> >>>>> Untouched may not be a good idea. Leaving anything untouched always has
> >>>>> the risk of leaking information through uninitialized memory. Maybe not
> >>>>> in this case because the destination is allocated by userspace already,
> >>>>> but nothing beats being obviously correct.
> >>>>
> >>>> Makes sense.
> >>>>
> >>>>>
> >>>>> Whatever you decide here, be prepared for it becoming de-facto kernel
> >>>>> UABI, because it is easy for userspace to (accidentally) rely on the
> >>>>> value, no matter what you pick.
> >>>>
> >>>> I hope to make the right decision then.
> >>>
> >>> The de-facto UABI seems to be already in place for {A, X}RGB8888.
> >>
> >> "Only XRGB_8888
> >
> > If that's only IGT, then you should raise an issue with IGT about this,
> > to figure out if IGT is wrong by accident or if it is deliberate, and
> > are we stuck with it.
> >
> > This is why I would want to fill X with garbage, to make the
> > expectations clear before the "obvious and logical constant value for X"
> > makes a mess by making XRGB indistinguishable from ARGB. Then the next
> > question is, do we need a special function to write out XRGB values, or
> > can we simply re-use the ARGB function.
> >
> > Do the tests expect X channel to be filled with 0xff or with the actual
> > A values? This question will matter when all planes have ARGB
> > framebuffers and no background color. Then even more questions will
> > arise about what should actually happen with A values (blending
> > equation).
>
> I dig into the igt code a little bit and found that it's waiting for the
> channel to not be changed.
> It fills all the pixels in the line with a value and calculates the CRC
> of the entire buffer, including the alpha.
>
> I will crate an issue asking if this is intended.
I just remembered this:
https://lists.freedesktop.org/archives/igt-dev/2022-March/039920.html
Thanks,
pq
Attachment:
pgpytDdxIiGd0.pgp
Description: OpenPGP digital signature
