On Wed, 27 Apr 2022 21:44:34 -0300 Igor Torrente <[email protected]> wrote:
> On 4/27/22 04:43, Pekka Paalanen wrote: > > On Tue, 26 Apr 2022 22:22:22 -0300 > > Igor Torrente <[email protected]> wrote: > > > >> On April 26, 2022 10:03:09 PM GMT-03:00, Igor Torrente > >> <[email protected]> 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 <[email protected]> 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
pgp7rwRA1SQh0.pgp
Description: OpenPGP digital signature
